On Wed, 28 Jan 2026 09:35:00 GMT, Alan Bateman <[email protected]> wrote:
> JDK-8364343 upgraded the virtual thread transition management to be
> independent of JVMTI. We can update java_lang_Thread::async_get_stack_trace
> to use it and remove the suspend + retry code from Thread.getStackTrace.
>
> A summary of the changes:
>
> - java_lang_Thread::async_get_stack_trace is changed to use the new handshake
> op so it can be called to get the stack trace of a started thread in any state
> - Thread::getStackTrace is changed to use async_get_stack_trace for all cases
> - The SUSPENDED substate in VirtualThread is removed
> - JVM_CreateThreadSnapshot is changed to be usable when JVMTI is not compiled
> in
> - ThreadSnapshotFactory::get_thread_snapshot is changed to not upcall to
> StackTraceElement to complete the init of the stack trace
>
> The changes mean that Thread::getStackTrace may be slower when sampling a
> virtual thread in transition. This case should be rare, and it isn't really a
> performance critical op anyway. I prototyped use a spin loop and an
> increasing wait time in MountUnmountDisabler::disable_transition_for_one to
> avoid the wait(10) but decided to leave it out for now. Future work may
> examine this issue as there may be other cases (with JVMTI) that would
> benefit from avoiding the wait.
>
> A future PR might propose to change Thread.getStackTrace to use
> ThreadSnapshot and allow java_lang_Thread::async_get_stack_trace be removed.
> This requires more extensive changes to ThreadSnapshotFactory to reduce
> overhead when only the stack trace is required.
>
> Testing: tier1-5. The changes has been already been tested in the loom repo
> for a few months.
I've taken a pass through this. Great to see all the cleanup and simplification
but unless you know the detailed back-story here it is difficult to understand
some of the changes (or even some of the code that hasn't changed).
src/hotspot/share/services/threadService.cpp line 1479:
> 1477: if (cl._thread_status == JavaThreadStatus::NEW || cl._thread_status
> == JavaThreadStatus::TERMINATED) {
> 1478: return nullptr;
> 1479: }
It is not obvious to me why this is only a possibility now?
src/hotspot/share/services/threadService.cpp line 1509:
> 1507:
> 1508: // call static StackTraceElement[]
> StackTraceElement.of(StackTraceElement[] stackTrace)
> 1509: // to properly initialize STEs.
Why can this be removed?
src/java.base/share/classes/java/lang/StackTraceElement.java line 578:
> 576: }
> 577:
> 578: static StackTraceElement[] finishInit(StackTraceElement[]
> stackTrace) {
Out of curiousity what does this actually do, and why do we need it?
src/java.base/share/classes/java/lang/Thread.java line 2218:
> 2216: }
> 2217: Object trace = getStackTrace0();
> 2218: if (trace instanceof StackTraceElement[] stackTrace) {
What can this return other than a `StackTraceElement[]` ??
src/java.base/share/classes/jdk/internal/vm/ThreadSnapshot.java line 65:
> 63: ThreadSnapshot snapshot = create(thread);
> 64: if (snapshot == null) {
> 65: return null; // thread not alive
I can understand that you might call this on a thread that terminates before
the operation can be performed. But not-alive implies NEW threads too and I
would have expected NEW threads to be filtered out long before we get here.
test/micro/org/openjdk/bench/java/lang/ThreadGetStackTraceWhenParked.java line
2:
> 1: /*
> 2: * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
Are these "new" test files from the Loom repo?
-------------
PR Review: https://git.openjdk.org/jdk/pull/29461#pullrequestreview-3720632723
PR Review Comment: https://git.openjdk.org/jdk/pull/29461#discussion_r2739903913
PR Review Comment: https://git.openjdk.org/jdk/pull/29461#discussion_r2739905261
PR Review Comment: https://git.openjdk.org/jdk/pull/29461#discussion_r2739932529
PR Review Comment: https://git.openjdk.org/jdk/pull/29461#discussion_r2739914245
PR Review Comment: https://git.openjdk.org/jdk/pull/29461#discussion_r2739919446
PR Review Comment: https://git.openjdk.org/jdk/pull/29461#discussion_r2739928137
