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

[Stefan Karlsson]

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.

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.

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