8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

[pchilano]

This is the implementation of JEP 491: Synchronize Virtual Threads without Pinning. See JEP 491 for further details.

In order to make the code review easier the changes have been split into the following initial 4 commits:

• Changes to allow unmounting a virtual thread that is currently holding monitors.
• Changes to allow unmounting a virtual thread blocked on synchronized trying to acquire the monitor.
• Changes to allow unmounting a virtual thread blocked in Object.wait() and its timed-wait variants.
• Changes to tests, JFR pinned event, and other changes in the JDK libraries.

The changes fix pinning issues for all 4 ports that currently implement continuations: x64, aarch64, riscv and ppc. Note: ppc changes were added recently and stand in its own commit after the initial ones.

The changes fix pinning issues when using LM_LIGHTWEIGHT, i.e. the default locking mode, (and LM_MONITOR which comes for free), but not when using LM_LEGACY mode. Note that the LockingMode flag has already been deprecated (JDK-8334299), with the intention to remove LM_LEGACY code in future releases.

Summary of changes

Unmount virtual thread while holding monitors

As stated in the JEP, currently when a virtual thread enters a synchronized method or block, the JVM records the virtual thread's carrier platform thread as holding the monitor, not the virtual thread itself. This prevents the virtual thread from being unmounted from its carrier, as ownership information would otherwise go wrong. In order to fix this limitation we will do two things:

• We copy the oops stored in the LockStack of the carrier to the stackChunk when freezing (and clear the LockStack). We copy the oops back to the LockStack of the next carrier when thawing for the first time (and clear them from the stackChunk). Note that we currently assume carriers don't hold monitors while mounting virtual threads.

• For inflated monitors we now record the java.lang.Thread.tid of the owner in the ObjectMonitor's _owner field instead of a JavaThread*. This allows us to tie the owner of the monitor to a java.lang.Thread instance, rather than to a JavaThread which is only created per platform thread. The tid is already a 64 bit field so we can ignore issues of the counter wrapping around.

General notes about this part:

• Since virtual threads don't need to worry about holding monitors anymore, we don't need to count them, except for LM_LEGACY. So the majority of the platform dependent changes in this commit have to do with correcting this.
• Zero and x86 (32 bits) where counting monitors even though they don't implement continuations, so I fixed that to stop counting. The idea is to remove all the counting code once we remove LM_LEGACY.
• Macro LOOM_MONITOR_SUPPORT was added at the time to exclude ports that implement continuations but don't yet implement monitor support. It is removed later with the ppc commit changes.
• Since now a virtual thread can be unmounted while holding monitors, JVMTI methods GetOwnedMonitorInfo and GetOwnedMonitorStackDepthInfo had to be adapted.

Notes specific to the tid changes:

• The tid is cached in the JavaThread object under _lock_id. It is set on JavaThread creation and changed on mount/unmount.
• Changes in the ObjectMonitor class in this commit are pretty much exclusively related to changing _owner and _succ from void* and JavaThread* respectively to int64_t.
• Although we are not trying to fix LM_LEGACY the tid changes apply to it as well since the inflated path is shared. Thus, in case of inflation by a contending thread, the BasicLock* cannot be stored in the _owner field as before. The _owner is instead set to anonymous as we do in LM_LIGHTWEIGHT, and the BasicLock* is stored in the new field _stack_locker.
• We already assume 32 bit platforms can handle 64 bit atomics, including cmpxchg (JDK-8318776) so the shared code can stay the same. The assembly code for the c2 fast paths has to be adapted though. On arm (32bits) we already jump directly to the slow path on inflated monitor case so there is nothing to do. For x86 (32bits), since the port is moving towards deprecation (JDK-8338285) there is no point in trying to optimize, so the code was changed to do the same thing we do for arm (32bits).

Unmounting a virtual thread blocked on synchronized

Currently virtual thread unmounting is always started from Java, either because of a voluntarily call to Thread.yield() or because of performing some blocking operation such as I/O. Now we allow to unmount from inside the VM too, specifically when facing contention trying to acquire a Java monitor.

