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

[Bengt Rutisson]

Hi John

Looks good to me.

A couple of minor comments:

g1CollectedHeap.cpp:

In the do_oop_work method (lines 4324-4362):

* To show the limited scope of the should_mark variable I would like to 
move the declaration of should_mark to just before it is being used at 
line 4349.

* At the two places in the do_oop_work method where you use the 
do_mark_forwardee paramter you have comments saying that it is an 
initial mark closure. This is correct, so I think that I would actually 
prefer that the parameter was called something with inital mark rather 
than having to have the comments.

* On the dead code subject: this closure seems to be unused (in 
g1CollectedHeap.cpp): "G1ParScanAndMarkHeapRSClosure   
scan_mark_heap_rs_cl(_g1h, &pss);"

g1OopClosures.hpp:

Just a nitpick: You removed a line break at row 114 and added a line 
break at row 122-123. Since you didn't change anything else on these 
lines it would make the diff easier to view if you left those changes out.

Bengt

On 2011-08-18 20: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
>
>