RFR: 8221392: Reduce ConcurrentGCThreads spinning during start up

[Per Liden]

Hi Kim,

On 2019-03-26 01:57, Kim Barrett wrote:
[...]
> So no, that doesn't make sense to me; I don't know what theory of
> usage leads to the release_store, since the locking done by the writer
> also makes the pairing unnecessary.  I don't think the release does
> anything useful, and I don't think we should be using OrderAccess
> operations when they don't serve some purpose.
> 
> (FWIW, I think both the load in wait and the store probably ought to
> be Atomic::load and Atomic::store (e.g. weak atomic accesses), because
> of the atomic access by the predicate (e.g. the type of the variable
> ought to be some Atomic<T> that we don't have); but that usage is
> pretty rare in HotSpot code.)
> 

Let me try to better explain how I think about this.

set_init_completed() needs to coordinate its actions with two other 
functions, is_init_completed() and wait_init_completed(). These can be 
thought of as two completely separate problems. The acquire/release pair 
coordinates the ordering between set_init_completed() and 
is_init_completed(), and the lock coordinates the ordering between 
set_init_completed() and wait_init_completed().

Now, it turns out that because of the strength of the lock we can 
optimize the release store in set_init_completed() into a relaxed store. 
But then we kind of loose or mix these two, otherwise separate, ordering 
problems. I'm not sure I think that worth doing. We certainly don't need 
that optimization, and I personally find it much easier to think about 
this as two separate non-overlapping problems.

I'm not sure if that explanation helped, but that's how I'm looking at this.

Do other people have opinions on this?

cheers,
Per