Hi all,
I added a test case and modified previous patch including fix the
copyright year to 2017.
I changed to output Java thread name next the separator lines in
"jhsdb jstack --mixed" case as following.
----------------- 32117 -----------------
"main"
0x00007f6c8feafa82 __pthread_cond_timedwait + 0x132
Could you possibly review for this following small change? If review
is ok, please commit this as cito.
Source:
diff --git
a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/PStack.java
b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/PStack.java
---
a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/PStack.java
+++
b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/PStack.java
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2003, 2013, Oracle and/or its affiliates. All rights
reserved.
+ * Copyright (c) 2003, 2017, Oracle and/or its affiliates. All rights
reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -88,6 +88,12 @@
out.print("----------------- ");
out.print(th);
out.println(" -----------------");
+ JavaThread jthread = (JavaThread) proxyToThread.get(th);
+ if (jthread != null) {
+ out.print("\"");
+ out.print(jthread.getThreadName());
+ out.println("\"");
+ }
while (f != null) {
ClosestSymbol sym = f.closestSymbolToPC();
Address pc = f.pc();
diff --git
a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/StackTrace.java
b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/StackTrace.java
---
a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/StackTrace.java
+++
b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/StackTrace.java
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2002, 2013, Oracle and/or its affiliates. All rights
reserved.
+ * Copyright (c) 2002, 2017, Oracle and/or its affiliates. All rights
reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -75,7 +75,9 @@
for (JavaThread cur = threads.first(); cur != null; cur =
cur.next(), i++) {
if (cur.isJavaThread()) {
Address sp = cur.getLastJavaSP();
- tty.print("Thread ");
+ tty.print("\"");
+ tty.print(cur.getThreadName());
+ tty.print("\" nid=");
cur.printThreadIDOn(tty);
tty.print(": (state = " + cur.getThreadState());
if (verbose) {
diff --git a/test/serviceability/sa/JhsdbThreadNameTest.java
b/test/serviceability/sa/JhsdbThreadNameTest.java
new file mode 100644
--- /dev/null
+++ b/test/serviceability/sa/JhsdbThreadNameTest.java
@@ -0,0 +1,107 @@
+/*
+ * Copyright (c) 2017, Oracle and/or its affiliates. All rights
reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but
WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file
that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License
version
+ * 2 along with this work; if not, write to the Free Software
Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA
94065 USA
+ * or visit www.oracle.com if you need additional information or have
any
+ * questions.
+ */
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.function.Consumer;
+
+import jdk.test.lib.apps.LingeredApp;
+import jdk.test.lib.JDKToolLauncher;
+import jdk.test.lib.Platform;
+import jdk.test.lib.process.OutputAnalyzer;
+import jdk.test.lib.Utils;
+
+/*
+ * @test
+ * @library /test/lib
+ * @run main/othervm JhsdbThreadNameTest
+ */
+public class JhsdbThreadNameTest {
+
+ private static String notMixedModeThreadNames[] =
{"Common-Cleaner", "Signal Dispatcher", "Finalizer", "Reference
Handler", "main"};
+ private static String mixedModeThreadNames[] = {"C2
CompilerThread0", "C1 CompilerThread1", "Sweeper thread", "Service
Thread"};
+
+ private static void startHsdbJstack(boolean mixed) throws
Exception {
+
+ LingeredApp app = null;
+
+ try {
+ List<String> vmArgs = new ArrayList<String>();
+ vmArgs.add("-Xmx10m");
+ vmArgs.addAll(Utils.getVmOptions());
+
+ app = LingeredApp.startApp(vmArgs);
+ System.out.println("Started LingeredApp with pid " +
app.getPid());
+
+ JDKToolLauncher jhsdbLauncher =
JDKToolLauncher.createUsingTestJDK("jhsdb");
+
+ jhsdbLauncher.addToolArg("jstack");
+ jhsdbLauncher.addToolArg("--pid");
+ jhsdbLauncher.addToolArg(Long.toString(app.getPid()));
+
+ if (mixed) {
+ jhsdbLauncher.addToolArg("--mixed");
+ }
+ ProcessBuilder pb = new ProcessBuilder();
+ pb.command(jhsdbLauncher.getCommand());
+ Process jhsdb = pb.start();
+
+ jhsdb.waitFor();
+
+ OutputAnalyzer out = new OutputAnalyzer(jhsdb);
+
+
Arrays.stream(notMixedModeThreadNames).map(JhsdbThreadNameTest::addQuotationMarks).forEach(out::shouldContain);
+ Consumer<String> testMethod = null;
+ if (mixed) {
+ testMethod = out::shouldContain;
+ } else {
+ testMethod = out::shouldNotContain;
+ }
+
Arrays.stream(mixedModeThreadNames).map(JhsdbThreadNameTest::addQuotationMarks).forEach(testMethod);
+
+ } catch (InterruptedException ie) {
+ throw new Error("Problem awaiting the child process: " +
ie, ie);
+ } catch (Exception attachE) {
+ throw new Error("Couldn't start jhsdb, attach to
LingeredApp or match ThreadName: " + attachE);
+
+ } finally {
+ LingeredApp.stopApp(app);
+ }
+ }
+
+ private static String addQuotationMarks(String str) {
+ return "\"" + str + "\"";
+ }
+
+ public static void main(String[] args) throws Exception {
+
+ if (!Platform.shouldSAAttach()) {
+ System.out.println("SA attach not expected to work - test
skipped.");
+ return;
+ }
+
+ startHsdbJstack(true);
+ startHsdbJstack(false);
+ }
+}
Regards,
Chihiro
On 2017/06/08 18:04, chihiro ito wrote:
Hi Jini,
Thank you for your advices. I try to add the test case and modify the
copyright year to 2017.
Basically, I agree with your idea, but I think that the separator
line should finally be the same as the output of the jstack command.
I worry which is better way.
Thanks,
Chihiro
On 2017/06/08 16:42, Jini George wrote:
Hi Chihiro,
Thank you for making this useful change. Your changes look good.
It would be great though if you could add a test case for this.
Could you also modify the copyright year to 2017 ? One additional
suggestion: The addition of the thread name makes the separator
lines unaligned in the pstack/jstack --mixed cases. Like:
----------------- "Service Thread" nid=16051 -----------------
and
----------------- nid=16052 -----------------
So I am wondering if it would make sense to have the name printed
out on a separate line to keep the separator lines aligned. But this
is a nit, and I would leave it to you to decide on this.
Thanks,
Jini (Not a (R)eviewer)
On 6/7/2017 3:16 PM, chihiro ito wrote:
Hi all,
I changed to output Java thread name in jhsdb jstack as following.
jhsdb jstack --pid 25879
"main" nid=25884: (state = BLOCKED)
jhsdb jstack --mixed --pid 25879
----------------- "main" nid=25884 -----------------
Could you possibly review for this following small change? If
review is ok, please commit this as cito.
Source:
diff --git
a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/PStack.java
b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/PStack.java
---
a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/PStack.java
+++
b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/PStack.java
@@ -86,6 +86,13 @@
try {
CFrame f = cdbg.topFrameForThread(th);
out.print("----------------- ");
+ JavaThread jthread = (JavaThread)
proxyToThread.get(th);
+ if (jthread != null) {
+ out.print("\"");
+ out.print(jthread.getThreadName());
+ out.print("\" ");
+ }
+ out.print("nid=");
out.print(th);
out.println(" -----------------");
while (f != null) {
diff --git
a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/StackTrace.java
b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/StackTrace.java
---
a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/StackTrace.java
+++
b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/StackTrace.java
@@ -75,7 +75,9 @@
for (JavaThread cur = threads.first(); cur != null;
cur = cur.next(), i++) {
if (cur.isJavaThread()) {
Address sp = cur.getLastJavaSP();
- tty.print("Thread ");
+ tty.print("\"");
+ tty.print(cur.getThreadName());
+ tty.print("\" nid=");
cur.printThreadIDOn(tty);
tty.print(": (state = " + cur.getThreadState());
if (verbose) {
Regards,
Chihiro
