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

Sergey Bylokhov Sergey.Bylokhov at oracle.com
Tue Jul 25 22:37:30 UTC 2017

On 25.07.2017 15:03, Jim Graham wrote:
> On 7/25/17 12:53 PM, Sergey Bylokhov wrote:
>> 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 > > Even though the assignment is the last statement in the method?

The assignment include these operations:
  - Allocate memory
  - Initialize the object
  - Assign the object to the field.

These operation may be reordered, so some other thread can see non-null 
reference in the field while initialization was not complete.

> In looking through the article that you sent out (only briefly) it looks 
> like the consideration is that the initialization method run in one 
> thread could reorder the instructions at run time so that the field is 
> assigned before the initialization steps are done, even if the code is 
> written with a different specific ordering.
> I'm guessing that the Java Memory Model ensures that membars are placed 
> to protect static field initialization from similar dangers?

The key feature here is initialization of the class.
The next line will be blocked while the LocalGE class will not be 
     return LocalGE.INSTANCE;
And initialization will start only when this code will run for the first 

> If that is the case then I think we have a few places where we do this 
> "manual conditional synchronization" that should probably be 
> investigated.  If I remember correctly, though, they may use the 
> following paradigm which might not suffer from that issue:
> Does it help if a local variable is used, as in:
> GE env = this.localEnv
> if (env == null) {
>      synchronized (class) {
>          if (this.localEnv == null) {
>              localEnv = createGE();
>          }
>          env = this.localEnv;
>      }
> }
> return env;
> This ensures that the second read from the field is done in the 
> synchronized block for the case that it was originally found to be null, 
> though this might not help if the line "localEnv = createGE()" can 
> reorder code from inside the call to createGE() to code outside of it.  
> However those dangling reordered stores should be resolved before it 
> leaves the synchronized block, no?

I think that it is possible that one thread will do:
  - create an object
  - assign the object to the localEnv.
  - Initialize the object.
And the second thread will do:
  - Read non-null value from the this.localEnv to the env
  - Check env to null
  - Return non-initialized object.

> In any case, I can see that the code used in the webrev will eventually 
> have no dead code at all in the final compiled version of the method 
> whereas this version would minimally at least have a null check that 
> must be executed even though it would be vestigial once the 
> initialization was done...
> +1 for the inner class version - it's the most straightforward way to do 
> this optimally...
Best regards, Sergey.

More information about the 2d-dev mailing list