RFR (S) 8213137: Remove static initialization of monitor/mutex instances

[David Holmes]

Hi Dan,

Thanks for looking at this.

On 6/11/2018 2:44 AM, Daniel D. Daugherty wrote:
> On 11/4/18 8:43 PM, David Holmes wrote:
>> This impacts GC, compiler, runtime and serviceability.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8213137
>> webrev: http://cr.openjdk.java.net/~dholmes/8213137/webrev/
> 
> src/hotspot/share/gc/shared/parallelCleaning.cpp
>      No comments.
> 
> src/hotspot/share/gc/shared/parallelCleaning.hpp
>      No comments.
> 
> src/hotspot/share/jfr/periodic/sampling/jfrThreadSampler.cpp
>      No comments.
> 
> src/hotspot/share/runtime/mutexLocker.cpp
>      L324:   def(JfrStacktrace_lock           , PaddedMutex  , 
> special,     true,  Monitor::_safepoint_check_sometimes);
>      L325:   def(JfrThreadSampler_lock, PaddedMonitor, leaf, true, 
> Monitor::_safepoint_check_never);
>          Please match the spaces before comma format used on L324.

Fixed

>      L334:   def(NMethodSweeper_stat_lock     , PaddedMutex  , 
> special,     true,  Monitor::_safepoint_check_sometimes);
>      L336:   def(Decoder_shared_decoder_lock,   PaddedMutex, 
> native,      false, Monitor::_safepoint_check_never);
>      L337:   def(DCmdFactory_lock,              PaddedMutex, 
> leaf,        true,  Monitor::_safepoint_check_never);
>          Please match the spaces before comma format used on L334.

Fixed (hard to get used to deliberately breaking the style rules!)

> src/hotspot/share/runtime/mutexLocker.hpp
>      No comments.
> 
> src/hotspot/share/runtime/sweeper.cpp
>      No comments.
> 
> src/hotspot/share/runtime/sweeper.hpp
>      No comments.
> 
> src/hotspot/share/runtime/threadSMR.cpp
>      No comments.
> 
> src/hotspot/share/runtime/threadSMR.hpp
>      No comments.
> 
> src/hotspot/share/services/diagnosticFramework.cpp
>      No comments.
> 
> src/hotspot/share/services/diagnosticFramework.hpp
>      No comments.
> 
> src/hotspot/share/utilities/decoder.cpp
>      L100:   MutexLockerEx locker(error_handling_thread ? NULL : 
> shared_decoder_lock(), true);
>      L110:   MutexLockerEx locker(error_handling_thread ? NULL : 
> shared_decoder_lock(), true);
>      L121:   MutexLockerEx locker(error_handling_thread ? NULL : 
> shared_decoder_lock(), true);
>          Not your mess, but those literal 'true' params should be replaced
>          with: Mutex::_no_safepoint_check_flag
>          Since you are touching these lines, would you mind fixing it?

Sure no problem. But in doing so I noticed an issue in this code - it 
defines a DecoderLocker class to encapsulate all that stuff but then 
doesn't use it at all. I'll file a RFE to either use it or lose it. :)

> src/hotspot/share/utilities/decoder.hpp
>      No comments.
> 
> Thumbs up. If you choose to fix the nits, I do not need to
> see another webrev.

Webrev updated in place for the benefit of later reviewers.

Thanks,
David

> Dan
> 
> 
>>
>> To facilitate changes to the Mutex/Monitor code we need to ensure 
>> there are no statically initialized Monitor/Mutex instances in the VM 
>> - as the constructors may rely on VM initialization that has not 
>> happened when C++ initializers execute.
>>
>> There were 6 locally defined "lock" members of classes that were 
>> statically initialized, and these are across all areas of the VM:
>>
>>  - Decoder::_shared_decoder_lock
>>  - DCmdFactory::_dcmdFactory_lock
>>  - JfrThreadSampler::_transition_block_lock
>>  - NMethodSweeper::_stat_lock
>>  - ThreadsSMRSupport::_delete_lock
>>  - CodeCacheUnloadingTask::_lock
>>
>> The last is actually now unused and so is just deleted. The rest join 
>> all other Monitor/Mutex instances defined in the global list in 
>> MutexLocker and initialized in mutex_init(). I've verified that none 
>> of the monitor/mutex instances can be used prior to mutex_init, by 
>> code inspection - more details in the bug report.
>>
>> I investigated asserting that a Monitor/Mutex is only constructed 
>> during mutex_init() but assertions don't fail cleanly during C++ 
>> static initialization - the result is that during the build jmod goes 
>> into an infinite loop printing out "[Too many errors, abort]".
>>
>> Tested using tier1-3
>>
>> Thanks,
>> David
>