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

Jim Graham james.graham at oracle.com
Tue Jul 25 09:48:43 UTC 2017


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


More information about the 2d-dev mailing list