The Client Libraries Group

Introduction

The Client Libraries Group is comprised of developers who participate in the design, implementation, and maintenence of the Java client libraries. It is a consolidation of the former 2D, AWT, Sound, and Swing Groups.

Archived documentation for each of the consolidated Groups may be found in the original Group pages:

These currently need some updates to remove obsolete information but there's still a lot of valid material there.

Community

Source code

The Client Libraries Group is responsible for the JDK source in the following modules

Together these modules enable

Like much of the rest of the JDK as much as possible of the implementation is written using Java, but because of the nature of platform integration there is still a vast amount of native code, much of it platform-specific. The majority of shared (cross-platform) native code is widely used open source libraries written in C or C++ for specialised functions such as font rasterisation and color management.

Demos

Additionally the Client Libraries Group maintains several demo applications. Not meant as an example of modern standard coding practice, but principally meant as named, to "demo" the functionality of the APIs : https://github.com/openjdk/jdk/tree/master/src/demo/share/jfc. They are also useful for manual testing of this functionality and it is not unusual for a reviewer to suggest running them to verify a fix.

Tests

The jtreg tests for the Client Libraries Group code are somewhat scattered around, since the tests tend to be organised by package name and there are plenty of those. Many of the tests are automated, but also many are manual. An incomplete list of directories for these tests is

The ongoing work of the Client Libraries Group

There's no plan to do "large new features' for Swing, Java2D etc but

Recent, Current and Upcoming Projects/JEPs etc

Some client contributors also contribute to the OpenJFX Project - there's a more or less 100% overlap in the skill set !


Group Policies.

First, refer to the OpenJDK Developer's Guide for JDK-wide policies, processes and practices. However there are some extra considerations for the client area.

Proposing a change

Like most other groups in OpenJDK for anything that is more than a localised bug fix, we prefer to see issues raised and discussed before we see a PR which may reflect a different direction than we'd want. This is especially true if you are a new contributor. The client-libs-dev@openjdk.org list is the place to go for these discussions

"Drive-by" contributions - will get a hard look because we prefer folks who contribute to stick around and maintain those contributions.

There are additional challenges of contributing to the Client Libraries Group area compared to some other areas since so much of what we do is platform-specific that you may need access to multiple platforms and be prepared to do a lot of cross-platform and manual testing. Depending on your reviewers for that isn't likely to speed along your contribution.

Guidelines on what kinds of changes we would like to see - or not see.

Preamble

Every change in JDK code carries risk of changes in behaviour which adversely affect applications. Generally we are looking to improve the functionality and capability and sometimes performance of the platform without that negative consequence. We do have many thousands of tests, but they are inevitably incomplete in coverage. So we need to ask ourselves, whether each change is worthwhile and some may not be no matter how well intentioned.

A lot of proposed changes may be broadly categorised as "internal code only" changes. This includes more than moving files and functions around. If a change being proposed is just a "code clean up", or "optimization" it may be of low value relative to the costs and risks. It is perhaps most succinctly characterised as any change that changes the JDK product source code without making things perceptibly better for applications. There isn't an agreed on single term for all of this, "refactoring" covers some of it, and all of the abovall of the above is the intended reading of that term when used in this doc.

So the guidelines that follow are something that should be read by developers before spending a lot of time on a proposed fix.

And is important to ensure that even a "cleanup" code change has been exercised by a test, so see Testing a Change

Guidelines and criteria that relate to changes we DO want

These do not automatically guarantee a change will be accepted, but if one of these is the focus of a change, it is much more likely to be accepted.
  1. Correctness changes are welcome.
    If there's something that affects program correctness, that's important. And to broaden this, what is most valuable is fixing of all kinds bugs that really make things better for applications in ways they can detect. i.e functional bugs
  2. Making the JDK code "more robust" for applications.
    Such changes are probably good, where "more robust" means say using some new platform API which is newer.
  3. Changes which make the platform more secure.
    Changes which are intended to increase the overall security of the platform following secure coding practices and using appropriate platform APIs for that are usually welcome. The exception might be if it is a potentially destabilising change in code where there is only a theoretical problem. NOTE: if you think you found a real security bug that might compromise the platform, then you should follow the process here

Guidelines and criteria that might cause changes to be less likely to be accepted

