RFR: JDK-8145442: Add an option to verify remembered sets for G1

[Jon Masamitsu]

Poonam,

Generally looks good.

http://cr.openjdk.java.net/~poonam/8145442/webrev.00/src/share/vm/gc/g1/g1CollectedHeap.cpp.frames.html

The new GC unified logging implementation has been pushed so you'll
need to replace of gclog_or_tty().

> 4060 if (G1VerifyRSetsAfterEvacuationPause) {
> 4061 gclog_or_tty->print("[Verifying RSets]");
> 4062 VerifyRegionRSetClosure v_cl;
> 4063 heap_region_iterate(&v_cl);
> 4064 }

Probably with

   log_debug(gc, verify)("[Verifying RSets]");


http://cr.openjdk.java.net/~poonam/8145442/webrev.00/src/share/vm/gc/g1/heapRegion.hpp.frames.html

> 755 void verifyRSet(VerifyOption vo, bool *failures) const;

Any comment worth adding for verifyRSet()?  Are there conditions on 
where and
when it can be used?

In verifyRSet() you always use VerifyOption_G1UsePrevMarking.  Will that 
always
be the value for the VerifyOption?  If it is always going to be that, 
does it work
to remove  the VerifyOption parameter from verifyRSet() and has verifyRSet()
always pass VerifyOption_G1UsePrevMarking to VerifyRSetClosure?

http://cr.openjdk.java.net/~poonam/8145442/webrev.00/src/share/vm/gc/g1/heapRegion.cpp.frames.html

917 VerifyRSetClosure v_rset_cl(g1, VerifyOption_G1UsePrevMarking);

I can see HeapRegionVerifyClosure::verifyRemSets() is used and it works out
well but it seems odd for HeapRegionVerifyClosure (which has a name
that I read as generally useful with different Verify closures) to have
a method that is specific to remembered-set verification.

I think it is like this.

HeapRegionVerifyClosure had a verifyRemSets() method that is
called from do_oop in VerifyRSetClosure and VerifyLiveClosure.

Then when you want to do remembered-set verification you
use a VerifyRSetClosure and when you want to verify liveness,
you use a VerifyLiveClosure and get the remembered-set
verfication included in the VerifyLiveClosure.

An alternative would be to put verifyRemSets() into VerifyRSetClosure
and have a VerifyRSetClosure as part of the VerifyLiveClosure. Then
instead of calling verifyRemSets() (as inherited from 
HeapRegionVerifyClosure)
in VerifyLiveClosure, you could call verifyRemSets() using the embedded
VerifyRSetClosure.

Or you could collapse HeapRegionVerifyClosure and VerifyRSetClosure
together (give it the name VerifyRSetClosure so that having a 
verifyRemSets()
seems more natural) and derive VerifyLiveClosure from the new
VerifyRSetClosure.

Jon

On 12/15/2015 1:32 PM, Poonam Bajaj Parhar wrote:
> Hello,
>
> Please review these changes that add a diagnostic VM option 
> 'G1VerifyRSetsAfterEvacuationPause' that when turned on enables the 
> verification of Rem Sets of heap regions.
>
> Bug: JDK-8145442: Add an option to verify remembered sets for G1
> Webrev: http://cr.openjdk.java.net/~poonam/8145442/webrev.00/
> Testing: JPRT, GCBasher
>
> Thanks,
> Poonam
>