On Tue, 5 Nov 2024 07:51:05 GMT, Erik Gahlin <egah...@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/jfr/metadata/metadata.xml line 160:
> 
>> 158: 
>> 159:   <Event name="VirtualThreadPinned" category="Java Virtual Machine, 
>> Runtime" label="Virtual Thread Pinned" thread="true" stackTrace="true">
>> 160:     <Field type="string" name="blockingOperation" label="Blocking 
>> operation" />
> 
> The label should be "Blocking Operation" with a capital "O".
> 
> Labels use headline-style capitalization. See here for more information: 
> https://docs.oracle.com/en/java/javase/21/docs/api/jdk.jfr/jdk/jfr/Label.html

Fixed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1829463128

Reply via email to