On Mon, 3 Jun 2024 13:31:16 GMT, Inigo Mediavilla Saiz <d...@openjdk.org> wrote:
>> Print the stack traces of mounted virtual threads when calling `jcmd <pid> >> Thread.print`. > > Inigo Mediavilla Saiz has updated the pull request incrementally with one > additional commit since the last revision: > > Print mounted virtual thread after carrier FWIW I considered the mounted VT to be effectively part of the carrier thread's state so it made sense to me to have it's stack first (and it is the more interesting stack). But if Alan and Ron want it this way so be it. I don't understand the logic you are using to get the indentation though. src/hotspot/share/runtime/javaThread.cpp line 1832: > 1830: st->print("\t"); > 1831: indentation--; > 1832: } Suggestion: while (indentation-- > 0) { st->print("\t"); } Though not sure using `\t` is the best way to indent this as stream indentation is based on spaces. test/hotspot/jtreg/serviceability/dcmd/thread/PrintVirtualThreadTest.java line 2: > 1: /* > 2: * Copyright (c) 2015, 2024, Oracle and/or its affiliates. All rights > reserved. New file should only have current year in copyright notice. test/hotspot/jtreg/serviceability/dcmd/thread/PrintVirtualThreadTest.java line 24: > 22: */ > 23: > 24: import com.beust.ah.A; ??? test/hotspot/jtreg/serviceability/dcmd/thread/PrintVirtualThreadTest.java line 37: > 35: import java.util.concurrent.atomic.AtomicBoolean; > 36: import java.util.concurrent.locks.ReentrantLock; > 37: import java.util.regex.Pattern; Seems to be a number of unneeded imports here. test/hotspot/jtreg/serviceability/dcmd/thread/PrintVirtualThreadTest.java line 46: > 44: * java.compiler > 45: * java.management > 46: * jdk.internal.jvmstat/sun.jvmstat.monitor These don't all seem necessary. test/hotspot/jtreg/serviceability/dcmd/thread/PrintVirtualThreadTest.java line 52: > 50: > 51: public void run(CommandExecutor executor) throws InterruptedException > { > 52: var shouldStop = new AtomicBoolean(); You never set this to stop the thread. ------------- Changes requested by dholmes (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19482#pullrequestreview-2095343672 PR Review Comment: https://git.openjdk.org/jdk/pull/19482#discussion_r1625365087 PR Review Comment: https://git.openjdk.org/jdk/pull/19482#discussion_r1625370894 PR Review Comment: https://git.openjdk.org/jdk/pull/19482#discussion_r1625371025 PR Review Comment: https://git.openjdk.org/jdk/pull/19482#discussion_r1625371252 PR Review Comment: https://git.openjdk.org/jdk/pull/19482#discussion_r1625371685 PR Review Comment: https://git.openjdk.org/jdk/pull/19482#discussion_r1625373779