On 25.07.2017 2:48, Jim Graham wrote:
This same thing is done in other places in a simpler way without
creating an inner class simply by having createGE() do the assignment
and test for null, as in:
I guess a solution which you mentions is a variant of "Double-checked
locking".
=> after:
public static GraphicsEnvironment getLocalGraphicsEnvironment() {
if (c== null) {
createGE();
}
return localEnv;
}
private static synchronized GraphicsEnvironment createGE() {
if (localEnv != null) return;
...
localEnv = ge;
}
In this variant the field should be volatile, and this will introduce
some synchronization. Otherwise it will be possible to read non-null
value in localEnv while the constructor of GraphicsEnvironment was not
completed on some other thread(but the field was assigned already).
OR => alternate after:
public static synchronized GraphicsEnvironment
getLocalGraphicsEnvironment() {
if (localEnv == null) {
synchronized (GraphicsEnvironment.class) {
if (localEnv == null) {
localEnv = createGE();
}
}
}
return localEnv;
}
// And no changes to createGE()
In this variant the field also should be volatile. I guess it was a typo
that the getLocalGraphicsEnvironment still "synchronized".
Note that there is a test for null both in getLGE() and also again in
createGE() because the second one is inside a synchronized block and
required to prevent multiple instances. The first one in getLGE()
avoids having to synchronize in the first place for the common case.
Note that in case of holder - after the first call there was not any
synchronization(like in case of "synchronized" and "volatile").
Here some additional information which I used:
https://shipilev.net/blog/2014/safe-public-construction/
...jim
On 7/24/17 5:09 PM, Sergey Bylokhov wrote:
Hello,
Please review the fix for jdk10.
Bug: https://bugs.openjdk.java.net/browse/JDK-8185093
Webrev can be found at:
http://cr.openjdk.java.net/~serb/8185093/webrev.00
jmh test: http://cr.openjdk.java.net/~serb/8185093/LocalGEproblem.java
While working on some other bug I found a performance issue in our
java2d pipeline in multi-threaded environment. We have a "hot spot" in
the "GraphicsEnvironment.getLocalGraphicsEnvironment()". This method
is executed every time the graphics object is created, for example
BufferedImage.java:
public Graphics2D createGraphics() {
GraphicsEnvironment env =
GraphicsEnvironment.getLocalGraphicsEnvironment();
return env.createGraphics(this);
}
So even if the application will draw to a different Images it will be
blocked for some time in this method.
I created a jmh test case which shows that implementing this method
via holder will speedup the next code:
Graphics2D graphics = bi.createGraphics();
graphics.drawLine(0, 0, 100, 100);
4 Threads:
- Before the fix: 8922 ops/ms
- After the fix : 9442 ops/ms
8 Threads:
- Before the fix: 4511 ops/ms
- After the fix : 11899 ops/ms
The main issue which I would like to clarify is it possible to remove
synchronize keyword from the getLocalGraphicsEnvironment(), possibly
with updating a specification? We have similar issues in other parts
of code which I would like to update later after this one.
--
Best regards, Sergey.