On failure to acquire a monitor inside ObjectMonitor::enter a virtual thread will call freeze to copy all Java frames to the heap. We will add the virtual thread to the ObjectMonitor's queue and return back to Java. Instead of continue execution in Java though, the virtual thread will jump to a preempt stub which will clear the frames copied from the physical stack, and will return to Continuation.run() to proceed with the unmount logic. Once the owner releases the monitor and selects it as the next successor the virtual thread will be added again to the scheduler queue to run again. The virtual thread will run and attempt to acquire the monitor again. If it succeeds then it will thaw frames as usual to continue execution back were it left off. If it fails it will unmount and wait again to be unblocked.

General notes about this part:

• The easiest way to review these changes is to start from the monitorenter call in the interpreter and follow all the flow of the virtual thread, from unmounting to running again.
• Currently we use a dedicated unblocker thread to submit the virtual threads back to the scheduler queue. This avoids calls to Java from monitorexit. We are experimenting on removing this limitation, but that will be left as an enhancement for a future change.
• We cannot unmount the virtual thread when the monitor enter call is coming from jni_enter() or ObjectLocker since we would need to freeze native frames.
• If freezing fails, which almost always will be due to having native frames on the stack, the virtual thread will follow the normal platform thread logic but will do a timed-park instead. This is to alleviate some deadlocks cases where the successor picked is an unmounted virtual thread that cannot run, which can happen during class loading or class initiatialization.
• After freezing all frames, and while adding itself to the _cxq the virtual thread could have successfully acquired the monitor. In that case we mark the preemption as cancelled. The virtual thread will still need to go back to the preempt stub to cleanup the physical stack but instead of unmounting it will call thaw to continue execution.
• The way we jump to the preempt stub is slightly different in the compiler and interpreter. For the compiled case we just patch a return address, so no new code is added. For the interpreter we cannot do this on all platforms so we just check a flag back in the interpreter. For the latter we also need to manually restore some state after we finally acquire the monitor and resume execution. All that logic is contained in new assembler method call_VM_preemptable().

Notes specific to JVMTI changes:

• Since we are not unmounting from Java, there is no call to VirtualThread.yieldContinuation(). This means that we have to execute the equivalent of notifyJvmtiUnmount(/*hide*/true) for unmount, and of notifyJvmtiMount(/*hide*/false) for mount in the VM. The former is implemented with JvmtiUnmountBeginMark in Continuation::try_preempt(). The latter is implemented in method jvmti_mount_end() in ContinuationFreezeThaw at the end of thaw.
• When unmounting from Java the vthread unmount event is posted before we try to freeze the continuation. If that fails then we post the mount event. This all happens in VirtualThread.yieldContinuation(). When unmounting from the VM we only post the event once we know the freeze succeeded. Since at that point we are in the middle of the VTMS transition, posting the event is done in JvmtiVTMSTransitionDisabler::VTMS_unmount_end() after the transition finishes. Maybe the same thing should be done when unmounting from Java.

Unmounting a virtual thread blocked on Object.wait()

This commit just extends the previous mechanism to be able to unmount inside the VM on ObjectMonitor::wait.

General notes about this part:

• The mechanism works as before with the difference that now the call will come from the native wrapper. This requires to add support to the continuation code to handle native wrapper frames, which is a main part of the changes in this commit.
• Both the compiled and interpreted native wrapper code will check for preemption on return from the wait call, after we have transitioned back to _thread_in_Java.

Note specific to JVMTI changes:

• If the monitor waited event is enabled we need to post it after the wait is done but before re-acquiring the monitor. Since the virtual thread is inside the VTMS transition at that point, we cannot do that directly. Currently in the code we end the transition, post the event and start the transition again. This is not ideal, and maybe we should unmount, post the event and then run again to try reacquire the monitor.

Test changes + JFR Updates + Library code changes

Tests

