RFR(S): 7092412: G1: Some roots not marked during an initial mark that gets an evacuation failure

[Bengt Rutisson]

Hi John,

Looks good to me. Splitting up the should_mark information into 
should_mark_root and should_mark_copy made the change much cleaner than 
when we went over this before.

One minor thing. ConcurrentMark::grayRoot() is now called in two places. 
In both places it is preceded by a test to verify that the address is in 
the G1 heap:

HeapWord* addr = (HeapWord*)obj;
if (_g1->is_in_g1_reserved(addr)) {
       _cm->grayRoot(oop(addr));
}

What do you think about moving this test into the grayRoot() method?

Bengt



On 2011-09-19 22:08, John Cuthbertson wrote:
> Hi Everyone,
>
> Can I have a couple of reviews of the fix for this CR? The webrev can 
> be found at: http://cr.openjdk.java.net/~johnc/7092412/webrev.0/
>
> Summary: As a result of the changes for 7080389, some of the roots may 
> not be marked during the initial mark pause in the event of an 
> evacuation failure. If the closure that successfully forwards an 
> object to itself is a root scanning closure and there are no other 
> paths to the object referenced from the root location then the object 
> may not be marked. This problem was discovered as a result of enabling 
> marking verification while investigating another CR (7086533). The fix 
> is to pass a flag into the evacuation failure code as to whether the 
> caller is a root scanning closure and, if that is the case, the thread 
> that successfully forwards an object to itself is then responsible for 
> marking that object.
>
> Testing: selected test cases for 7086533 with +VerifyDuringGC; the GC 
> test suite with a high marking threshold and +VerifyDuringGC
>
> Thanks,
>
> JohnC