-
Notifications
You must be signed in to change notification settings - Fork 6.2k
8355078: java.awt.Color.createContext() uses unnecessary synchronization #24771
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
👋 Welcome back serb! A progress list of the required criteria for merging this PR into |
|
/csr |
|
@mrserb This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been no new commits pushed to the ➡️ To integrate this PR with the above commit message to the |
|
@mrserb has indicated that a compatibility and specification (CSR) request is needed for this pull request. @mrserb please create a CSR request for issue JDK-8355078 with the correct fix version. This pull request cannot be integrated until the CSR request is approved. |
|
Is a CSR requested for the access flags change? Some method flags, notably |
| protected ColorPaintContext(int color, ColorModel cm) { | ||
| private final int color; | ||
| private volatile WritableRaster savedTile; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since ColorPaintContext is no longer cached within the Color class, it's unlikely that this object will be accessed by multiple threads in typical JDK usage. Therefore, the synchronized modifier has been removed from the getRaster() method. However, to ensure thread safety in the rare case where a ColorPaintContext instance is shared across threads (if app decides to do so for some reason), the savedTile field has been declared as volatile.
Webrevs
|
|
/integrate |
|
Going to push as commit 890456f.
Your commit was automatically rebased without conflicts. |
At some point, java.awt.Color.createContext() was marked as synchronized to support caching of a ColorPaintContext instance. The cache was later removed, but the synchronized modifier remained. Since there is no shared mutable state anymore, the synchronization is no longer necessary and can be safely removed.
Note: This code path is rarely executed because a special optimization was introduced in SunGraphics2D.setPaint. As a result, unless a custom wrapper around the Color class is used, the Color.createContext() method is typically bypassed during rendering.
Two tests are added:
Progress
Issues
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/24771/head:pull/24771$ git checkout pull/24771Update a local copy of the PR:
$ git checkout pull/24771$ git pull https://git.openjdk.org/jdk.git pull/24771/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 24771View PR using the GUI difftool:
$ git pr show -t 24771Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/24771.diff
Using Webrev
Link to Webrev Comment