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

[Ioi Lam]

On 6/22/20 8:32 AM, Calvin Cheung wrote:
> 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


I think we just disable part of JFR. If you run 
test/hotspot/jtreg/runtime/cds/appcds/CDSandJFR.java, this comes out of 
STDERR


Java HotSpot(TM) 64-Bit Server VM warning: JFR will be disabled during 
CDS dumping


But STDOUT also says:

0.210s][warning][cds] Skipping jdk/internal/event/Event: Unsupported 
location
[0.210s][warning][cds] Skipping jdk/internal/event/Event: JFR event class
[0.210s][warning][cds] Skipping jdk/jfr/Event: Unsupported location
[0.210s][warning][cds] Skipping jdk/jfr/Event: JFR event class
[0.211s][warning][cds] Skipping GetFlightRecorder$TestEvent: Unsupported 
location
[0.211s][warning][cds] Skipping GetFlightRecorder$TestEvent: JFR event class
[0.211s][warning][cds] Skipping jdk/jfr/events/ActiveRecordingEvent: 
Unsupported location
[0.211s][warning][cds] Skipping jdk/jfr/events/ActiveRecordingEvent: JFR 
event class

We currently have JDK-8222945 -- Support JFR during CDS dumping. We need 
to decide what  to do -- whether we should completely disable JFR during 
CDS dumps, or completely support it, but undo the side effects cleanly.

Thanks
- Ioi



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