RFR(L): 8151601: Cleanup locking of the Reference pending list

[Per Liden]

On 2016-03-15 12:49, Stefan Karlsson wrote:
> Hi Per,
>
> On 15/03/16 12:02, Per Liden wrote:
>> Hi,
>>
>> I got some minor comments from StefanK offline, here's an updated webrev:
>>
>> Incremental: http://cr.openjdk.java.net/~pliden/8151601/webrev.diff-0vs1/
>>
>> Full: http://cr.openjdk.java.net/~pliden/8151601/webrev.1/
>
> Reviewed.

Thanks.

>
> I reviewed this offline with Per, and I questioned the circular
> dependency between the ReferencePendingListLocker and the
> ReferencePendingListLockerThread. Since, at least to me, that part makes
> the code slightly non-obvious. I've created a small PoC patch:
> http://cr.openjdk.java.net/~stefank/8151601/webrev.non-circular.00.delta/
> http://cr.openjdk.java.net/~stefank/8151601/webrev.non-circular.00/
>
> In this patch, ReferencePendingListLocker is the user visible class that
> determines if we can directly lock the reference pending list lock or if
> we have to delegate to the ReferencePendingListLockerThread. The
> ReferencePendingListLockerFromJava class is used to lock the reference
> pending lock from both ReferencePendingListLocker and
> ReferencePendingListLockerThread.
>
> We were not entirely happy with the names, so we decided to defer this
> and maybe revisit this later.

Sounds good.

thanks,
Per

>
> Thanks,
> StefanK
>
>>
>> thanks,
>> Per
>>
>> On 2016-03-10 16:53, Per Liden wrote:
>>> Hi,
>>>
>>> The SurrogateLockerThread is currently used by both G1 and CMS to lock
>>> the Reference pending list. However, there's quite a bit of unnecessary
>>> code duplication and special case handling in this area. This patch is
>>> an attempt to clean this up.
>>>
>>> The main changes:
>>>
>>> - All locking of the pending list is how done through the new
>>> ReferencePendingListLocker. This locker delegates to a locker thread if
>>> needed.
>>>
>>> - The locking related code in instanceRefKlass was moved to the new
>>> ReferencePendingListLocker.
>>>
>>> - What used to be the SurrogateLockerThread is now the
>>> ReferencePendingListLockerThread.
>>>
>>> - The new virtual function
>>> CollectedHeap::needs_reference_pending_list_locker_thread() indicates if
>>> a collector needs a locker thread. G1 and CMS returns true here.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8151601
>>> Webrev: http://cr.openjdk.java.net/~pliden/8151601/webrev.0/
>>> Testing: jprt, vm.gc
>>>
>>> cheers,
>>> Per
>