• The tests in java/lang/Thread/virtual are updated to add more tests for monitor enter/exit and Object.wait/notify. New tests are added for JFR events, synchronized native methods, and stress testing for several scenarios.
• test/hotspot/gtest/nmt/test_vmatree.cpp is changed due to an alias that conflicts.
• A small number of tests, e.g. test/hotspot/jtreg/serviceability/sa/ClhsdbInspect.java and test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/bcinstr/BI04/bi04t002, are updated so they are in sync with the JDK code.
• A number of JVMTI tests are updated to fix various issues, e.g. some tests saved a JNIEnv in a static.

Diagnosing remaining pinning issues

• The diagnostic option jdk.tracePinnedThreads is removed.
• The JFR jdk.VirtualThreadPinned event is changed so that it's now recorded in the VM, and for the following cases: parking when pinned, blocking in monitor enter when pinned, Object.wait when pinned, and waiting for a class to be initialized by another thread. The changes to object monitors should mean that only a few events are recorded. Future work may change this to a sampling approach.

Other changes to VirtualThread class

The VirtualThread implementation includes a few robustness changes. The park/parkNanos methods now park on the carrier if the freeze throws OOME. Moreover, the use of transitions is reduced so that the call out to the scheduler no longer requires a temporary transition.

Other changes to libraries:

• ReferenceQueue is reverted to use synchronized, the subclass based on ReentrantLock is removed. This change is done now because the changes for object monitors impact this area when there is preemption polling a reference queue.
• java.io is reverted to use synchronized. This change has been important for testing virtual threads. There will be follow-up cleanup in main-line after the JEP is integrated to remove InternalLock and its uses in java.io.
• The epoll and kqueue based Selectors are changed to preempt when doing blocking selects. This has been useful for testing virtual threads with some libraries, e.g. JDBC drivers. We could potentially separate this update if needed but it has been included in all testing and EA builds.
• sun.security.ssl.X509TrustManagerImpl is changed to eagerly initialize AnchorCertificates, a forced change due to deadlocks in this code when testing.

Testing

The changes have been running in the Loom pipeline for several months now. They have also been included in EA builds throughout the year at different stages (EA builds from earlier this year did not had Object.wait() support yet but more recent ones did) so there has been some external exposure too.

The current patch has been run through mach5 tiers 1-8. I'll keep running tests periodically until integration time.

---

Progress

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

Issues

• JDK-8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning (Enhancement - P2)
• JDK-8338813: Implement JEP 491: Synchronize Virtual Threads without Pinning (CSR)

Reviewers

• Axel Boldt-Christmas (@xmas92 - Reviewer)
• David Holmes (@dholmes-ora - Reviewer)
• Coleen Phillimore (@coleenp - Reviewer) Review applies to 9fd4c036
• Fredrik Bredberg (@fbredber - Committer) Review applies to 52c26642
• Dean Long (@dean-long - Reviewer) Review applies to 52c26642
• Serguei Spitsyn (@sspitsyn - Reviewer) Review applies to 62b16c6a

Contributors

• Patricio Chilano Mateo <pchilanomate@openjdk.org>
• Alan Bateman <alanb@openjdk.org>
• Andrew Haley <aph@openjdk.org>
• Fei Yang <fyang@openjdk.org>
• Coleen Phillimore <coleenp@openjdk.org>
• Richard Reingruber <rrich@openjdk.org>
• Martin Doerr <mdoerr@openjdk.org>

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 21565

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

Using diff file

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

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back pchilanomate! 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]

@pchilano This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

Co-authored-by: Patricio Chilano Mateo <pchilanomate@openjdk.org>
Co-authored-by: Alan Bateman <alanb@openjdk.org>
Co-authored-by: Andrew Haley <aph@openjdk.org>
Co-authored-by: Fei Yang <fyang@openjdk.org>
Co-authored-by: Coleen Phillimore <coleenp@openjdk.org>
Co-authored-by: Richard Reingruber <rrich@openjdk.org>
Co-authored-by: Martin Doerr <mdoerr@openjdk.org>
Reviewed-by: aboldtch, dholmes, coleenp, fbredberg, dlong, sspitsyn

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 12 new commits pushed to the master branch:

