RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v30]

[Amit Kumar]

On Tue, 5 Nov 2024 01:40:15 GMT, Patricio Chilano Mateo <pchilanomate 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...
>
> Patricio Chilano Mateo has updated the pull request incrementally with five additional commits since the last revision:
> 
>  - Add oopDesc::has_klass_gap() check
>  - Rename waitTimeout/set_waitTimeout accessors
>  - Revert suggestion to ThawBase::new_stack_frame
>  - Improve JFR pinned reason in event
>  - Use freeze_result consistently

Hi @pchilano, 

I see couple of failures on s390x, can you apply this patch: 


diff --git a/src/hotspot/cpu/s390/macroAssembler_s390.cpp b/src/hotspot/cpu/s390/macroAssembler_s390.cpp
index f342240f3ca..d28b4579824 100644
--- a/src/hotspot/cpu/s390/macroAssembler_s390.cpp
+++ b/src/hotspot/cpu/s390/macroAssembler_s390.cpp
@@ -3492,7 +3492,7 @@ void MacroAssembler::increment_counter_eq(address counter_address, Register tmp1
 void MacroAssembler::compiler_fast_lock_object(Register oop, Register box, Register temp1, Register temp2) {
 
   assert(LockingMode != LM_LIGHTWEIGHT, "uses fast_lock_lightweight");
-  assert_different_registers(oop, box, temp1, temp2);
+  assert_different_registers(oop, box, temp1, temp2, Z_R0_scratch);
 
   Register displacedHeader = temp1;
   Register currentHeader   = temp1;
@@ -3566,8 +3566,8 @@ void MacroAssembler::compiler_fast_lock_object(Register oop, Register box, Regis
   // If csg succeeds then CR=EQ, otherwise, register zero is filled
   // with the current owner.
   z_lghi(zero, 0);
-  z_l(Z_R1_scratch, Address(Z_thread, JavaThread::lock_id_offset()));
-  z_csg(zero, Z_R1_scratch, OM_OFFSET_NO_MONITOR_VALUE_TAG(owner), monitor_tagged);
+  z_lg(Z_R0_scratch, Address(Z_thread, JavaThread::lock_id_offset()));
+  z_csg(zero, Z_R0_scratch, OM_OFFSET_NO_MONITOR_VALUE_TAG(owner), monitor_tagged);
 
   // Store a non-null value into the box.
   z_stg(box, BasicLock::displaced_header_offset_in_bytes(), box);
@@ -3576,7 +3576,7 @@ void MacroAssembler::compiler_fast_lock_object(Register oop, Register box, Regis
 
   BLOCK_COMMENT("fast_path_recursive_lock {");
   // Check if we are already the owner (recursive lock)
-  z_cgr(Z_R1_scratch, zero); // owner is stored in zero by "z_csg" above
+  z_cgr(Z_R0_scratch, zero); // owner is stored in zero by "z_csg" above
   z_brne(done); // not a recursive lock
 
   // Current thread already owns the lock. Just increment recursion count.
@@ -3594,7 +3594,7 @@ void MacroAssembler::compiler_fast_lock_object(Register oop, Register box, Regis
 void MacroAssembler::compiler_fast_unlock_object(Register oop, Register box, Register temp1, Register temp2) {
 
   assert(LockingMode != LM_LIGHTWEIGHT, "uses fast_unlock_lightweight");
-  assert_different_registers(oop, box, temp1, temp2);
+  assert_different_registers(oop, box, temp1, temp2, Z_R0_scratch);
 
   Register displacedHeader = temp1;
   Register currentHeader   = temp2;
@@ -3641,8 +3641,8 @@ void MacroAssembler::compiler_fast_unlock_object(Register oop, Register box, Reg
   // Handle existing monitor.
   bind(object_has_monitor);
 
-  z_l(Z_R1_scratch, Address(Z_thread, JavaThread::lock_id_offset()));
-  z_cg(Z_R1_scratch, Address(currentHeader, OM_OFFSET_NO_MONITOR_VALUE_TAG(owner)));
+  z_lg(Z_R0_scratch, Address(Z_thread, JavaThread::lock_id_offset()));
+  z_cg(Z_R0_scratch, Address(currentHeader, OM_OFFSET_NO_MONITOR_VALUE_TAG(owner)));
   z_brne(done);
 
   BLOCK_COMMENT("fast_path_recursive_unlock {");
@@ -6164,7 +6164,7 @@ void MacroAssembler::lightweight_unlock(Register obj, Register temp1, Register t
 }
 
 void MacroAssembler::compiler_fast_lock_lightweight_object(Register obj, Register box, Register tmp1, Register tmp2) {
-  assert_different_registers(obj, box, tmp1, tmp2);
+  assert_different_registers(obj, box, tmp1, tmp2, Z_R0_scratch);
 
   // Handle inflated monitor.
   NearLabel inflated;
@@ -6296,12 +6296,12 @@ void MacroAssembler::compiler_fast_lock_lightweight_object(Register obj, Registe
     // If csg succeeds then CR=EQ, otherwise, register zero is filled
     // with the current owner.
     z_lghi(zero, 0);
-    z_l(Z_R1_scratch, Address(Z_thread, JavaThread::lock_id_offset()));
-    z_csg(zero, Z_R1_scratch, owner_address);
+    z_lg(Z_R0_scratch, Address(Z_thread, JavaThread::lock_id_offset()));
+    z_csg(zero, Z_R0_scratch, owner_address);
     z_bre(monitor_locked);
 
     // Check if recursive.
-    z_cgr(Z_R1_scratch, zero); // zero contains the owner from z_csg instruction
+    z_cgr(Z_R0_scratch, zero); // zero contains the owner from z_csg instruction
     z_brne(slow_path);
 
     // Recursive


CC: @RealLucy

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

PR Review: https://git.openjdk.org/jdk/pull/21565#pullrequestreview-2414585800