These do not automatically disqualify a change, but if any of these characterise a proposed change without some benefit from the above list, you should think twice, and be ready to justify it, and perhaps for it ultimately not to be accepted.
  1. Stable code is not a good candidate for refactoring.
    Much of the code in java.desktop has been stable for a long time, and if it is stable, perhaps it has few bugs or need for change. So changes in stable code naturally have a higher risk to reward ratio.
  2. Performance changes need to really matter
    Can you demonstrate a user perceptible change ?
    If you can't measure the change, or you measure it and is small and perhaps once per VM then maybe it isn't worth it.
    Some API may be more performant than another but in UI applications the CPU is often just waiting for the user. Collection class "A" may be faster than class "B" as demonstrated in some benchmarks, but if that's when there are 10,000 entries and client code never has more than 100 and the CPU can traverse the collection faster than the user can move a mouse one pixel .. what have you added ?
  3. Modernising API usage needs to be more than for its own sake.
    "There's a cool new feature in JDK 17/C++ 20/etc", is not enough if you need to update stable code to use it, and there's no other benefit.
  4. Be a subject matter expert, not just a language expert.
    Fixes that say "this is a better way of writing this code" are not guaranteed to be safe, no matter how much the submitter writes claims like "safe, simple, more clear". A change of behaviour is always possible and unless you understand the code you are changing at more than the core language/API level, and have looked into appropriate tests and can speak to the risks, then you should first find a subject matter expert to discuss it with.
  5. Large changes - whether by complexity or volume - are more risky.
    For volume, you may think of this as no different than proposing 10 individual small unrelated changes but what happens in practice is there's a focus on say 5-6 of these and no one really looks hard at the other 4-5. For complexity, even small changes that are hard to understand and test may be risky.
  6. Backports may be more costly in refactored code.
    This is perhaps less a guideline than a warning. If some file in mainline has been refactored and there's then an important functional bug found that needs to be fixed and backported, then the more similar the code across releases then the more straightforward the backport. Refactoring means a backport may be almost a new bug fix. So this is another added cost to weigh against the benefit of the change.
  7. There is no such thing as a zero risk change.
    Once you start changing product code, its very hard to prove otherwise. Supposedly equivalent APIs may not be the drop in replacement you thought. Even removing dead code (!) has mysteriously caused performance regressions - but yes in general removing dead code is a good thing.

Finally

The area lead will always have discretion to evolve these guidelines, and if there's a PR that falls between the lines, to decide which side it falls. Amendments to these guidelines may follow such cases.


Source code conventions

All source code should follow the standard Java source code conventions as well as jcheck rules. Some of the most common ones to remember are : Consult Code Conventions for the Java(TM) Programming Language for the full list.

Testing a change

Do not overlook testing. Test before even submitting your PR.

Regression tests

Tests should be provided unless clearly infeasible. Automated tests are desirable. SQE rarely run manual tests. Nor will other developers. Don't give up easily. There are tests that render to a BufferedImage and analyse the resulting contents to see if they behaved correctly, so writing automated tests is possible in more cases than immediately apparent.

Code Reviews

Code reviews are one of the most important mechanisms we have for integrating and shipping good, solid, compatible code. Reviews are also an invaluable source of education for all of us, and a great mechanism for ensuring consistent code quality, as we all read and learn from reading each other's code.

The standard requirement in the JDK project is for one reviewer to approve the fix. The Java Client Library Group has always standardized on two approvals - where at least one must have the Reviewer role. Historically this was addressed entirely by social conventions but today the tooling plays a role - and the JDK project is set up to mark a PR as ready for integration after a single approval by a person with the Reviewer role - which is not consistent with the Client Libraries policy. The tooling cannot automatically enforce this on a per-module basis and it is not reasonable to expect others to add "/reviewers 2" to every PR. The fixer therefore needs to understand the policies and wait for a second approval.

Note that there may sometimes be confusion between the terms "reviewers" and "approvers". The tooling tends to use the word review/reviewer where it really means "Approval by someone with the Reviewer role". Anyone with an OpenJDK github role can "review" a fix - meaning make comments, and even add an Approval. And indeed we encourage non-Reviewers to actively review PRs. So really you only have two "reviews" when you have two "approvals". If more than one person has expressed a viewpoint on a fix, and say one has approved and the other made substantive comments, without expressly approving, then they are active in reviewing that fix. So you need to take the nature of their comments into consideration, and allow them reasonable time (let's say 24 hrs during the work week for non-urgent fixes) to respond to any written answers or code updates and add their approval before pushing. This is both for courtesy, and so that someone who has spent time on the review may be credited in the commit, and to ensure that they have at least a chance to agree you've addressed their concerns. . Fixers who are not also Reviewers for the client area should consult a client Reviewer to determine if one reviewer is enough. Potential reasons for granting that exception are enumerated below.

The choice of which people review the code is not usually up to the fixing engineer and and who should review it depends upon each specific situation, but some general guidelines are:

It is the responsibility of the implementing engineer to respond to the reviewers, and their concerns, and make the final code adhere to changes agreed upon by the engineer and the reviewers.

It is the responsibility of the reviewers to provide timely reviews, and understand (to the extent possible) and agree to the changes that the engineer has implemented; when the code is putback to the repository, the reviewers are also taking responsibility for these changes. We can only have good reviews, and good resulting code, if the reviewers take their jobs seriously and review the changes thoroughly. Given the costs and hassles of maintaining backward-compatible code indefinitely, we cannot risk code going in that is only cursorily reviewed; it is far easier and cheaper to catch flaws in the review process than it is to fix them in bugs and escalations later on.

The most common exceptions to the two reviewer policy would be for