Re: [OpenJDK 2D-Dev] [10] Review Request: 8185093 Expensive multi-core choke point when any graphics objects are created

[Jim Graham]

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:

before:
    public static synchronized GraphicsEnvironment 
getLocalGraphicsEnvironment() {
        if (localEnv == null) {
            localEnv = createGE();
        }

        return localEnv;
    }

    private static GraphicsEnvironment createGE() {
        ...
        return ge;
    }

=> after:

    public static GraphicsEnvironment getLocalGraphicsEnvironment() {
        if (localEnv == null) {
            createGE();
        }

        return localEnv;
    }

    private static synchronized GraphicsEnvironment createGE() {
        if (localEnv != null) return;
        ...
        localEnv = ge;
    }

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()

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.

                        ...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.