add extra comments

Author: pchilano

src/hotspot/cpu/aarch64/interp_masm_aarch64.cpp

@@ -1539,20 +1539,20 @@ void InterpreterMacroAssembler::call_VM_preemptable(Register oop_result,
 
   push_cont_fastpath();
 
-  // Make VM call. In case of preemption set last_pc to
-  // the one we want to resume to.
+  // Make VM call. In case of preemption set last_pc to the one we want to resume to.
   adr(rscratch1, resume_pc);
   str(rscratch1, Address(rthread, JavaThread::last_Java_pc_offset()));
   call_VM_base(oop_result, noreg, noreg, entry_point, 1, false /*check_exceptions*/);
 
   pop_cont_fastpath();
 
-  // Check if preempted
+  // Check if preempted.
   ldr(rscratch1, Address(rthread, JavaThread::preempt_alternate_return_offset()));
   cbz(rscratch1, not_preempted);
   str(zr, Address(rthread, JavaThread::preempt_alternate_return_offset()));
   br(rscratch1);
 
+  // In case of preemption, this is where we will resume once we finally acquire the monitor.
   bind(resume_pc);
   restore_after_resume(false /* is_native */);
 

src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp

@@ -7150,19 +7150,20 @@ class StubGenerator: public StubCodeGenerator {
     // reset the flag
     __ strb(zr, Address(rthread, JavaThread::preempting_offset()));
 
-    // Set sp to enterSpecial frame and then remove it from the stack
+    // Set sp to enterSpecial frame, i.e. remove all frames copied into the heap.
     __ ldr(rscratch2, Address(rthread, JavaThread::cont_entry_offset()));
     __ mov(sp, rscratch2);
 
     Label preemption_cancelled;
     __ ldrb(rscratch1, Address(rthread, JavaThread::preemption_cancelled_offset()));
     __ cbnz(rscratch1, preemption_cancelled);
 
-    // Remove enterSpecial frame from the stack and return to Continuation.run()
+    // Remove enterSpecial frame from the stack and return to Continuation.run() to unmount.
     SharedRuntime::continuation_enter_cleanup(_masm);
     __ leave();
     __ ret(lr);
 
+    // We acquired the monitor after freezing the frames so call thaw to continue execution.
     __ bind(preemption_cancelled);
     __ strb(zr, Address(rthread, JavaThread::preemption_cancelled_offset()));
     __ lea(rfp, Address(sp, checked_cast<int32_t>(ContinuationEntry::size())));

src/hotspot/cpu/riscv/interp_masm_riscv.cpp

@@ -1585,20 +1585,20 @@ void InterpreterMacroAssembler::call_VM_preemptable(Register oop_result,
 
   push_cont_fastpath();
 
-  // Make VM call. In case of preemption set last_pc to
-  // the one we want to resume to.
+  // Make VM call. In case of preemption set last_pc to the one we want to resume to.
   la(t0, resume_pc);
   sd(t0, Address(xthread, JavaThread::last_Java_pc_offset()));
   call_VM_base(oop_result, noreg, noreg, entry_point, 1, false /*check_exceptions*/);
 
   pop_cont_fastpath();
 
-  // Check if preempted
+  // Check if preempted.
   ld(t0, Address(xthread, JavaThread::preempt_alternate_return_offset()));
   beqz(t0, not_preempted);
   sd(zr, Address(xthread, JavaThread::preempt_alternate_return_offset()));
   jr(t0);
 
+  // In case of preemption, this is where we will resume once we finally acquire the monitor.
   bind(resume_pc);
   restore_after_resume(false /* is_native */);
 

src/hotspot/cpu/riscv/stubGenerator_riscv.cpp

@@ -3883,18 +3883,19 @@ class StubGenerator: public StubCodeGenerator {
     // reset the flag
     __ sb(zr, Address(xthread, JavaThread::preempting_offset()));
 
-    // Set sp to enterSpecial frame and then remove it from the stack
+    // Set sp to enterSpecial frame, i.e. remove all frames copied into the heap.
     __ ld(sp, Address(xthread, JavaThread::cont_entry_offset()));
 
     Label preemption_cancelled;
     __ lbu(t0, Address(xthread, JavaThread::preemption_cancelled_offset()));
     __ bnez(t0, preemption_cancelled);
 
-    // Remove enterSpecial frame from the stack and return to Continuation.run()
+    // Remove enterSpecial frame from the stack and return to Continuation.run() to unmount.
     SharedRuntime::continuation_enter_cleanup(_masm);
     __ leave();
     __ ret();
 
+    // We acquired the monitor after freezing the frames so call thaw to continue execution.
     __ bind(preemption_cancelled);
     __ sb(zr, Address(xthread, JavaThread::preemption_cancelled_offset()));
     __ la(fp, Address(sp, checked_cast<int32_t>(ContinuationEntry::size() + 2 * wordSize)));

src/hotspot/cpu/x86/interp_masm_x86.cpp

@@ -345,22 +345,22 @@ void InterpreterMacroAssembler::call_VM_preemptable(Register oop_result,
 
   push_cont_fastpath();
 
-  // Make VM call. In case of preemption set last_pc to
-  // the one we want to resume to.
+  // Make VM call. In case of preemption set last_pc to the one we want to resume to.
   lea(rscratch1, resume_pc);
   push(rscratch1);
   MacroAssembler::call_VM_helper(oop_result, entry_point, 1, false /*check_exceptions*/);
   pop(rscratch1);
 
   pop_cont_fastpath();
 
-  // Check if preempted
+  // Check if preempted.
   movptr(rscratch1, Address(r15_thread, JavaThread::preempt_alternate_return_offset()));
   cmpptr(rscratch1, NULL_WORD);
   jccb(Assembler::zero, not_preempted);
   movptr(Address(r15_thread, JavaThread::preempt_alternate_return_offset()), NULL_WORD);
   jmp(rscratch1);
 
+  // In case of preemption, this is where we will resume once we finally acquire the monitor.
   bind(resume_pc);
   restore_after_resume(false /* is_native */);
 
@@ -371,7 +371,7 @@ void InterpreterMacroAssembler::restore_after_resume(bool is_native) {
   lea(rscratch1, ExternalAddress(Interpreter::cont_resume_interpreter_adapter()));
   call(rscratch1);
   if (is_native) {
-    // On resume we need to set up stack as expected
+    // On resume we need to set up stack as expected.
     push(dtos);
     push(ltos);
   }

src/hotspot/cpu/x86/stubGenerator_x86_64.cpp

@@ -3799,7 +3799,7 @@ address StubGenerator::generate_cont_preempt_stub() {
 
   __ reset_last_Java_frame(true);
 
-  // reset _preempting flag
+  // Check and reset _preempting flag.
 #ifdef ASSERT
   { Label L;
     __ movbool(rscratch1, Address(r15_thread, JavaThread::preempting_offset()));
@@ -3811,19 +3811,20 @@ address StubGenerator::generate_cont_preempt_stub() {
 #endif
   __ movbool(Address(r15_thread, JavaThread::preempting_offset()), false);
 
-  // Set rsp to enterSpecial frame
+  // Set rsp to enterSpecial frame, i.e. remove all frames copied into the heap.
   __ movptr(rsp, Address(r15_thread, JavaThread::cont_entry_offset()));
 
   Label preemption_cancelled;
   __ movbool(rscratch1, Address(r15_thread, JavaThread::preemption_cancelled_offset()));
   __ testbool(rscratch1);
   __ jcc(Assembler::notZero, preemption_cancelled);
 
-  // Remove enterSpecial frame from the stack and return to Continuation.run()
+  // Remove enterSpecial frame from the stack and return to Continuation.run() to unmount.
   SharedRuntime::continuation_enter_cleanup(_masm);
   __ pop(rbp);
   __ ret(0);
 
+  // We acquired the monitor after freezing the frames so call thaw to continue execution.
   __ bind(preemption_cancelled);
   __ movbool(Address(r15_thread, JavaThread::preemption_cancelled_offset()), false);
   __ lea(rbp, Address(rsp, checked_cast<int32_t>(ContinuationEntry::size())));

src/hotspot/share/runtime/continuationFreezeThaw.cpp

@@ -2216,6 +2216,8 @@ NOINLINE intptr_t* Thaw<ConfigT>::thaw_slow(stackChunkOop chunk, Continuation::t
   _preempted_case = chunk->preempted();
   if (_preempted_case) {
     if (chunk->object_waiter() != nullptr) {
+      // Mounted again after preemption. Resume the pending monitor operation,
+      // which will be either a monitorenter or Object.wait() call.
       assert(chunk->current_pending_monitor() != nullptr || chunk->current_waiting_monitor() != nullptr, "");
       ObjectWaiter* waiter = chunk->object_waiter();
       ObjectMonitor* mon = waiter->monitor();
@@ -2224,15 +2226,16 @@ NOINLINE intptr_t* Thaw<ConfigT>::thaw_slow(stackChunkOop chunk, Continuation::t
       bool mon_acquired = mon->resume_operation(_thread, waiter, _cont);
       assert(!mon_acquired || mon->is_owner(_thread), "invariant");
       if (!mon_acquired) {
+        // Failed to aquire monitor. Return to enterSpecial to unmount again.
         return push_cleanup_continuation();
       }
-      chunk = _cont.tail();  // reload oop in case of safepoint in resume_operation
+      chunk = _cont.tail();  // reload oop in case of safepoint in resume_operation (if posting JVMTI events).
     } else {
       // Preemption cancelled in moniterenter case. We actually acquired
       // the monitor after freezing all frames so nothing to do.
       preempt_kind = Continuation::freeze_on_monitorenter;
     }
-    // Call this first to avoid racing with GC threads later when modifying the flags.
+    // Call this first to avoid racing with GC threads later when modifying the chunk flags.
     relativize_chunk_concurrently(chunk);
     chunk->set_preempted(false);
     retry_fast_path = true;
@@ -2447,6 +2450,7 @@ intptr_t* ThawBase::handle_preempted_continuation(intptr_t* sp, Continuation::pr
   assert(top.pc() == *(address*)(sp - frame::sender_sp_ret_address_offset()), "");
 
 #if INCLUDE_JVMTI
+  // Finish the VTMS transition.
   assert(_thread->is_in_VTMS_transition(), "must be");
   bool is_vthread = Continuation::continuation_scope(_cont.continuation()) == java_lang_VirtualThread::vthread_scope();
   if (is_vthread) {
@@ -2469,6 +2473,7 @@ intptr_t* ThawBase::handle_preempted_continuation(intptr_t* sp, Continuation::pr
   }
 
   if (preempt_kind == Continuation::freeze_on_wait) {
+    // Check now if we need to throw IE exception.
     if (_thread->pending_interrupted_exception()) {
       throw_interrupted_exception(_thread, top);
       _thread->set_pending_interrupted_exception(false);

src/hotspot/share/runtime/objectMonitor.cpp

@@ -512,6 +512,9 @@ void ObjectMonitor::enter_with_contention_mark(JavaThread *current, ObjectMonito
       if (result == freeze_ok) {
         bool acquired = VThreadMonitorEnter(current);
         if (acquired) {
+          // We actually acquired the monitor while trying to add the vthread to the
+          // _cxq so cancel preemption. We will still go through the preempt stub
+          // but instead of unmounting we will call thaw to continue execution.
           current->set_preemption_cancelled(true);
           if (JvmtiExport::should_post_monitor_contended_entered()) {
             // We are going to call thaw again after this and finish the VMTS
@@ -1151,16 +1154,15 @@ bool ObjectMonitor::resume_operation(JavaThread* current, ObjectWaiter* node, Co
   oop vthread = current->vthread();
   if (is_succesor(current)) clear_succesor();
 
-  // Invariant: after clearing _succ a thread *must* retry _owner before parking.
+  // Invariant: after clearing _succ a thread *must* retry acquiring the monitor.
   OrderAccess::fence();
 
   if (TryLock(current) == TryLockResult::Success) {
     VThreadEpilog(current, node);
     return true;
   }
 
-  // The JT will read this variable on return to the resume_monitor_operation stub
-  // and will unmount (enterSpecial frame removed and return to Continuation.run()).
+  // We will return to Continuation.run() and unmount so set the right state.
   java_lang_VirtualThread::set_state(vthread, java_lang_VirtualThread::BLOCKING);
 
   return false;
@@ -1188,6 +1190,7 @@ void ObjectMonitor::VThreadEpilog(JavaThread* current, ObjectWaiter* node) {
   UnlinkAfterAcquire(current, node);
   delete node;
 
+  // Remove the ObjectWaiter* from the stackChunk.
   oop vthread = current->vthread();
   oop cont = java_lang_VirtualThread::continuation(vthread);
   stackChunkOop chunk  = jdk_internal_vm_Continuation::tail(cont);
@@ -1624,6 +1627,10 @@ static void vthread_monitor_waited_event(JavaThread *current, ObjectWaiter* node
       post_monitor_wait_event(event, node->_monitor, node->_notifier_tid, timeout, timed_out);
     }
     if (JvmtiExport::should_post_monitor_waited()) {
+      // We mark this call in case of an upcall to Java while posting the event.
+      // If somebody walks the stack in that case, processing the enterSpecial
+      // frame should not include processing callee arguments since there is no
+      // actual callee (see nmethod::preserve_callee_argument_oops()).
       ThreadOnMonitorWaitedEvent tmwe(current);
       JvmtiExport::vthread_post_monitor_waited(current, node->_monitor, timed_out);
     }
@@ -2004,7 +2011,7 @@ void ObjectMonitor::VThreadWait(JavaThread* current, jlong millis) {
   Thread::SpinRelease(&_WaitSetLock);
 
   node->_recursions = _recursions;   // record the old recursion count
-  _recursions = 0;                   // set the recursion level to be 1
+  _recursions = 0;                   // set the recursion level to be 0
   _waiters++;                        // increment the number of waiters
   exit(current);                     // exit the monitor
   guarantee(!is_owner(current), "invariant");
@@ -2013,17 +2020,16 @@ void ObjectMonitor::VThreadWait(JavaThread* current, jlong millis) {
   java_lang_VirtualThread::set_state(vthread, millis == 0 ? java_lang_VirtualThread::WAITING : java_lang_VirtualThread::TIMED_WAITING);
   java_lang_VirtualThread::set_waitTimeout(vthread, millis);
 
-  // Save the ObjectWaiter* in the chunk since we will need it
-  // when resuming execution.
+  // Save the ObjectWaiter* in the chunk since we will need it when resuming execution.
   oop cont = java_lang_VirtualThread::continuation(vthread);
   stackChunkOop chunk  = jdk_internal_vm_Continuation::tail(cont);
   chunk->set_object_waiter(node);
 }
 
 bool ObjectMonitor::VThreadWaitReenter(JavaThread* current, ObjectWaiter* node, ContinuationWrapper& cont) {
   // First time we run after being preempted on Object.wait().
-  // We need to check if we were interrupted or wait() timed-out
-  // and in that case remove ourselves from the _WaitSet queue.
+  // Check if we were interrupted or the wait timed-out, and in
+  // that case remove ourselves from the _WaitSet queue.
   if (node->TState == ObjectWaiter::TS_WAIT) {
     Thread::SpinAcquire(&_WaitSetLock, "WaitSet - unlink");
     if (node->TState == ObjectWaiter::TS_WAIT) {
@@ -2034,18 +2040,20 @@ bool ObjectMonitor::VThreadWaitReenter(JavaThread* current, ObjectWaiter* node,
     Thread::SpinRelease(&_WaitSetLock);
   }
 
+  // If this was an interrupted case, set the _interrupted boolean so that
+  // once we re-acquire the monitor we know if we need to throw IE or not.
   ObjectWaiter::TStates state = node->TState;
   bool was_notified = state == ObjectWaiter::TS_ENTER || state == ObjectWaiter::TS_CXQ;
   assert(was_notified || state == ObjectWaiter::TS_RUN, "");
-
-  // save it so that once we re-acquire the monitor we know if we need to throw IE.
   node->_interrupted = !was_notified && current->is_interrupted(false);
 
+  // Post JFR and JVMTI events.
   EventJavaMonitorWait event;
   if (event.should_commit() || JvmtiExport::should_post_monitor_waited()) {
     vthread_monitor_waited_event(current, node, cont, &event, !was_notified && !node->_interrupted);
   }
 
+  // Mark that we are at reenter so that we don't call this method again.
   node->_at_reenter = true;
   assert(!is_owner(current), "invariant");