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

[Tony Printezis]

Ramki,

I discussed this with John, the motivation was to avoid any phantom 
mistakes in the future if for some reason we really need to take the PLL 
before we do the cleanup pause and we forget to do so. Given that any 
effect I'd guess would be marginal / non-existent (cleanup pauses are 
really not very frequent) I'd like to play it safe here.

Tony

On 10/19/2011 08: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
>>