RFR (S): 7080389: G1: refactor marking code in evacuation pause copy closures

[Bengt Rutisson]

Looks good to me.

Bengt


On 2011-08-25 18:57, John Cuthbertson wrote:
> Hi Everyone,
>
> I'm opening up a new version of these changes for code review that 
> includes a fix for the issue that was causing the failure I saw below 
> (many thanks to Tony for helping to diagnose the issue). The new 
> webrev can be found at: 
> http://cr.openjdk.java.net/~johnc/7080389/webrev.2/
>
> Testing: around 30 iterations of the GC test suite with both an 
> instrumented and non instrumented product VM ,with a marking threshold 
> of 20%, and with marking verification and VerifyBeforeGC enabled (this 
> combo should the problem the most frequently - about 1/50 runs of xalan).
>
> Thanks,
>
> JohnC
>
> On 08/22/11 15:12, John Cuthbertson wrote:
>> Hi Bengt, Tony, Stefan,
>>
>> Thanks again for the review comments. I have to hold off pushing this 
>> as I have seen a failure while doing a sanity check after merging the 
>> workspace. So until I get to the bottom of the failure - consider 
>> this as "on hold".
>>
>> <sigh>
>>
>> JohnC
>>
>> On 08/21/11 14:03, Bengt Rutisson wrote:
>>>
>>> Hi John,
>>>
>>> This new webrev looks good to me.
>>>
>>> The discussion that Stefan and Tony had was good. I like the new 
>>> template parameter name (do_mark_object) and the new comments much 
>>> better. In fact, I realize my initial comments regarding those were 
>>> a bit wrong since I had misinterpreted the code due to the old 
>>> comments. Thanks for finding a good solution to this.
>>>
>>> Ship it!
>>> Bengt
>>>
>>> On 2011-08-20 01:12, John Cuthbertson wrote:
>>>> Hi Everyone,
>>>>
>>>> Hopefully this webrev 
>>>> (http://cr.openjdk.java.net/~johnc/7080389/webrev.1/) addresses 
>>>> everyones' comments.
>>>>
>>>> Thanks,
>>>>
>>>> JohnC
>>>>
>>>> On 08/18/11 11:17, John Cuthbertson wrote:
>>>>> Hi Everyone,
>>>>>
>>>>> Can I have a couple of volunteers review these refactoring changes 
>>>>> to the marking code used during evacuation pauses (both initial 
>>>>> mark pauses and regular evacuation pauses when marking is active) 
>>>>> - the change can be found at 
>>>>> http://cr.openjdk.java.net/~johnc/7080389/webrev.0/.
>>>>>
>>>>> The refactoring changes fix an issue that was seen with the code 
>>>>> changes for 6486945.
>>>>>
>>>>> During an initial mark pause, during root scanning, one thread had 
>>>>> successfully forwarded an object and had started to copy it. While 
>>>>> the object was being copied to its new location, another thread 
>>>>> saw that the object had been forwarded and, after checking that 
>>>>> the new location was unmarked, successfully marked the new 
>>>>> location. The first thread would finish the copying, see that the 
>>>>> new location was marked and skip the mark. The situation I ran 
>>>>> into was that I was attempting to obtain the size of the new 
>>>>> object just after it was marked (by the thread doing the marking) 
>>>>> and the old object had not yet been fully copied to its new location.
>>>>>
>>>>> With these refactoring changes, the thread that successfully 
>>>>> forwards an object in the collection set will mark the forwardee 
>>>>> after copying - allowing me to safely obtain it's size.
>>>>>
>>>>> Testing: several runs of the GC test suite with a marking 
>>>>> threshold of 10 and 20%, Kitchensink, and jprt.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> JohnC
>>>>>
>>>>>
>>>>
>>>
>>
>