Skip to content

I20220324-1800

@carstenartur carstenartur tagged this 24 Mar 21:37
Reduce synchronization scope for few methods to the minimum according to
the well known double-check idiom.

The duration of the test

org.eclipse.jdt.ui.tests.quickfix.CleanUpStressTest.testAllCleanUps()

drops a few seconds with this change.

Besides there are a few cases where methods are changed that did not use
the synchronize keyword. It should not be slower this way but safer. But
it is not clear if it really is needed.

Basically this change should have been applied already when the code was
migrated to depend as minimum on java 5. I did not change all code
affected but a few cases.

Here a quote from Joshua Bloch (see last link)

If you need high-performance lazy initializing of an instance field, use
the double-check idiom with a volatile field. This idiom wasn't
guaranteed to work until release 5.0, when the platform got a new memory
model. The idiom is very fast but also complicated and delicate, so
don't be tempted to modify it in any way. Just copy and paste --
normally not a good idea, but appropriate here:

// Double-check idiom for lazy initialization of instance fields
private volatile FieldType field;

private FieldType getField() {
   FieldType result = field;
   if (result != null)    // First check (no locking)
       return result;

   synchronized(this) {
       if (field == null) // Second check (with locking)
           field = computeFieldValue();
       return field;
   }
}

see https://docs.google.com/document/d/1mAeEgQu4H4ADxa03k7YaVDjIP5vJBvjVIjg3DIvoc8E/edit

don't use
https://www.oracle.com/technical-resources/articles/javase/bloch-effective-08-qa.html

Change-Id: I7eb783a7feef0d544fcbd4c30938668032be8b12
Signed-off-by: Carsten Hammer <carsten.hammer@t-online.de>
Reviewed-on: https://git.eclipse.org/r/c/jdt/eclipse.jdt.ui/+/173560
Tested-by: JDT Bot <jdt-bot@eclipse.org>
Reviewed-by: Jeff Johnston <jjohnstn@redhat.com>
Assets 2
Loading