• 3727f40: 8343745: Only update Last Value Assertion Predicates in Loop Unrolling
• b53ee05: 8202617: javadoc generates broken links to undocumented (e.g. private) members
• cfe719f: 8340565: Create separate index page for terms defined by the index tag
• baabfbb: 8341904: Search tag in inherited doc comment creates additional index item
• 4fa760a: 8343936: Adjust timeout in test javax/management/monitor/DerivedGaugeMonitorTest.java
• cbf4dd5: 8343555: RISC-V: make some verified (on hardware) extension options diagnostic
• ef0dc25: 8342707: Prepare Gatherers for graduation from Preview
• 889f906: 8343774: Positive list platforms for ir checks of compiler/c2/TestCastX2NotProcessedIGVN.java
• 6088d62: 8343755: Unproblemlist java/lang/Thread/jni/AttachCurrentThread/AttachTest.java
• 80f4c0c: 8343442: Add since checker tests to the networking area modules
• ... and 2 more: https://git.openjdk.org/jdk/compare/babb52a08361b00eb4bc6e2e109b1fdc198dbd59...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[openjdk]

@pchilano The following labels will be automatically applied to this pull request:

• core-libs
• graal
• 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.

[pchilano]

/contributor add @pchilano
/contributor add @AlanBateman
/contributor add @theRealAph
/contributor add @RealFYang
/contributor add @coleenp

[openjdk]

@pchilano
Contributor Patricio Chilano Mateo <pchilanomate@openjdk.org> successfully added.

[openjdk]

@pchilano
Contributor Alan Bateman <alanb@openjdk.org> successfully added.

[openjdk]

@pchilano
Contributor Andrew Haley <aph@openjdk.org> successfully added.

[openjdk]

@pchilano
Contributor Fei Yang <fyang@openjdk.org> successfully added.

[openjdk]

@pchilano
Contributor Coleen Phillimore <coleenp@openjdk.org> successfully added.

[pchilano]

/label remove security

[openjdk]

@pchilano
The security label was successfully removed.

[pchilano]

/contributor add @reinrich
/contributor add @TheRealMDoerr

[openjdk]

@pchilano
Contributor Richard Reingruber <rrich@openjdk.org> successfully added.

[pchilano]

I merged with master, including the changes for JEP 450, and run it through tiers1-8 in mach5 with no related failures. I would like to integrate tomorrow if I could get some re-approvals. Also, I filed JDK-8343957 to possibly improve the naming of _lock_id/owner_from as discussed in some of the comments.

[dholmes-ora]

Still good. Thanks

[pchilano]

Many thanks to all reviewers and contributors of this JEP!

[pchilano]

/integrate

[openjdk]

Going to push as commit 78b8015.
Since your change was applied there have been 17 commits pushed to the master branch:

• 8a2a75e: 8339892: Several security shell tests don't set TESTJAVAOPTS
• 50b6e41: 8300732: Whitebox functions for Metaspace test should use byte size
• 67d1ef1: 8344018: Remove unlimited memory setting from TestScalarReplacementMaxLiveNodes
• 5729227: 8343250: ArrayBlockingQueue serialization not thread safe
• 2c1e4c3: 8343600: RISC-V: enable CRC32 intrinsic when either Zba or RVV are supported
• 3727f40: 8343745: Only update Last Value Assertion Predicates in Loop Unrolling
• b53ee05: 8202617: javadoc generates broken links to undocumented (e.g. private) members
• cfe719f: 8340565: Create separate index page for terms defined by the index tag
• baabfbb: 8341904: Search tag in inherited doc comment creates additional index item
• 4fa760a: 8343936: Adjust timeout in test javax/management/monitor/DerivedGaugeMonitorTest.java
• ... and 7 more: https://git.openjdk.org/jdk/compare/babb52a08361b00eb4bc6e2e109b1fdc198dbd59...master

Your commit was automatically rebased without conflicts.

[openjdk]

@pchilano Pushed as commit 78b8015.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.