-
Enhancement
-
Resolution: Fixed
-
P4
-
11
-
b09
Issue | Fix Version | Assignee | Priority | Status | Resolution | Resolved In Build |
---|---|---|---|---|---|---|
JDK-8240391 | openjdk8u262 | Harold Seigel | P4 | Resolved | Fixed | team |
Mail conversation during Flight Recorder review JDK-8199712:
On 5/3/18 10:34 AM, Markus Gronlund wrote:
> Hi Coleen,
>
> thanks for taking a look.
>
> We have filed https://bugs.openjdk.java.net/browse/JDK-8202578 to follow up on the placement of where to post the class unload events.
>
> About that, it might be possible to at least move the actual post_class_unload_event() function to InstanceKlass::notify_unload_class() as you suggest, with the proviso that we insert the "unloading hook", Jfr::on_unloading_classes(), just after the classes_do(InstanceKlass::notify_unload_class). Another problem, maybe minor, with moving the posting of class unload events to InstanceKlass::notify_unload_class() is how to maintain the same timestamp for each individual unload event.
>
> The posting for unloads is "two-stage" in that the first pass fires an unload event (instant, same timestamp) for each klass; the second stage enters the JFR framework via on_unloading_classes() to save all constants for these classes now about to disappear. I think we could do that though, something like this:
>
> …
> // Tell serviceability tools these classes are unloading
> classes_do(InstanceKlass::notify_unload_class);
>
> JFR_ONLY(Jfr::on_unloading_classes();)
> ...
>
> About also getting post_class_load_event() and
> post_class_define_event() to InstanceKlass, this will be more
> problematic. The class load event is a duration event, in that it
> measures the actual time it took to load the class (it is stack
> allocated inside SystemDictionary::resolve_instance_class_or_null()).
> Therefore it might not be possible to move it. Maybe for
> define_event() as it is only an instant event.\
Yes, thank you for filing the RFE to follow up about this. Can you cut/paste this into the description?
>
> We can sort out the intricacies involved in the context of https://bugs.openjdk.java.net/browse/JDK-8202578 .
>
>
> -----Original Message-----
> From: Coleen Phillimore
> Sent: den 26 april 2018 19:59
> To: hotspot-dev@openjdk.java.net
> Subject: Re: RFR(XL): 8199712: Flight Recorder
>
> http://cr.openjdk.java.net/~egahlin/8199712.0/src/hotspot/share/classf
> ile/classLoaderData.cpp.udiff.html
>
> We can file another RFE for this but I think you could call
> post_class_unload_event() from in InstanceKlass and call from inside of InstanceKlass::notify_unload_class.
>
> void ClassLoaderData::unload() {
> _unloading = true;
>
> // Tell serviceability tools these classes are unloading
> classes_do(InstanceKlass::notify_unload_class);
>
> Rather than walking through _klasses again during unloading. I think we should see if this is possible to improve this after this checkin.
>
> http://cr.openjdk.java.net/~egahlin/8199712.0/src/hotspot/share/classfile/systemDictionary.cpp.udiff.html
>
> Then move post_class_load and post_class_define events to instanceKlass.cpp too.
On 5/3/18 10:34 AM, Markus Gronlund wrote:
> Hi Coleen,
>
> thanks for taking a look.
>
> We have filed https://bugs.openjdk.java.net/browse/JDK-8202578 to follow up on the placement of where to post the class unload events.
>
> About that, it might be possible to at least move the actual post_class_unload_event() function to InstanceKlass::notify_unload_class() as you suggest, with the proviso that we insert the "unloading hook", Jfr::on_unloading_classes(), just after the classes_do(InstanceKlass::notify_unload_class). Another problem, maybe minor, with moving the posting of class unload events to InstanceKlass::notify_unload_class() is how to maintain the same timestamp for each individual unload event.
>
> The posting for unloads is "two-stage" in that the first pass fires an unload event (instant, same timestamp) for each klass; the second stage enters the JFR framework via on_unloading_classes() to save all constants for these classes now about to disappear. I think we could do that though, something like this:
>
> …
> // Tell serviceability tools these classes are unloading
> classes_do(InstanceKlass::notify_unload_class);
>
> JFR_ONLY(Jfr::on_unloading_classes();)
> ...
>
> About also getting post_class_load_event() and
> post_class_define_event() to InstanceKlass, this will be more
> problematic. The class load event is a duration event, in that it
> measures the actual time it took to load the class (it is stack
> allocated inside SystemDictionary::resolve_instance_class_or_null()).
> Therefore it might not be possible to move it. Maybe for
> define_event() as it is only an instant event.\
Yes, thank you for filing the RFE to follow up about this. Can you cut/paste this into the description?
>
> We can sort out the intricacies involved in the context of https://bugs.openjdk.java.net/browse/JDK-8202578 .
>
>
> -----Original Message-----
> From: Coleen Phillimore
> Sent: den 26 april 2018 19:59
> To: hotspot-dev@openjdk.java.net
> Subject: Re: RFR(XL): 8199712: Flight Recorder
>
> http://cr.openjdk.java.net/~egahlin/8199712.0/src/hotspot/share/classf
> ile/classLoaderData.cpp.udiff.html
>
> We can file another RFE for this but I think you could call
> post_class_unload_event() from in InstanceKlass and call from inside of InstanceKlass::notify_unload_class.
>
> void ClassLoaderData::unload() {
> _unloading = true;
>
> // Tell serviceability tools these classes are unloading
> classes_do(InstanceKlass::notify_unload_class);
>
> Rather than walking through _klasses again during unloading. I think we should see if this is possible to improve this after this checkin.
>
> http://cr.openjdk.java.net/~egahlin/8199712.0/src/hotspot/share/classfile/systemDictionary.cpp.udiff.html
>
> Then move post_class_load and post_class_define events to instanceKlass.cpp too.
- backported by
-
JDK-8240391 Revisit location for class unload events
- Resolved
-
JDK-8243902 Revisit location for class unload events
- Resolved
- relates to
-
JDK-8202669 Intermittent crash in ClassLoadingService::compute_class_size()
- Closed
-
JDK-8239140 Backport JFR to OpenJDK 8
- Resolved