From d6313cf7d860de51e809ae445aa681f35e3928e5 Mon Sep 17 00:00:00 2001 From: Patricio Chilano Mateo Date: Fri, 25 Oct 2024 13:39:56 -0400 Subject: [PATCH] Add/fix comments for David --- src/hotspot/share/classfile/javaClasses.cpp | 4 ++++ src/hotspot/share/runtime/objectMonitor.cpp | 6 +++++- src/hotspot/share/runtime/objectMonitor.hpp | 4 ++++ src/hotspot/share/runtime/objectMonitor.inline.hpp | 4 +--- src/hotspot/share/runtime/synchronizer.hpp | 2 +- 5 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/hotspot/share/classfile/javaClasses.cpp b/src/hotspot/share/classfile/javaClasses.cpp index 8d59f1a7341f0..79361bff67957 100644 --- a/src/hotspot/share/classfile/javaClasses.cpp +++ b/src/hotspot/share/classfile/javaClasses.cpp @@ -2079,6 +2079,10 @@ void java_lang_VirtualThread::set_next(oop vthread, oop next_vthread) { vthread->obj_field_put(_next_offset, next_vthread); } +// Add vthread to the waiting list if it's not already in it. Multiple threads +// could be trying to add vthread to the list at the same time, so we control +// access with a cmpxchg on onWaitingList. The winner adds vthread to the list. +// Method returns true if we added vthread to the list, false otherwise. bool java_lang_VirtualThread::set_onWaitingList(oop vthread, OopHandle& list_head) { jboolean* addr = vthread->field_addr(_onWaitingList_offset); jboolean vthread_on_list = Atomic::load(addr); diff --git a/src/hotspot/share/runtime/objectMonitor.cpp b/src/hotspot/share/runtime/objectMonitor.cpp index 08f9b869aba52..2d08a5ab729cf 100644 --- a/src/hotspot/share/runtime/objectMonitor.cpp +++ b/src/hotspot/share/runtime/objectMonitor.cpp @@ -1137,8 +1137,12 @@ bool ObjectMonitor::VThreadMonitorEnter(JavaThread* current, ObjectWaiter* waite return false; } +// Called from thaw code to resume the monitor operation that caused the vthread +// to be unmounted. Method returns true if the monitor is successfully acquired, +// which marks the end of the monitor operation, otherwise it returns false. bool ObjectMonitor::resume_operation(JavaThread* current, ObjectWaiter* node, ContinuationWrapper& cont) { assert(java_lang_VirtualThread::state(current->vthread()) == java_lang_VirtualThread::RUNNING, "wrong state for vthread"); + assert(!has_owner(current), ""); if (node->is_wait() && !node->at_reenter()) { bool acquired_monitor = VThreadWaitReenter(current, node, cont); @@ -1935,6 +1939,7 @@ void ObjectMonitor::INotify(JavaThread* current) { // is the only thread that grabs _WaitSetLock. There's almost no contention // on _WaitSetLock so it's not profitable to reduce the length of the // critical section. + if (!iterator->is_vthread()) { iterator->wait_reenter_begin(this); } @@ -2051,7 +2056,6 @@ bool ObjectMonitor::VThreadWaitReenter(JavaThread* current, ObjectWaiter* node, // Mark that we are at reenter so that we don't call this method again. node->_at_reenter = true; - assert(!has_owner(current), "invariant"); if (!was_notified) { bool acquired = VThreadMonitorEnter(current, node); diff --git a/src/hotspot/share/runtime/objectMonitor.hpp b/src/hotspot/share/runtime/objectMonitor.hpp index 1039a0b570750..a33e03e7cb10d 100644 --- a/src/hotspot/share/runtime/objectMonitor.hpp +++ b/src/hotspot/share/runtime/objectMonitor.hpp @@ -305,6 +305,10 @@ class ObjectMonitor : public CHeapObj { // Same as above but uses tid of current as new_value. int64_t try_set_owner_from(int64_t old_value, JavaThread* current); + // Methods to check and set _succ. The successor is the thread selected + // from _cxq/_EntryList by the current owner when releasing the monitor, + // to run again and re-try acquiring the monitor. It is used to avoid + // unnecessary wake-ups if there is already a successor set. bool has_successor(); bool has_successor(JavaThread* thread); void set_successor(JavaThread* thread); diff --git a/src/hotspot/share/runtime/objectMonitor.inline.hpp b/src/hotspot/share/runtime/objectMonitor.inline.hpp index 2074535486a20..e5742ff6616f7 100644 --- a/src/hotspot/share/runtime/objectMonitor.inline.hpp +++ b/src/hotspot/share/runtime/objectMonitor.inline.hpp @@ -107,7 +107,7 @@ inline bool ObjectMonitor::has_owner() const { return owner != NO_OWNER && owner != DEFLATER_MARKER; } -// Returns null if DEFLATER_MARKER is observed. +// Returns NO_OWNER if DEFLATER_MARKER is observed. inline int64_t ObjectMonitor::owner() const { int64_t owner = owner_raw(); return owner != DEFLATER_MARKER ? owner : NO_OWNER; @@ -126,8 +126,6 @@ inline void ObjectMonitor::set_stack_locker(BasicLock* locker) { } // Returns true if owner field == DEFLATER_MARKER and false otherwise. -// This accessor is called when we really need to know if the owner -// field == DEFLATER_MARKER and any non-null value won't do the trick. inline bool ObjectMonitor::owner_is_DEFLATER_MARKER() const { return owner_raw() == DEFLATER_MARKER; } diff --git a/src/hotspot/share/runtime/synchronizer.hpp b/src/hotspot/share/runtime/synchronizer.hpp index 900fbfc495dc2..cddc7f4835046 100644 --- a/src/hotspot/share/runtime/synchronizer.hpp +++ b/src/hotspot/share/runtime/synchronizer.hpp @@ -169,7 +169,7 @@ class ObjectSynchronizer : AllStatic { static void owned_monitors_iterate_filtered(MonitorClosure* closure, OwnerFilter filter); // Iterate ObjectMonitors where the owner is thread; this does NOT include - // ObjectMonitors where owner is set to a stack lock address in thread. + // ObjectMonitors where the owner is anonymous. static void owned_monitors_iterate(MonitorClosure* m, JavaThread* thread); // Iterate ObjectMonitors where the owner is vthread.