RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

Wed Nov 6 17:40:06 UTC 2024

On Mon, 4 Nov 2024 02:12:40 GMT, David Holmes <dholmes at 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...
>
> src/hotspot/share/classfile/javaClasses.cpp line 2107:
> 
>> 2105: 
>> 2106: jlong java_lang_VirtualThread::waitTimeout(oop vthread) {
>> 2107:   return vthread->long_field(_timeout_offset);
> 
> Not sure what motivated the name change but it seems odd to have the method named differently to the field it accesses. ??

It was initially parkTimeout and waitTimeout but it doesn't require two fields as you can't be waiting in Object.wait(timeout) and LockSupport.parkNanos at the same time. So the field was renamed, the accessors here should probably be renamed too.

> src/hotspot/share/prims/jvm.cpp line 4012:
> 
>> 4010:     }
>> 4011:     ThreadBlockInVM tbivm(THREAD);
>> 4012:     parkEvent->park();
> 
> What code does the unpark to wake this thread up? I can't quite see how this unparker thread operates as its logic seems dispersed.

It's very similar to the "Reference Handler" thread. That thread calls into the VM to get the pending-Reference list. Now we have "VirtualThread-unblocker" calling into the VM to get the list of virtual threads to unblock. ObjectMonitor::ExitEpilog will the unpark this thread when the virtual thread successor is on the list to unblock.

> src/java.base/share/classes/java/lang/Thread.java line 655:
> 
>> 653:      * early startup to generate the identifier for the primordial thread. The
>> 654:      * counter is off-heap and shared with the VM to allow it assign thread
>> 655:      * identifiers to non-Java threads.
> 
> Why do non-JavaThreads need an identifier of this kind?

JFR. We haven't changed anything there, just the initial tid.

> src/java.base/share/classes/java/lang/VirtualThread.java line 115:
> 
>> 113:      *   RUNNING -> WAITING        // transitional state during wait on monitor
>> 114:      *   WAITING -> WAITED         // waiting on monitor
>> 115:      *    WAITED -> BLOCKED        // notified, waiting to be unblocked by monitor owner
> 
> Waiting to re-enter the monitor?

yes

> src/java.base/share/classes/java/lang/VirtualThread.java line 178:
> 
>> 176:     // timed-wait support
>> 177:     private long waitTimeout;
>> 178:     private byte timedWaitNonce;
> 
> Strange name - what does this mean?

Sequence number, nouce, anything will work here as it's just to deal with the scenario where the timeout task for a previous wait may run concurrently with a subsequent wait.

> src/java.base/share/classes/java/lang/VirtualThread.java line 530:
> 
>> 528:                 && carrier == Thread.currentCarrierThread();
>> 529:         carrier.setCurrentThread(carrier);
>> 530:         Thread.setCurrentLockId(this.threadId()); // keep lock ID of virtual thread
> 
> I'm struggling to understand the different threads in play when this is called and what the method actual does to which threads. ??

A virtual thread is mounted but doing a timed-park that requires temporarily switching to the identity of the carrier (identity = Therad.currentThread) when queuing the timer task. As mentioned in a reply to Axel, we are close to the point of removing this (nothing to do with object monitors of course, we've had the complexity with temporary transitions since JDK 19).

More context here is that there isn't support yet for a carrier to own a monitor before a virtual thread is mounted, and same thing during these temporary transitions. If support for custom schedulers is exposed then that issue will need to be addressed as you don't want some entries on the lock stack owned by the carrier and the others by the mounted virtual thread. Patricio has mentioned inflating any held monitors before mount. There are a couple of efforts in this area going on now, all would need that issue fixed before anything is exposed.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1827219720
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1814450822
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1814387940
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1810579901
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1810583267
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1810598265