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.
>>
>> 本电邮(包括任何附件)可能含有机密资料并受法律保护。如您不是正确的收件人，请您立即删除本邮件。请不要将本电邮进行复制并用作任何其他用途、或透露本邮件之内容。谢谢。
>>
