On Wed, 19 Nov 2025 13:50:44 GMT, Alan Bateman <[email protected]> wrote:
>> Patricio Chilano Mateo has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Add fixes to DumpThreadsWithEliminatedLock.java
>
> src/java.base/share/classes/java/lang/VirtualThread.java line 1390:
>
>> 1388: }
>> 1389:
>> 1390: // -- JVM TI support --
>
> We'll need to update is comment as it no longer only for JVMTI.
>
> This might be a good place for a block comment to define "transitions"
> covering the changing of thread identity the continuation mount/unmount, and
> how the notification to the VM support JVMTI and handshakes. Maybe I could
> contribute a block comment to include here?
That would be great.
> src/java.base/share/native/libjava/VirtualThread.c line 38:
>
>> 36: { "startFinalTransition", "()V", (void *)&JVM_VirtualThreadEnd
>> },
>> 37: { "startTransition", "(Z)V", (void
>> *)&JVM_VirtualThreadStartTransition },
>> 38: { "endTransition", "(Z)V", (void
>> *)&JVM_VirtualThreadEndTransition },
>
> I wonder if JVM_VirtualThreadStart and JVM_VirtualThreadEnd should be renamed
> to have EndFirstTransition and StartFinalTransaction in the names so it's
> easy to follow through from the Java code down to
> MountUnmountDisabler::start_transition/end_transition.
How about removing these methods and just have an extra boolean parameter in
`start/endTransition`?
https://github.com/pchilano/jdk/compare/JDK-8364343...pchilano:jdk:startEndTransitionsOnly
> test/jdk/com/sun/management/HotSpotDiagnosticMXBean/DumpThreadsWhenParking.java
> line 94:
>
>> 92: });
>> 93: }
>> 94: // wait for all virtual threads to start so all have a
>> non-empty stack
>
> This reminds me the loom repo has a small update to to the
> DumpThreadsWithEliminatedLock.java test to ensure that the virtual thread
> starts execution before doing the thread dump. This was noticed with
> test-repeat runs of the new test to ensure it was stable.
Added the fixes to `DumpThreadsWithEliminatedLock.java` from the loom repo.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28361#discussion_r2543217770
PR Review Comment: https://git.openjdk.org/jdk/pull/28361#discussion_r2543212884
PR Review Comment: https://git.openjdk.org/jdk/pull/28361#discussion_r2543215701
