RFR (S): 8182703: Correct G1 barrier queue lock orderings

[Erik Österlund]

Hi,

Thomas and I discussed offline and came to the following conclusions:
1) Lowering the lock rank of HeapRegionRemSet::_m to access would be 
nice indeed, but probably deserves a separate RFE with further reasoning 
and analysis. Will stick to the queue-related lock ranks in this RFE.
2) We agree mostly about the comments, but I have a new webrev that 
hopefully has even more clear comments regarding the new access rank.

Incremental webrev:
http://cr.openjdk.java.net/~eosterlund/8182703/webrev.02_03/

Full webrev:
http://cr.openjdk.java.net/~eosterlund/8182703/webrev.03/

Thanks for reviewing, and hope the new comments are satisfactory.

/Erik

On 2017-07-05 10:53, Erik Österlund wrote:
> Hi Thomas,
>
> Thanks for the review.
>
> On 2017-07-04 19:43, Thomas Schatzl wrote:
>> Hi,
>>
>> On Mon, 2017-06-26 at 15:34 +0200, Erik Österlund wrote:
>>> Hi,
>>>
>>> Webrev: http://cr.openjdk.java.net/~eosterlund/8182703/webrev.02/
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8182703
>>>
>>    looks good apart from the comment at Monitor::event_types. It now
>> contradicts itself from one sentence to the next ("special must be
>> lowest" and then "oh no, after all access must be lowest"). Please try
>> to find some better wording here :)
>
> Agreed. Will fix and send out new webrev after I receive a reply to my 
> reply to your other email. That turned into a more complicated 
> sentence than I anticipated.
>
> Thanks,
> /Erik
>
>>> The G1 barrier queues have very awkward lock orderings for the
>>> following reasons:
>>>
>> [...]
>>> I do recognize that long term, we *might* want a lock-free solution
>>> or something (not saying we do or do not). But until then, the ranks
>>> ought to be corrected so that they do not cause these problems
>>> causing everyone to bash their head against the awkward G1 lock ranks
>>> throughout the code and make hacks around it.
>>>
>>> Testing: JPRT with hotspot all and lots of local testing.
>> Thanks,
>>    Thomas
>>
>