I don't understand why this format is totally different from the normal stack traces? At least the header with the stack names could be similar?
Gruss Bernd -- http://bernd.eckenfels.net ________________________________ From: serviceability-dev <serviceability-dev-boun...@openjdk.java.net> on behalf of chihiro ito <chihiro....@oracle.com> Sent: Wednesday, June 14, 2017 9:17:42 AM To: Jini George; serviceability-dev@openjdk.java.net Subject: Re: [10] RFR 8181647: jhsdb jstack could not output thread name 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<http://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 >>> >> > -- Chihiro Ito | Principal Consultant | +81.90.6148.8815 Oracle <http://www.oracle.com> Consultant ORACLE Japan | Akasaka Center Bldg. | Motoakasaka 1-3-13 | 1070051 Minato-ku, Tokyo, JAPAN Oracle is committed to developing practices and products that help protect the environment <http://www.oracle.com/commitment>
