RFR(S): 7099824: G1: we should take the pending list lock before doing the remark pause

[Ramki Ramakrishna]

Definitely cleaner!

On 10/19/2011 6:29 PM, Dean Long wrote:
> How about leaving CGC alone and adding a subclass that does the extra 
> PLL locking?
>
> dl
>
> On 10/19/2011 05:23 PM, Ramki Ramakrishna wrote:
>> Hi John, I don't think it would make any performance (latency) 
>> difference (given how
>> the reference handler thread takes the lock today). It just seemed
>> unnecessary, that's all.... OK to take it for all CGC ops, if it helps
>> keep the code from being messy... your subjective choice (anyone
>> else have an opinion on code maintainability/robustness here because
>> of the extra option/attribute in the CGC op c'tor?)
>>
>> -- ramki
>>
>> On 10/19/2011 5:09 PM, John Cuthbertson wrote:
>>> Hi Ramki,
>>>
>>> Thanks for the review. Do you think that waiting for the pending 
>>> list lock will significantly delay cleanups? If so then adding an 
>>> extra "needs_pll" flag to VM_CGC_Operation is an easy change.
>>>
>>> Thanks,
>>>
>>> JohnC
>>>
>>> On 10/19/11 16:50, Ramki Ramakrishna wrote:
>>>> Hi John -- This looks good to me, although I didn't see any obvious 
>>>> reason to make clean up pauses take the
>>>> PLL lock given they do not mess with the pending list in any 
>>>> manner. Otherwise looks good to me.
>>>>
>>>> -- ramki
>>>>
>>>> On 10/17/2011 2:36 PM, John Cuthbertson wrote:
>>>>> Hi Everyone,
>>>>>
>>>>> Can I have a couple of volunteers review these changes? The webrev 
>>>>> can be found at: http://cr.openjdk.java.net/~johnc/7099824/wevrev.0/
>>>>>
>>>>> Summary: During a G1 remark pause, the JVM may enqueue some 
>>>>> discovered references on to the pending list. Whenever the JVM 
>>>>> modifies the pending list, it is supposed to synchronize with the 
>>>>> ReferenceHandler thread and acquire the pending list lock but G1's 
>>>>> concurrent GC operations were not doing so.
>>>>>
>>>>> Testing: the GC test suite with both CMS and G1, and jprt.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> JohnC
>>>