RFR: 8225344: Avoid VM_EnableBiasedLocking VM operation during bootstrap if BiasedLockingStartupDelay is 0

[Daniel D. Daugherty]

On 6/26/19 9:10 AM, Claes Redestad wrote:
> Hi,
>
> please review this cleanup to avoid a safepointing VM operation during
> bootstrap to enable biased locking if the configured delay is zero
> (current default).
>
> Webrev: http://cr.openjdk.java.net/~redestad/8225344/open.00/

src/hotspot/share/runtime/biasedLocking.cpp
     L45: static bool _biased_locking_enabled = false;

         In old code set here:
             old L69:     _biased_locking_enabled = true;
         in VM_EnableBiasedLocking::doit() (so at a safepoint).

         In new code set here:
             new L56:   _biased_locking_enabled = true;
         in enable_biased_locking() which is called from:

             new L72:     enable_biased_locking();
             new L108:       enable_biased_locking();

         L72 is in VM_EnableBiasedLocking::doit() so no change.
         L108 is in BiasedLocking::init() which is called from
         fairly late in Threads::create_vm() so this is new.

         _biased_locking_enabled was queried in BiasedLocking::enabled():

             old L116:   return _biased_locking_enabled;

         and is now queried like this:

             new L116:   return _biased_locking_enabled || 
BiasedLockingStartupDelay == 0;

         By checking "BiasedLockingStartupDelay == 0" you are guarding
         against an early call to BiasedLocking::enabled() seeing the
         initial _biased_locking_enabled value of 'false' before
         BiasedLocking::init() can set it true. I don't know if it is
         possible for another thread to call BiasedLocking::enabled()
         before the "creat_vm()" thread has called BiasedLocking::init(),
         but your check of "BiasedLockingStartupDelay == 0" should fix
         that case. I think you need a short comment explaining your
         reasoning. Perhaps:

           // We check "BiasedLockingStartupDelay == 0" here to cover the
           // possibility of a call to BiasedLocking::enabled() before
           // BiasedLocking::init() has been called.

src/hotspot/share/classfile/systemDictionary.cpp
     No comments.

Thumbs up! Thanks for detangling this.

Dan


> Bug: https://bugs.openjdk.java.net/browse/JDK-8225344
>
> While this avoids a safepoint during bootstrap it's effectively
> performance neutral on all startup and throughput tests I've done.
> Still, it seems like a good cleanup to reduce the amount of visible
> (logged) work happening during a typical bootstrap.
>
> Testing: tier1-4
>
> Thanks!
>
> /Claes