On 10/05/2013 12:32 PM, 云达(Yunda) wrote:
Thanks for looking at this, Rickard. David, do you like this option which is 
easier to read, too :)

Sure.

David
-----


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.

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

Reply via email to