RFR: 8221392: Reduce ConcurrentGCThreads spinning during start up

[Per Liden]

On 03/25/2019 04:08 PM, Kim Barrett wrote:
>> On Mar 25, 2019, at 4:36 AM, Per Liden <per.liden at oracle.com> wrote:
>>
>> During startup, ConcurrentGCThreads end up in a loop sleeping for 1ms over and over (typically 30-50 times), until is_init_completed() is true. This is not optimal, and we should instead have them wait until they are signaled. This problem has been observed before, as comments in JDK-8136854 suggests.
>>
>> This patch introduces wait_init_completed(), which will block until set_init_completed() is called. Note that the newly introduced InitCompleted_lock is only used to control the interaction between wait_init_completed() and set_init_completed(). is_init_completed() is still lock-free and was just cleaned up to use proper atomic load_acquire/release_store semantics.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8221392
>> Webrev: http://cr.openjdk.java.net/~pliden/8221392/webrev.0
>>
>> /Per
> 
> ------------------------------------------------------------------------------
> src/hotspot/share/runtime/init.cpp
>   203   OrderAccess::release_store(&_init_completed, true);
> 
> I don't think this needs a release_store.  If it did, then the access
> in the wait_init_completed loop should also be barriered.  But since
> both of those are under the lock...

The release_store here is to pair it with the load_acquire in 
is_init_completed(), which doesn't grab the lock. Without this, the 
store in set_init_completed() will only be atomic with correct ordering 
for readers who also grab the lock, like we do in wait_init_completed(). 
Makes sense?

> 
> ------------------------------------------------------------------------------
> 
> Looks good, other than that.  I don't need a new webrev for removal of
> the unnecessary release_store.
> 

Thanks for reviewing, Kim!

/Per