RFR: 8237143: Eliminate DirtyCardQ_cbl_mon

[Thomas Schatzl]

Hi Kim,

On 31.01.20 23:25, Kim Barrett wrote:
>> On Jan 23, 2020, at 3:10 PM, Kim Barrett <kim.barrett at oracle.com> wrote:
>>
>>> On Jan 22, 2020, at 11:12 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
>>> On 16.01.20 09:51, Kim Barrett wrote:
>>>> Please review this change to eliminate the DirtyCardQ_cbl_mon.  This
>>>> is one of the two remaining super-special "access" ranked mutexes.
>>>> (The other is the Shared_DirtyCardQ_lock, whose elimination is covered
>>>> by JDK-8221360.)
>>>> There are three main parts to this change.
>>>> (1) Replace the under-a-lock FIFO queue in G1DirtyCardQueueSet with a
>>>> lock-free FIFO queue.
>>>> (2) Replace the use of a HotSpot monitor for signaling activation of
>>>> concurrent refinement threads with a semaphore-based solution.
>>>> (3) Handle pausing of buffer refinement in the middle of a buffer in
>>>> order to handle a pending safepoint request.  This can no longer just
>>>> push the partially processed buffer back onto the queue, due to ABA
>>>> problems now that the buffer is lock-free.
>>>> CR:
>>>> https://bugs.openjdk.java.net/browse/JDK-8237143
>>>> Webrev:
>>>> https://cr.openjdk.java.net/~kbarrett/8237143/open.00/
>>>> Testing:
>>>> mach5 tier1-5
>>>> Normal performance testing showed no significant change.
>>>> specjbb2015 on a very big machine showed a 3.5% average critical-jOPS
>>>> improvement, though not statistically significant; removing contention
>>>> for that lock by many hardware threads may be a little bit noticeable.
>>>
>>> initial comments only, and so far only about comments :( The code itself looks good to me, but I want to look over it again.
>>
>> After some offline discussion with Thomas, I’m doing some restructuring that
>> makes it probably not very efficient for anyone else to do a careful review of
>> the open.00 version.
> 
> Here's a new webrev:
> 
> https://cr.openjdk.java.net/~kbarrett/8237143/open.02/
> 

   I think this is good. Thanks for your extensive changes.

Two minor issues. Do not need re-review:

* s/unsufficient/insufficient in g1DirtyCardQueue.cpp
* simple predicates returning bool tend to have an "is_" or "has_" 
prepended to it, i.e. s/PausedBuffers::empty()/...::is_empty()/

Thanks,
   Thomas