[15]: RFR(S): 8241718: assert ((klass)->trace_id()) & ((JfrTraceIdEpoch::method_and_class_in_use_this_epoch_bits()))) != 0 in ObjectSampleCheckpoint::add_to_leakp_set

[Markus Gronlund]

Thank you David for taking a look.

"I'm wondering though how this arises in practice. I can see this could be an issue with dynamic archiving as one might expect JFR to be active when running the app that will do the dynamic archiving. But for a static archive I would (naively perhaps) expect dumping to disable JFR."

Yes, I am a bit unclear on this as well - maybe we can get some clarification from the CDS/AppCDS folks?

"Regardless, incidental state changes due to VM configuration at dump time should not be stored in the archive, so I hope there is nothing else that we may be storing of a similar nature."

I was not aware that Method*'s are treated like this too; I knew from before that there is a Klass::remove_unshareable_info(), Ioi and I worked with it many years ago letting it mask off certain bits for the Klass*. From what I can see, there is also a ConstantPool::remove_unsharable_info(), but for JFR we don't trace ConstantPools.

If, say, CLDs are incorporated into the CDS archive, we would need to put in place a similar remove_unsharable_info() for those artifacts as well.

Thank you for the review
Markus

-----Original Message-----
From: David Holmes 
Sent: den 22 juni 2020 03:23
To: Markus Gronlund <markus.gronlund at oracle.com>; hotspot-runtime-dev at openjdk.java.net
Subject: Re: [15]: RFR(S): 8241718: assert ((klass)->trace_id()) & ((JfrTraceIdEpoch::method_and_class_in_use_this_epoch_bits()))) != 0 in ObjectSampleCheckpoint::add_to_leakp_set

Hi Markus,

On 22/06/2020 3:44 am, Markus Gronlund wrote:
> Greetings,
> 
> Kindly asking for review for the following change set to fix an interoperability issue between JFR and CDS.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8241718
> Webrev: http://cr.openjdk.java.net/~mgronlun/8241718/webrev00/
> Testing: jdk_jfr, runtime/cds
> 
> Summary:
> General: Method* trace flags should not be serialized to the CDS archive and must therefore be removed in Method::remove_unsharable_info().
> JFR specific: CLEAR_METHOD_LEAKP(method) removes the leakp flag on first method write to have it serialize only once.

The fix seems reasonable given the description of the problem.

I'm wondering though how this arises in practice. I can see this could be an issue with dynamic archiving as one might expect JFR to be active when running the app that will do the dynamic archiving. But for a static archive I would (naively perhaps) expect dumping to disable JFR. 
Regardless, incidental state changes due to VM configuration at dump time should not be stored in the archive, so I hope there is nothing else that we may be storing of a similar nature.

Thanks,
David
-----

> Thanks
> Markus
> 
> 
> 
>