[13] RFR(XS): 8225706: JFR RootResolver resets CLD claims with no restore

[Erik Gahlin]

Looks good

Erik

> On 26 Jun 2019, at 23:55, Markus Gronlund <markus.gronlund at oracle.com> wrote:
> 
> Hi Zhengyu,
> 
> Thanks for taking a look and for the valuable suggestion.
> 
> Iterating over the CLDG passing _claim_none in the closure will bypass interfering with existing claims (and yes, the code is currently single-threaded here). 
> 
> This means we can actually avoid the whole save/restore operation altogether. At least as long as we don't attempt to traverse over metadata (tbd), which would need save/restore and setting claims.
> 
> I think we can simplify this quite a bit in following your suggestion:
> 
> Webrev02: http://cr.openjdk.java.net/~mgronlun/8225706/webrev02/ 
> 
> Thanks again Zhengyu
> 
> Markus
> 
> -----Original Message-----
> From: Zhengyu Gu <zgu at redhat.com> 
> Sent: den 26 juni 2019 14:09
> To: Markus Gronlund <markus.gronlund at oracle.com>; hotspot-jfr-dev at openjdk.java.net; hotspot-runtime-dev at openjdk.java.net
> Subject: Re: [13] RFR(XS): 8225706: JFR RootResolver resets CLD claims with no restore
> 
> Hi Markus,
> 
> Looks like RootSetClosure::process_roots() is single-threaded. Could you just not clear claimed masks for CLDG and use _claim_none instead to walk CLDG?
> 
> Thanks,
> 
> -Zhengyu
> 
> On 6/25/19 4:58 PM, Markus Gronlund wrote:
>> Greetings,
>> 
>> Kindly asking for reviews for the following changeset:
>> 
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8225706
>> Webrev: http://cr.openjdk.java.net/~mgronlun/8225706/webrev01/
>> Testing: test/jdk/:jdk_jfr
>> 
>> Summary:
>> We need to move SaveRestoreCLDClaimBits, from currently inside RootSetClosure::process_roots() back up to EmitEventOperation::doit() for proper scope.
>> RootResolver::resolve() clears out already set claim bits via ClassLoaderDataGraph::clear_claimed_marks() under the (currently false) premise that the original claims will be restored later.
>> The problematic path triggers only if the option "path-to-gc-roots=true" is set via the command-line or via jcmd AND an existing old object candidate is found.
>> 
>> Thanks to Stefan Karlsson for calling attention to this.
>> 
>> Thank you
>> Markus
>>