RFR(M): 8012182: Add information about class loading and unloading to event based tracing framework (hs24) - updated

[Markus Grönlund]

First,

I forgot to mention in the original post that parts of this code was contributed-by:

Calvin Cheung (calvin.cheung at oracle.com) 

Calvin will be attributed to this fact in a Contributed-by: when this goes back.


Now,

"I still find it odd that you post the event before the loader constraint checks are done. Not normally an issue of course. And conversely it seems odd that JVMTI only posts an event if we needed to do the loader constraint checking ???"

Yes, this is not so easy to follow (I haven't gone though it in excruciating detils that is) - there is another JvmtiExport::should_post_class_load() in SystemDictionary::define_instance_class as well...

With now having the TracingTime type available unconditionally, it open up some more flexibility to match the tracing code with JvmtiExport::should_post_class_load(), I will see if I can come up with something that allows for tighter pairing with JVMTI.


"Following on to Karen's comment, so you rely on the ResourceMark being in the "caller" (which is the generated code in this case) ? Should that be documented in the file?"

Absolutely yes. I will add in comments about the assumptions in TraceStream.hpp. In addition I will fix up the reference variables to be const references as well. Thanks.


Thanks
Markus



-----Original Message-----
From: David Holmes 
Sent: den 17 april 2013 03:46
To: Markus Grönlund
Cc: hotspot-runtime-dev at openjdk.java.net; serviceability-dev at openjdk.java.net; hotspot-gc-dev at openjdk.java.net
Subject: Re: RFR(M): 8012182: Add information about class loading and unloading to event based tracing framework (hs24) - updated

Hi Markus,

On 17/04/2013 7:35 AM, Markus Grönlund wrote:
> Hi again,
>
> The changeset for this has been updated slightly to reflect underlying 
> changes in hs24.
>
> Still looking for reviews for this change to add information about 
> class loading and unloading to the event based tracing framework for HS24.
>
> BugID:
>
> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8012182
>
> Webrev (updated):
>
> http://cr.openjdk.java.net/~mgronlun/8012182/webrev02/

systemDictionary.cpp:

Minor nit: this comment, now it is split from earlier comment

801         // class_loader is NOT the defining loader, do a little more 
bookkeeping.

should say

801         // If class_loader is NOT the defining loader, do a little 
more bookkeeping.

I still find it odd that you post the event before the loader constraint checks are done. Not normally an issue of course. And conversely it seems odd that JVMTI only posts an event if we needed to do the loader constraint checking ???

---

traceStream.hpp

Following on to Karen's comment, so you rely on the ResourceMark being in the "caller" (which is the generated code in this case) ? Should that be documented in the file?

David
-----

> Thanks in advance
>
> Markus
>
> *From:*Markus Grönlund
> *Sent:* den 15 april 2013 10:17
> *To:* hotspot-runtime-dev at openjdk.java.net;
> serviceability-dev at openjdk.java.net
> *Subject:* RFR(M): 8012182: Add information about class loading and 
> unloading to event based tracing framework (hs24)
>
> Greetings,
>
> Kindly asking for reviews for the change to add class load and unload 
> information to the event based tracing framework to HS24.
>
> BugID:
>
> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8012182
>
> Webrev:
>
> http://cr.openjdk.java.net/~mgronlun/8012182/webrev01/
>
> Thanks
>
> Markus
>