8337199: Add jcmd Thread.vthread_summary diagnostic command #22121

larry-cable · 2024-11-14T21:34:08Z

c.f: https://bugs.openjdk.org/browse/JDK-8339420

Summary

Add jcmd <pid> Thread.vthread_summary to print summary information that is useful when trying to diagnose issues with virtual threads.

Problem

The JDK is lacking tooling to diagnose issues with virtual threads.

Solution

Add a new command that the jcmd command line tool can use to print information about virtual threads. The output includes the virtual thread scheduler, the schedulers used to support timeouts, and the I/O pollers used to support virtual threads doing socket I/O, and a summary of the thread groupings.

Here is sample output. The output is intended for experts and is not intended for automated parsing.

Virtual thread scheduler:
java.util.concurrent.ForkJoinPool@4a624db0[Running, parallelism = 16, size = 2, active = 0, running = 0, steals = 2, tasks = 0, submissions = 0]

Timeout schedulers:
[0] java.util.concurrent.ScheduledThreadPoolExecutor@1f17ae12[Running, pool size = 0, active threads = 0, queued tasks = 0, completed tasks = 0]
[1] java.util.concurrent.ScheduledThreadPoolExecutor@6193b845[Running, pool size = 1, active threads = 0, queued tasks = 1, completed tasks = 0]
[2] java.util.concurrent.ScheduledThreadPoolExecutor@c4437c4[Running, pool size = 0, active threads = 0, queued tasks = 0, completed tasks = 0]
[3] java.util.concurrent.ScheduledThreadPoolExecutor@3f91beef[Running, pool size = 0, active threads = 0, queued tasks = 0, completed tasks = 0]

Read I/O pollers:
[0] sun.nio.ch.KQueuePoller@524bf25 [registered = 1]

Write I/O pollers:
[0] sun.nio.ch.KQueuePoller@25c41da2 [registered = 0]

Thread groupings:
<root> [platform threads = 11, virtual threads = 0]
java.util.concurrent.ScheduledThreadPoolExecutor@c4437c4 [platform threads = 0, virtual threads = 0]
java.util.concurrent.ScheduledThreadPoolExecutor@3f91beef [platform threads = 0, virtual threads = 0]
ForkJoinPool.commonPool/jdk.internal.vm.SharedThreadContainer@4fa374ea [platform threads = 0, virtual threads = 0]
java.util.concurrent.ThreadPoolExecutor@506e1b77 [platform threads = 1, virtual threads = 0]
java.util.concurrent.ScheduledThreadPoolExecutor@1f17ae12 [platform threads = 0, virtual threads = 0]
java.util.concurrent.ThreadPerTaskExecutor@24155ffc [platform threads = 0, virtual threads = 2]
ForkJoinPool-1/jdk.internal.vm.SharedThreadContainer@48a03463 [platform threads = 2, virtual threads = 0]
java.util.concurrent.ScheduledThreadPoolExecutor@6193b845 [platform threads = 1, virtual threads = 0]

The "Virtual thread scheduler" section show the target parallelism, the number of threads in the scheduler's pool, and other useful counters.

The "Timeout schedulers" section provides information on the usage of the ScheduledThreadPoolExecutor used to support virtual thread doing timed operations.

The "I/O pollers" section will vary by OS. The sample output here is macOS which, by default, has a one read and one write poller. This is useful to see how many blocking network I/O operations on virtual threads are in progress.

The "Thread groupings" appears a support people to get a sense of how the virtual threads are grouped. Virtual threads created directly with the Thread API are in the "root" grouping. Some servers may have one or more ThreadPerTaskExecutor instances. For now, this section doesn't print the tree of thread groupings that arises when using the Structured Concurrency API.

Specification

The output from jcmd <pid> Thread.vthread_summary -help is:

