RFR: 8221392: Reduce ConcurrentGCThreads spinning during start up

[Erik Osterlund]

Hi,

I agree, I would keep the release in, rather than riding on the fencing of the lock. It’s easier to get that they are paired. If anyone was to add code inside of that critical section in the future, they might not have anticipated that the locking was intended to release a store inside of the critical section, which could lead to unnecessary bugs due to the implicitness. 

Eliding it will at the very least require a comment explaining how the release has been merged with the locking, to avoid future mistakes. But I would prefer to just have release_store there, as it is self explanatory.

Thanks,
/Erik

> On 26 Mar 2019, at 11:06, Per Liden <per.liden at oracle.com> wrote:
> 
> 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