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

[Calvin Cheung]

Hi Markus,

On 6/22/20 6:04 AM, Markus Gronlund wrote:
> 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?

Currently, JFR will be disabled during CDS dumping, both static and 
dynamic. There's a is_cds_dump_requested function in the following:

http://hg.openjdk.java.net/jdk/jdk/file/8d722fb14093/src/hotspot/share/jfr/recorder/jfrRecorder.cpp#l174

thanks,

Calvin

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