Thanks David and Sergeui, I have added (back) the INCLUDE_TRACE for the load code.
Webrev version 5: http://cr.openjdk.java.net/~mgronlun/8012182/webrev05/ BugID: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8012182 Thanks Markus -----Original Message----- From: David Holmes Sent: den 18 april 2013 04:42 To: Markus Grönlund Cc: Karen Kinnear; serviceability-dev@openjdk.java.net; hotspot-gc-...@openjdk.java.net; hotspot-runtime-...@openjdk.java.net Subject: Re: RFR(M): 8012182: Add information about class loading and unloading to event based tracing framework (hs24) - updated On 18/04/2013 3:39 AM, Markus Grönlund wrote: > Hi again, > > Seems it wasn't really as straightforward as originally suggesting with > collocating the event tracing and the JVMTI code - especially if I attempt to > breakout the JVMTI notification code from > SystemDictionary::define_instance_class(); this seems to have detrimental > effects on the UTE JVMTI tests...so I am leaving all JVMTI code as is... > > Event notification now posted after check_constraints and exception checks. Okay that seems fine. I just have the same query as Serguei as to why the unload code is conditional on INCLUDE_TRACE but the load code isnt ?? Thanks, David > Webrev version 4: > http://cr.openjdk.java.net/~mgronlun/8012182/webrev04/ > > > BugID: > http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8012182 > > Thanks again > Markus > > > -----Original Message----- > From: Markus Grönlund > Sent: den 17 april 2013 12:41 > To: David Holmes; Karen Kinnear > Cc: serviceability-dev@openjdk.java.net; > hotspot-gc-...@openjdk.java.net; hotspot-runtime-...@openjdk.java.net > Subject: RE: RFR(M): 8012182: Add information about class loading and > unloading to event based tracing framework (hs24) - updated > > Hi again, > > Updated webrev (v3) trying to respect the loader constraint checking before > event posting - in addition trying to consolidate the event notification code > (instrumentation in SystemDictionary::define_instance_class() seems to > indicate the ok'ness of moving the JVMTI::should_post call from out of > there)...but I am sure Karen will put me right if this ok for real. > > Also updated the patch to include comments + contributions. > > Webrev version 3: > http://cr.openjdk.java.net/~mgronlun/8012182/webrev03/ > > BugID: > http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8012182 > > Many thanks for your comments. > > Cheers > Markus > > > > -----Original Message----- > From: Markus Grönlund > Sent: den 17 april 2013 09:48 > To: David Holmes > Cc: serviceability-dev@openjdk.java.net; > hotspot-gc-...@openjdk.java.net; hotspot-runtime-...@openjdk.java.net > Subject: RE: RFR(M): 8012182: Add information about class loading and > unloading to event based tracing framework (hs24) - updated > > First, > > I forgot to mention in the original post that parts of this code was > contributed-by: > > Calvin Cheung (calvin.che...@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-...@openjdk.java.net; > serviceability-dev@openjdk.java.net; hotspot-gc-...@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-...@openjdk.java.net; >> serviceability-dev@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 >>
