RFR: 8221392: Reduce ConcurrentGCThreads spinning during start up
kim.barrett at oracle.com
Wed Mar 27 00:14:01 UTC 2019
> On Mar 26, 2019, at 6:06 AM, 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.
OK, that seems like a defensible position. I would come at the problem
from a different direction, and I still think that use of release_store
looks really strange and out of place, and wouldn't write it that way myself.
But I don't hate it so much that I can't see your point. So I'll withdraw
that comment, and the change looks good as is.
More information about the hotspot-gc-dev