On Mon, 3 Jun 2024 13:31:16 GMT, Inigo Mediavilla Saiz <[email protected]> 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
