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

[Zhengyu Gu]

On 6/26/19 5:55 PM, Markus Gronlund wrote:
> Hi Zhengyu,
> 
> Thanks for taking a look and for the valuable suggestion.

You are welcome.

I recently encountered similar problem when Shenandoah moved CLDG 
evacuation into concurrent phase, Shenandoah's heap iteration code 
(single-threaded marking from roots) started interfering with concurrent 
CLDG iteration.

> 
> 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/

Looks good to me.

Thanks,

-Zhengyu

> 
> 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
>>