Thread.vthread_summary`
   Print the virtual thread scheduler, timeout schedulers, I/O pollers, and thread groupings.

   Impact: Low

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change requires CSR request JDK-8339420 to be approved
Change must not contain extraneous whitespace
Commit message must refer to an issue

Integration blocker

⚠️ Title mismatch between PR and JBS for issue JDK-8337199

Warnings

⚠️ Patch contains a binary file (src/java.base/share/classes/jdk/internal/icu/impl/data/icudt74b/nfc.nrm)
⚠️ Patch contains a binary file (src/java.base/share/classes/jdk/internal/icu/impl/data/icudt74b/nfkc.nrm)
⚠️ Patch contains a binary file (src/java.base/share/classes/jdk/internal/icu/impl/data/icudt74b/ubidi.icu)
⚠️ Patch contains a binary file (src/java.base/share/classes/jdk/internal/icu/impl/data/icudt74b/uprops.icu)
⚠️ Patch contains a binary file (src/java.desktop/share/classes/javax/swing/text/doc-files/Document-insert.gif)
⚠️ Patch contains a binary file (src/java.desktop/share/classes/sun/awt/resources/security-icon-bw16.png)
⚠️ Patch contains a binary file (src/java.desktop/share/classes/sun/awt/resources/security-icon-bw24.png)
⚠️ Patch contains a binary file (src/java.desktop/share/classes/sun/awt/resources/security-icon-bw32.png)
⚠️ Patch contains a binary file (src/java.desktop/share/classes/sun/awt/resources/security-icon-bw48.png)
⚠️ Patch contains a binary file (src/java.desktop/share/classes/sun/awt/resources/security-icon-interim16.png)
⚠️ Patch contains a binary file (src/java.desktop/share/classes/sun/awt/resources/security-icon-interim24.png)
⚠️ Patch contains a binary file (src/java.desktop/share/classes/sun/awt/resources/security-icon-interim32.png)
⚠️ Patch contains a binary file (src/java.desktop/share/classes/sun/awt/resources/security-icon-interim48.png)
⚠️ Patch contains a binary file (src/java.desktop/share/classes/sun/awt/resources/security-icon-yellow16.png)
⚠️ Patch contains a binary file (src/java.desktop/share/classes/sun/awt/resources/security-icon-yellow24.png)
⚠️ Patch contains a binary file (src/java.desktop/share/classes/sun/awt/resources/security-icon-yellow32.png)
⚠️ Patch contains a binary file (src/java.desktop/share/classes/sun/awt/resources/security-icon-yellow48.png)
⚠️ Patch contains a binary file (src/java.desktop/windows/native/libawt/windows/security_warning.ico)
⚠️ Patch contains a binary file (src/utils/IdealGraphVisualizer/View/src/main/resources/com/sun/hotspot/igv/view/images/hideDuplicates.png)

Issues

JDK-8337199: Add jcmd Thread.vthread_scheduler and Thread.vthread_pollers diagnostic commands (Enhancement - P4) ⚠️ Title mismatch between PR and JBS. ⚠️ Issue is already resolved. Consider making this a "backport pull request" by setting the PR title to Backport <hash> with the hash of the original commit. See Backports.
JDK-8339420: Add jcmd Thread.vthread_scheduler and Thread.vthread_pollers diagnostic commands (CSR)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/22121/head:pull/22121
$ git checkout pull/22121

Update a local copy of the PR:
$ git checkout pull/22121
$ git pull https://git.openjdk.org/jdk.git pull/22121/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 22121

View PR using the GUI difftool:
$ git pr show -t 22121

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/22121.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2024-11-14T21:35:00Z

👋 Welcome back larry-cable! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2024-11-14T21:35:47Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

openjdk · 2024-11-14T21:36:12Z

@larry-cable this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout JDK-8337199
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

openjdk · 2024-11-14T21:36:32Z

@larry-cable The following labels will be automatically applied to this pull request:

core-libs
hotspot
nio
security
serviceability

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

…and disabled asssociated tests

mlbridge · 2024-11-25T16:22:47Z

Webrevs

00: Full (df08aa67)

AlanBateman · 2024-11-25T16:50:01Z

In the CSR I think we've agreed to drop the thread groupings from this command. Can you update the Solution sectioning this PR so the sample output aligns with that?

The PR shows merge conflicts, can you sync up the branch to closer to main line? The man page update moves from jcmd.1 to jcmd.md. Drop "thread groupings" from the description, and also VThreadSummaryDCmd::description (diagnosticCommand.hpp).

I see the commit to comment out printing the thread groupings but I think better to remove the unused code. You can grab the latest VThreadSummary.java and VThreadSummaryTest.java from the loom repo. Also can you grab Poller.java too as it has the correct merge with JEP 491.

AlanBateman · 2024-11-25T16:54:49Z

/label remove security
/label remove nio

openjdk · 2024-11-25T16:56:00Z

@AlanBateman
The security label was successfully removed.

openjdk · 2024-11-25T16:56:36Z

@AlanBateman
The nio label was successfully removed.

larry-cable · 2024-11-25T17:53:42Z

ok

…

On 11/25/24 8:50 AM, Alan Bateman wrote: In the CSR I think we've agreed to drop the thread groupings from this command. Can you update the Solution section so the sample output aligns with that? The PR shows merge conflicts, can you sync up the branch to closer to main line? The man page update moves from jcmd.1 to jcmd.md. Drop "thread groupings" from the description, and also VThreadSummaryDCmd::description (diagnosticCommand.hpp). I see the commit to comment out printing the thread groupings but I think better to remove the unused code. You can grab the latest VThreadSummary.java and VThreadSummaryTest.java from the loom repo. Also can you grab Poller.java too as it has the correct merge with JEP 491. — Reply to this email directly, view it on GitHub <https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/22121*issuecomment-2498536459__;Iw!!ACWV5N9M2RV99hQ!JjPtvmHtMJvorfhhN34WXQwd5s4Dfp1nV0vfAizdAFkCIr7fneoIgYFNyK5oKvkoNygpkMv8yeJiPHBbx7pBSB4glQ$>, or unsubscribe <https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ANTA67SBYHIVRB7FN6EIPVD2CNINBAVCNFSM6AAAAABRZ3DKS2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIOJYGUZTMNBVHE__;!!ACWV5N9M2RV99hQ!JjPtvmHtMJvorfhhN34WXQwd5s4Dfp1nV0vfAizdAFkCIr7fneoIgYFNyK5oKvkoNygpkMv8yeJiPHBbx7qqQGP0tg$>. You are receiving this because you were mentioned.Message ID: ***@***.***>

--------------43A07On2mpTNaH2Wy6LHFItz Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit <!DOCTYPE html><html><head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body> ok <div class="moz-cite-prefix">On 11/25/24 8:50 AM, Alan Bateman wrote: </div> <blockquote type="cite" ***@***.***"> In the CSR I think we've agreed to drop the thread groupings from this command. Can you update the Solution section so the sample output aligns with that? The PR shows merge conflicts, can you sync up the branch to closer to main line? The man page update moves from jcmd.1 to jcmd.md. Drop "thread groupings" from the description, and also VThreadSummaryDCmd::description (diagnosticCommand.hpp). I see the commit to comment out printing the thread groupings but I think better to remove the unused code. You can grab the latest VThreadSummary.java and VThreadSummaryTest.java from the loom repo. Also can you grab Poller.java too as it has the correct merge with JEP 491. — Reply to this email directly, <a href="https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/22121*issuecomment-2498536459__;Iw!!ACWV5N9M2RV99hQ!JjPtvmHtMJvorfhhN34WXQwd5s4Dfp1nV0vfAizdAFkCIr7fneoIgYFNyK5oKvkoNygpkMv8yeJiPHBbx7pBSB4glQ$" moz-do-not-send="true">view it on GitHub</a>, or <a href="https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ANTA67SBYHIVRB7FN6EIPVD2CNINBAVCNFSM6AAAAABRZ3DKS2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIOJYGUZTMNBVHE__;!!ACWV5N9M2RV99hQ!JjPtvmHtMJvorfhhN34WXQwd5s4Dfp1nV0vfAizdAFkCIr7fneoIgYFNyK5oKvkoNygpkMv8yeJiPHBbx7qqQGP0tg$" moz-do-not-send="true">unsubscribe</a>. You are receiving this because you were mentioned.<img src="https://github.com/notifications/beacon/ANTA67SHNLF7ZLZXXI2FCHT2CNINBA5CNFSM6AAAAABRZ3DKS2WGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTUU5SSAW.gif" alt="" moz-do-not-send="true" width="1" height="1">Message ID: <openjdk/jdk/pull/22121/c2498536459@github.com> <script type="application/ld+json">[ { ***@***.***": "http://schema.org", ***@***.***": "EmailMessage", "potentialAction": { ***@***.***": "ViewAction", "target": "#22121 (comment)", "url": "#22121 (comment)", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { ***@***.***": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script> </blockquote> </body> </html>

--------------43A07On2mpTNaH2Wy6LHFItz--

dholmes-ora

Generally looks like a good addition. We need to finalize names and content as per CSR request. A few coding/style issues that I've flagged.

Thanks.

dholmes-ora · 2024-11-26T01:31:24Z

src/hotspot/share/services/diagnosticCommand.cpp

+  Klass* k = SystemDictionary::resolve_or_fail(sym, true, CHECK);
+  if (HAS_PENDING_EXCEPTION) {


You can never reach line 1124 if there is a pending exception because the CHECK macro will do a return from the method. I see the same problem pre-exists in the other DCmd code. If you want to handle exceptions directly then you want to use THREAD not CHECK.

dholmes-ora · 2024-11-26T01:33:25Z

src/hotspot/share/services/diagnosticCommand.cpp

+  // check that result is byte array
+  oop res = cast_to_oop(result.get_jobject());
+  assert(res->is_typeArray(), "just checking");
+  assert(TypeArrayKlass::cast(res->klass())->element_type() == T_BYTE, "just checking");


I just called this out in another PR. These assertions are completely unnecessary unless you are trying to check the basic JavaCall operation - which you should not be. You are calling a method that returns a byte[] and that is what you will get back.

dholmes-ora · 2024-11-26T01:34:57Z

src/hotspot/share/services/diagnosticCommand.hpp

  static const char* name() { return "Thread.print"; }
  static const char* description() {
-    return "Print all threads with stacktraces.";
+    return "Print all platform threads with stacktraces.";


Probably best not hide this correction in this PR but file a separate issue for it.

dholmes-ora · 2024-11-26T01:37:33Z

src/java.base/share/classes/jdk/internal/vm/VThreadSummary.java

+        sb.append("Virtual thread scheduler:")
+                .append(System.lineSeparator());
+        sb.append(JLA.virtualThreadDefaultScheduler())
+                .append(System.lineSeparator());


Stylistically it is common/customary to align the dot operator in chained method calls.

It was done this way in the loom repo because the first string is the heading, the strings that follow go under the heading. Also you'll find the IDE will just indent these anyway.

dholmes-ora · 2024-11-26T01:38:55Z

src/java.base/share/classes/jdk/internal/vm/VThreadSummary.java

+        sb.append("Virtual thread scheduler:")
+                .append(System.lineSeparator());


Can't you just do:

sb.append("Virtual thread scheduler:\n")

?

Sorry I was thinking about %n but you can't use that here.

dholmes-ora · 2024-11-26T01:41:34Z

src/java.base/share/classes/jdk/internal/vm/VThreadSummary.java

+                    .append(System.lineSeparator());
+            sb.append(System.lineSeparator());


Is this style trying to draw attention to the blank lines between sections? It looks odd to me to not chain the final append as well.

It is a blank line, shouldn't be chained to the previous statement.

dholmes-ora · 2024-11-26T01:43:20Z

src/java.base/share/classes/sun/nio/ch/Poller.java

+    @Override
+    public String toString() {
+        return Objects.toIdentityString(this) + " [registered = " + map.size() + "]";
+    }


Why did you move this and "inline" the content of registered()?

I think that is PR merge issue, it's correct in the loom repo.

dholmes-ora · 2024-11-26T01:44:56Z

src/jdk.jcmd/share/man/jcmd.1

The .1 nroff files are no longer in the repo, you need to edit the markdown source instead.

That was already pointed out, I think the PR was merged from an older version.

dholmes-ora · 2024-11-26T01:45:57Z

test/hotspot/jtreg/serviceability/dcmd/thread/VThreadSummaryTest.java

+        jcmd().shouldContain("Virtual thread scheduler:")
+                .shouldContain(Objects.toIdentityString(defaultScheduler()))
+                .shouldContain("Timeout schedulers:")
+                .shouldContain("[0] " + ScheduledThreadPoolExecutor.class.getName());


Again please align dots on chained calls.

and the IDE will of course re-align it at next edit :-)

bridgekeeper · 2024-12-24T07:50:07Z

@larry-cable This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper · 2025-01-21T09:50:52Z

@larry-cable This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.

AlanBateman added 4 commits November 14, 2024 12:48

Initial commit

d9a13ed

Update from loom repo

1fee1ad

Sync up from loom repo

64566b5

Sync with loom repo

57e840c

openjdk bot added the merge-conflict Pull request has merge conflict with target branch label Nov 14, 2024

openjdk bot added serviceability [email protected] security [email protected] nio [email protected] hotspot [email protected] core-libs [email protected] labels Nov 14, 2024

JDK-8337199: merge master

aa91058

openjdk bot added merge-conflict Pull request has merge conflict with target branch and removed merge-conflict Pull request has merge conflict with target branch labels Nov 14, 2024

JDK-8337199: removed thread groupings iimpl from dcmd as per Alan B, …

df08aa6

…and disabled asssociated tests

larry-cable changed the title ~~JDK-83371899: Add jcmd Thread.vthread_summary diagnostic command~~ JDK-8337199: Add jcmd Thread.vthread_summary diagnostic command Nov 25, 2024

openjdk bot changed the title ~~JDK-8337199: Add jcmd Thread.vthread_summary diagnostic command~~ 8337199: Add jcmd Thread.vthread_summary diagnostic command Nov 25, 2024

openjdk bot added csr Pull request needs approved CSR before integration rfr Pull request is ready for review labels Nov 25, 2024

openjdk bot removed the security [email protected] label Nov 25, 2024

openjdk bot removed the nio [email protected] label Nov 25, 2024

dholmes-ora suggested changes Nov 26, 2024

View reviewed changes

AlanBateman mentioned this pull request Nov 27, 2024

8337199: Add jcmd Thread.vthread_scheduler and Thread.vthread_pollers diagnostic commands #22414

Closed

4 tasks

openjdk bot removed the csr Pull request needs approved CSR before integration label Dec 2, 2024

bridgekeeper bot closed this Jan 21, 2025

		Klass* k = SystemDictionary::resolve_or_fail(sym, true, CHECK);
		if (HAS_PENDING_EXCEPTION) {

		sb.append("Virtual thread scheduler:")
		.append(System.lineSeparator());

		.append(System.lineSeparator());
		sb.append(System.lineSeparator());

8337199: Add jcmd Thread.vthread_summary diagnostic command #22121

8337199: Add jcmd Thread.vthread_summary diagnostic command #22121

Uh oh!

Conversation

larry-cable commented Nov 14, 2024 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Problem

Solution

Specification

Progress

Integration blocker

Warnings

Issues

Reviewing

Uh oh!

bridgekeeper bot commented Nov 14, 2024

Uh oh!

openjdk bot commented Nov 14, 2024

Uh oh!

openjdk bot commented Nov 14, 2024

Uh oh!

openjdk bot commented Nov 14, 2024

Uh oh!

mlbridge bot commented Nov 25, 2024

Webrevs

Uh oh!

AlanBateman commented Nov 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlanBateman commented Nov 25, 2024

Uh oh!

openjdk bot commented Nov 25, 2024

Uh oh!

openjdk bot commented Nov 25, 2024

Uh oh!

larry-cable commented Nov 25, 2024 via email

Uh oh!

dholmes-ora left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlanBateman Nov 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bridgekeeper bot commented Dec 24, 2024

Uh oh!

bridgekeeper bot commented Jan 21, 2025

Uh oh!

Reviewers

Assignees

Labels

larry-cable commented Nov 14, 2024 •

edited by openjdk bot

Loading

AlanBateman commented Nov 25, 2024 •

edited

Loading

AlanBateman Nov 26, 2024 •

edited

Loading