Yunda, sorry for taking a while. I created JDK-8014478 for this and I'll sponsor the change.
/R On May 10, 2013, at 4:32 AM, 云达(Yunda) wrote: > Thanks for looking at this, Rickard. David, do you like this option which is > easier to read, too :) > > > Regards, > Yunda > >> -----Original Message----- >> From: Rickard Bäckman [mailto:rickard.back...@oracle.com] >> Sent: Thursday, May 09, 2013 4:53 PM >> To: 云达(Yunda) >> Cc: David Holmes; serviceability-dev@openjdk.java.net; >> hotspot-runtime-...@openjdk.java.net >> Subject: Re: [PATCH] EnableTracing: output from multiple threads may be mixed >> together >> >> Looks good to me! >> >> /R >> >> On May 9, 2013, at 8:38 AM, 云达(Yunda) wrote: >> >>> Rickard, >>> >>> Actually I've considered the option and I just thought it's less elegant. >>> But just >> as you said, it's clearer and easier to read. The updated and tested change: >>> >>> diff -r 627cf9e9ea31 src/share/vm/runtime/globals.hpp >>> --- a/src/share/vm/runtime/globals.hpp Mon May 06 10:29:49 2013 -0700 >>> +++ b/src/share/vm/runtime/globals.hpp Thu May 09 14:36:35 2013 +0800 >>> @@ -3634,7 +3634,10 @@ >>> "Include GC cause in GC logging") >> \ >>> >> \ >>> product(bool, EnableTracing, false, >> \ >>> - "Enable event-based tracing") >>> + "Enable event-based tracing") >> \ >>> + >> \ >>> + product(bool, UseLockedTracing, false, >> \ >>> + "Use locked-tracing when doing event-based >>> + tracing") >>> >>> /* >>> * Macros for factoring of globals >>> diff -r 627cf9e9ea31 src/share/vm/trace/traceEventClasses.xsl >>> --- a/src/share/vm/trace/traceEventClasses.xsl Mon May 06 10:29:49 >>> 2013 -0700 >>> +++ b/src/share/vm/trace/traceEventClasses.xsl Thu May 09 14:36:35 >>> +++ 2013 +0800 >>> @@ -132,6 +132,15 @@ >>> void writeEvent(void) { >>> ResourceMark rm; >>> HandleMark hm; >>> + if (UseLockedTracing) { >>> + ttyLocker lock; >>> + writeEventContent(); >>> + } else { >>> + writeEventContent(); >>> + } >>> + } >>> + >>> + void writeEventContent() { >>> TraceStream ts(*tty); >>> ts.print("<xsl:value-of select="@label"/>: ["); >>> <xsl:apply-templates select="value|structvalue" mode="write-data"/> >>> >>> >>> Regards, >>> Yunda >>> >>>> -----Original Message----- >>>> From: Rickard Bäckman [mailto:rickard.back...@oracle.com] >>>> Sent: Wednesday, May 08, 2013 3:03 PM >>>> To: David Holmes >>>> Cc: 云达(Yunda); serviceability-dev@openjdk.java.net; >>>> hotspot-runtime-...@openjdk.java.net >>>> Subject: Re: [PATCH] EnableTracing: output from multiple threads may >>>> be mixed together >>>> >>>> One option could be to move the content of writeEvent() to its own >>>> method (maybe writeEventContent()) and just do >>>> >>>> if (UseLockedTracing) { >>>> ttyLocker lock; >>>> writeEventContent(); >>>> } else { >>>> writeEventContent(); >>>> } >>>> >>>> I think it makes it easier to read. >>>> >>>> /R >>>> >>>> On May 8, 2013, at 2:41 AM, David Holmes wrote: >>>> >>>>> On 7/05/2013 10:33 PM, 云达(Yunda) wrote: >>>>>> David, >>>>>> >>>>>> Sorry about the stupid mistake. And please see the tested change below. >>>> Since I don't find an elegant way to define a ttyLocker object, I use >>>> the static methods of ttyLocker instead. >>>>> >>>>> Yeah there's no elegant solution when you need to conditionalize >>>>> things. :( >>>>> >>>>> This looks fine to me but the serviceability folk should chime back in. >>>>> >>>>> Thanks, >>>>> David >>>>> >>>>>> diff -r 627cf9e9ea31 src/share/vm/runtime/globals.hpp >>>>>> --- a/src/share/vm/runtime/globals.hpp Mon May 06 10:29:49 2013 >>>>>> -0700 >>>>>> +++ b/src/share/vm/runtime/globals.hpp Tue May 07 20:04:24 2013 >>>>>> +++ +0800 >>>>>> @@ -3634,7 +3634,10 @@ >>>>>> "Include GC cause in GC logging") >>>> \ >>>>>> >>>> \ >>>>>> product(bool, EnableTracing, false, >>>> \ >>>>>> - "Enable event-based tracing") >>>>>> + "Enable event-based tracing") >>>> \ >>>>>> + >>>> \ >>>>>> + product(bool, UseLockedTracing, false, >>>> \ >>>>>> + "Use locked-tracing when doing event-based >>>>>> + tracing") >>>>>> >>>>>> /* >>>>>> * Macros for factoring of globals diff -r 627cf9e9ea31 >>>>>> src/share/vm/trace/traceEventClasses.xsl >>>>>> --- a/src/share/vm/trace/traceEventClasses.xsl Mon May 06 10:29:49 >>>>>> 2013 -0700 >>>>>> +++ b/src/share/vm/trace/traceEventClasses.xsl Tue May 07 20:04:24 >>>>>> +++ 2013 +0800 >>>>>> @@ -132,10 +132,17 @@ >>>>>> void writeEvent(void) { >>>>>> ResourceMark rm; >>>>>> HandleMark hm; >>>>>> + int holder; >>>>>> + if (UseLockedTracing) { >>>>>> + holder = ttyLocker::hold_tty(); >>>>>> + } >>>>>> TraceStream ts(*tty); >>>>>> ts.print("<xsl:value-of select="@label"/>: ["); >>>>>> <xsl:apply-templates select="value|structvalue" mode="write-data"/> >>>>>> ts.print("]\n"); >>>>>> + if (UseLockedTracing) { >>>>>> + ttyLocker::release_tty(holder); >>>>>> + } >>>>>> } >>>>>> }; >>>>>> >>>>>> >>>>>> Regards, >>>>>> Yunda >>>>>> >>>>>> >>>>>>> -----Original Message----- >>>>>>> From: David Holmes [mailto:david.hol...@oracle.com] >>>>>>> Sent: Tuesday, May 07, 2013 5:50 PM >>>>>>> To: 云达(Yunda) >>>>>>> Cc: hotspot-runtime-...@openjdk.java.net; >>>>>>> serviceability-dev@openjdk.java.net >>>>>>> Subject: Re: [PATCH] EnableTracing: output from multiple threads >>>>>>> may be mixed together >>>>>>> >>>>>>> On 7/05/2013 7:43 PM, 云达(Yunda) wrote: >>>>>>>> Hi David, >>>>>>>> >>>>>>>> Thanks for the review and I see your concern. Please see the >>>>>>>> updated change >>>>>>> below: >>>>>>>> >>>>>>>> diff -r 627cf9e9ea31 src/share/vm/runtime/globals.hpp >>>>>>>> --- a/src/share/vm/runtime/globals.hpp Mon May 06 10:29:49 2013 >>>>>>>> -0700 >>>>>>>> +++ b/src/share/vm/runtime/globals.hpp Tue May 07 17:38:58 2013 >>>>>>>> +++ +0800 >>>>>>>> @@ -3634,7 +3634,10 @@ >>>>>>>> "Include GC cause in GC logging") >>>>>>> \ >>>>>>>> >>>>>>> \ >>>>>>>> product(bool, EnableTracing, false, >>>>>>> \ >>>>>>>> - "Enable event-based tracing") >>>>>>>> + "Enable event-based tracing") >>>>>>> \ >>>>>>>> + >>>>>>> \ >>>>>>>> + product(bool, UseLockedTracing, false, >>>>>>> \ >>>>>>>> + "Use locked-tracing when doing event-based >>>>>>>> + tracing") >>>>>>>> >>>>>>>> /* >>>>>>>> * Macros for factoring of globals diff -r 627cf9e9ea31 >>>>>>>> src/share/vm/trace/traceEventClasses.xsl >>>>>>>> --- a/src/share/vm/trace/traceEventClasses.xsl Mon May 06 >>>>>>>> 10:29:49 >>>>>>>> 2013 -0700 >>>>>>>> +++ b/src/share/vm/trace/traceEventClasses.xsl Tue May 07 >>>>>>>> +++ 17:38:58 >>>>>>>> +++ 2013 +0800 >>>>>>>> @@ -132,6 +132,9 @@ >>>>>>>> void writeEvent(void) { >>>>>>>> ResourceMark rm; >>>>>>>> HandleMark hm; >>>>>>>> + if (UseLockedTracing) { >>>>>>>> + ttyLocker ttyl; >>>>>>>> + } >>>>>>> >>>>>>> Ah that doesn't work - the ttyLocker is block-scoped by the if >>>>>>> statement so it will be created, grab the lock, then immediately >>>>>>> be destructed and the lock released. >>>>>>> >>>>>>> David >>>>>>> ----- >>>>>>> >>>>>>>> TraceStream ts(*tty); >>>>>>>> ts.print("<xsl:value-of select="@label"/>: ["); >>>>>>>> <xsl:apply-templates select="value|structvalue" >>>>>>>> mode="write-data"/> >>>>>>>> >>>>>>>> Regards, >>>>>>>> Yunda >>>>>>>> >>>>>>>>> -----Original Message----- >>>>>>>>> From: David Holmes [mailto:david.hol...@oracle.com] >>>>>>>>> Sent: Tuesday, May 07, 2013 10:39 AM >>>>>>>>> To: 云达(Yunda) >>>>>>>>> Cc: hotspot-runtime-...@openjdk.java.net; >>>>>>>>> serviceability-dev@openjdk.java.net >>>>>>>>> Subject: Re: [PATCH] EnableTracing: output from multiple threads >>>>>>>>> may be mixed together >>>>>>>>> >>>>>>>>> Hi Yunda, >>>>>>>>> >>>>>>>>> There is a potential problem with using a ttyLocker here, >>>>>>>>> depending on exactly what events are being traced and from where >>>>>>>>> - you must always be in code where it is both safe to acquire >>>>>>>>> the lock, and safe to block waiting for the lock if it is not >>>>>>>>> available. >>>>>>>>> >>>>>>>>> I think I would prefer to see unlocked tracing by default with a >>>>>>>>> flag to use locked-tracing if requested. >>>>>>>>> >>>>>>>>> David >>>>>>>>> >>>>>>>>> On 19/04/2013 6:26 PM, 云达(Yunda) wrote: >>>>>>>>>> Hi all, >>>>>>>>>> >>>>>>>>>> I found that the output from multiple threads may be mixed >>>>>>>>>> together when using EnableTracing. It happens many times in my >>>>>>>>>> test >>>> case like this: >>>>>>>>>> >>>>>>>>>> Allocation outside TLAB: [Allocation in new TLAB: [Allocation >>>>>>>>>> in new >>>>>>>>>> TLAB: [Allocation in new TLAB: [Class = [C, Allocation in new TLAB: >>>>>>>>>> [Class = [I, Allocation in new TLAB: [Java Monitor Wait: [Class >>>>>>>>>> = java/lang/String, Allocation Size = 24, Allocation Size = 24, >>>>>>>>>> Class = java/lang/String, Class = [I, Allocation in new TLAB: >>>>>>>>>> [Class = [C, Allocation in new TLAB: [Allocation Size = 192] >>>>>>>>>> >>>>>>>>>> Class = >>>>>>>>>> com/sun/org/apache/xerces/internal/dom/DeferredElementNSImpl, >>>>>>>>>> Allocation Size = 24, Allocation Size = 24, TLAB Size = >>>>>>>>>> 23712280] >>>>>>>>>> >>>>>>>>>> TLAB Size = 24607080] >>>>>>>>>> >>>>>>>>>> Allocation Size = 24, Monitor Class = >>>>>>>>>> java/lang/ref/Reference$Lock, TLAB Size = 25054480] >>>>>>>>>> >>>>>>>>>> TLAB Size = 25054480] >>>>>>>>>> >>>>>>>>>> Allocation in new TLAB: [Class = [CTLAB Size = 24607080] >>>>>>>>>> >>>>>>>>>> Allocation Size = 72, Class = [C, TLAB Size = 24159728] >>>>>>>>>> >>>>>>>>>> , Allocation Size = 32, TLAB Size = 23712288] >>>>>>>>>> >>>>>>>>>> It's very confusing and it's even not easy to tell how many >>>>>>>>>> events there are. I think the reason is that the writeEvent() >>>>>>>>>> method of each >>>>>>>>>> Event* class output the fields of event one by one without >>>>>>>>>> using any lock. So I made a small patch which add ttyLocker to >>>>>>>>>> writeEvent() method and after applying this patch there's no >>>>>>>>>> output mixed together in my test case(against >>>>>>> http://hg.openjdk.java.net/hsx/hsx24/hotspot/): >>>>>>>>>> >>>>>>>>>> diff -r edd1619a3ae4 src/share/vm/trace/traceEventClasses.xsl >>>>>>>>>> >>>>>>>>>> --- a/src/share/vm/trace/traceEventClasses.xsl Thu Apr 18 >>>> 13:50:58 >>>>>>>>>> 2013 -0700 >>>>>>>>>> >>>>>>>>>> +++ b/src/share/vm/trace/traceEventClasses.xsl Fri Apr 19 >>>>>>>>>> +++ 16:12:38 >>>>>>>>>> +++ 2013 >>>>>>>>>> +0800 >>>>>>>>>> >>>>>>>>>> @@ -132,6 +132,7 @@ >>>>>>>>>> >>>>>>>>>> void writeEvent(void) { >>>>>>>>>> >>>>>>>>>> ResourceMark rm; >>>>>>>>>> >>>>>>>>>> HandleMark hm; >>>>>>>>>> >>>>>>>>>> + ttyLocker ttyl; >>>>>>>>>> >>>>>>>>>> TraceStream ts(*tty); >>>>>>>>>> >>>>>>>>>> ts.print("<xsl:value-of select="@label"/>: ["); >>>>>>>>>> >>>>>>>>>> <xsl:apply-templates select="value|structvalue" >>>>>>>>>> mode="write-data"/> >>>>>>>>>> >>>>>>>>>> I searched before sending this mail I didn't find anyone who >>>>>>>>>> covering thisJ >>>>>>>>>> >>>>>>>>>> Regards, >>>>>>>>>> >>>>>>>>>> Yunda >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> --------------------------------------------------------------- >>>>>>>>>> -- >>>>>>>>>> --- >>>>>>>>>> -- >>>>>>>>>> -- >>>>>>>>>> >>>>>>>>>> This email (including any attachments) is confidential and may >>>>>>>>>> be legally privileged. If you received this email in error, >>>>>>>>>> please delete it immediately and do not copy it or use it for >>>>>>>>>> any purpose or disclose its contents to any other person. Thank you. >>>>>>>>>> >>>>>>>>>> 本电邮(包括任何附件)可能含有机密资料并受法律保护。如您不 >> 是 >>>> 正确 >>>>>>> 的 >>>>>>>>> 收件人， >>>>>>>>>> 请您立即删除本邮件。请不要将本电邮进行复制并用作任何其他 >> 用 >>>> 途、 >>>>>>>>> 或透露本邮 >>>>>>>>>> 件之内容。谢谢。 >>>>>>>> >>>>>>>> ________________________________ >>>>>>>> >>>>>>>> This email (including any attachments) is confidential and may be >>>>>>>> legally >>>>>>> privileged. If you received this email in error, please delete it >>>>>>> immediately and do not copy it or use it for any purpose or >>>>>>> disclose its contents to any other person. Thank you. >>>>>>> >>>>>>>> >>>>>>>> 本电邮(包括任何附件)可能含有机密资料并受法律保护。如您不是 >> 正 >>>> 确的 >>>>>>> 收件人，请您立即删除本邮件。请不要将本电邮进行复制并用作任 >> 何 >>>> 其他 >>>>>>> 用途、或透 >>>>>>>> 露本邮件之内容。谢谢。 >>>>>>>> >>>>>> >>>>>> ________________________________ >>>>>> >>>>>> This email (including any attachments) is confidential and may be >>>>>> legally >>>> privileged. If you received this email in error, please delete it >>>> immediately and do not copy it or use it for any purpose or disclose >>>> its contents to any other person. Thank you. >>>>>> >>>>>> 本电邮(包括任何附件)可能含有机密资料并受法律保护。如您不是正 >> 确 >>>> 的收件人，请您立即删除本邮件。请不要将本电邮进行复制并用作任何 >> 其 >>>> 他用途、或 >>>>>> 透露本邮件之内容。谢谢。 >>>>>> >>> >>> >>> ________________________________ >>> >>> This email (including any attachments) is confidential and may be legally >> privileged. If you received this email in error, please delete it >> immediately and >> do not copy it or use it for any purpose or disclose its contents to any >> other >> person. Thank you. >> >>> >>> 本电邮(包括任何附件)可能含有机密资料并受法律保护。如您不是正确的 >> 收件人，请您立即删除本邮件。请不要将本电邮进行复制并用作任何其他 >> 用途、或透 >>> 露本邮件之内容。谢谢。 > > > ________________________________ > > This email (including any attachments) is confidential and may be legally > privileged. If you received this email in error, please delete it immediately > and do not copy it or use it for any purpose or disclose its contents to any > other person. Thank you. > > 本电邮(包括任何附件)可能含有机密资料并受法律保护。如您不是正确的收件人，请您立即删除本邮件。请不要将本电邮进行复制并用作任何其他用途、或透露本邮件之内容。谢谢。
