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

[Poonam Bajaj Parhar]

Hello Mikael,

Thanks for your feedback.

Here's the updated webrev:
http://cr.openjdk.java.net/~poonam/8145442/webrev.01/

On 12/21/2015 6:11 AM, Mikael Gerdin wrote:
> Hi Poonam,
>
> On 2015-12-15 22:32, 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/
>
> I have some general comments on the structure of the code:
>
> First of all, I would prefer if we could avoid adding yet another 
> command line flag.
> I think it would be preferable to either use VerifyRememberedSets 
> which already exists or to use the code you've proposed adding under 
> 8072725 with a "remset" verification subset (and deprecating 
> VerifyRememberedSets).
>
I removed the new option and am using VerifyRememberedSets now. After I 
have integrated the changes for 8072725, we can add 'remset' to the 
VerifySubSet and can control the rem-set verification of all the 
collectors under that.

> Having RSet verification only post-gc and not before does not appeal 
> to me, since it's quite possible that we can have missing write 
> barriers in compiled code leading to missing remembered set entries as 
> well.
>
I have made changes to verify rem-sets both before and after GC.

> Folding this into the VerifyBefore/After code would allow users to select
> -XX:+VerifyAfterGC -XX:VerifySubSet=remset
> to get the behavior you are suggesting in this webrev.
>
>
> I like Jon's suggestion of moving all the remset verification to a 
> VerifyRemSetClosure which can be aggregated in the VerifyLiveClosure 
> if full verification is enabled.
>
I moved the rem-set verification code to VerifyRSetClosure and derived 
VerifyLiveClosure from that.

> So, given that I've asked for re-use of the VerifyBefore/After code I 
> would suggest some changes in
> HeapRegion::verify(VerifyOption vo, bool* failures)
> to make the code select either the VerifyLiveClosure or the 
> VerifyRemSetClosure depending on the level of verification.
>

Yes, sure we can add more VerifySubSet levels after I have integrated 
the changes for 8072725.


Thanks,
Poonam

> The BOT verification near the end of ::verify could be conditionalized 
> on VerifySubSet=bot or something similar.
>
> /Mikael
>
>> Testing: JPRT, GCBasher
>>
>> Thanks,
>> Poonam
>>
>