RFR: 8237143: Eliminate DirtyCardQ_cbl_mon
kim.barrett at oracle.com
Fri Feb 7 00:00:22 UTC 2020
> On Feb 4, 2020, at 9:39 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> 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.
>>>>> 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:
> I think this is good. Thanks for your extensive changes.
> Two minor issues. Do not need re-review:
> * s/unsufficient/insufficient in g1DirtyCardQueue.cpp
Thanks for spotting that.
> * simple predicates returning bool tend to have an "is_" or "has_" prepended to it, i.e. s/PausedBuffers::empty()/...::is_empty()/
Agreed; will change to is_empty. Old habits seem to die hard; I think someday
we might want to be more consistent with the Standard Library, but not today.
More information about the hotspot-gc-dev