On Fri, 25 Oct 2024 21:33:24 GMT, Patricio Chilano Mateo <pchilanom...@openjdk.org> wrote:
>> This is the implementation of JEP 491: Synchronize Virtual Threads without >> Pinning. See [JEP 491](https://bugs.openjdk.org/browse/JDK-8337395) 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](https://bugs.openjdk.org/browse/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 th... > > Patricio Chilano Mateo has updated the pull request incrementally with two > additional commits since the last revision: > > - Restore use of atPointA in test StopThreadTest.java > - remove interruptible check from conditional in Object::wait src/hotspot/share/jvmci/vmStructs_jvmci.cpp line 329: > 327: nonstatic_field(ObjArrayKlass, _element_klass, > Klass*) \ > 328: > \ > 329: unchecked_nonstatic_field(ObjectMonitor, _owner, > int64_t) \ to make the type assert more precise: diff --git a/src/hotspot/share/jvmci/vmStructs_jvmci.cpp b/src/hotspot/share/jvmci/vmStructs_jvmci.cpp index 20b9609cdbf..f2b8a69c03f 100644 --- a/src/hotspot/share/jvmci/vmStructs_jvmci.cpp +++ b/src/hotspot/share/jvmci/vmStructs_jvmci.cpp @@ -326,7 +326,7 @@ \ nonstatic_field(ObjArrayKlass, _element_klass, Klass*) \ \ - unchecked_nonstatic_field(ObjectMonitor, _owner, int64_t) \ + volatile_nonstatic_field(ObjectMonitor, _owner, int64_t) \ volatile_nonstatic_field(ObjectMonitor, _recursions, intptr_t) \ volatile_nonstatic_field(ObjectMonitor, _cxq, ObjectWaiter*) \ volatile_nonstatic_field(ObjectMonitor, _EntryList, ObjectWaiter*) \ diff --git a/src/hotspot/share/runtime/vmStructs.cpp b/src/hotspot/share/runtime/vmStructs.cpp index 86d7277f88b..0492f28e15b 100644 --- a/src/hotspot/share/runtime/vmStructs.cpp +++ b/src/hotspot/share/runtime/vmStructs.cpp @@ -786,8 +786,8 @@ \ volatile_nonstatic_field(ObjectMonitor, _metadata, uintptr_t) \ unchecked_nonstatic_field(ObjectMonitor, _object, sizeof(void *)) /* NOTE: no type */ \ - unchecked_nonstatic_field(ObjectMonitor, _owner, int64_t) \ - unchecked_nonstatic_field(ObjectMonitor, _stack_locker, BasicLock*) \ + volatile_nonstatic_field(ObjectMonitor, _owner, int64_t) \ + volatile_nonstatic_field(ObjectMonitor, _stack_locker, BasicLock*) \ volatile_nonstatic_field(ObjectMonitor, _next_om, ObjectMonitor*) \ volatile_nonstatic_field(BasicLock, _metadata, uintptr_t) \ nonstatic_field(ObjectMonitor, _contentions, int) \ ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1818818274
