From 45a9750b2c9364a05e00cdb935ef986c0a2c3e19 Mon Sep 17 00:00:00 2001 From: Fredrik Bredberg Date: Wed, 26 Mar 2025 07:30:04 +0000 Subject: [PATCH] 8351655: Optimize ObjectMonitor::unlink_after_acquire() Reviewed-by: pchilanomate, dholmes, coleenp --- src/hotspot/share/runtime/objectMonitor.cpp | 80 +++++++++++++++------ src/hotspot/share/runtime/objectMonitor.hpp | 1 + 2 files changed, 59 insertions(+), 22 deletions(-) diff --git a/src/hotspot/share/runtime/objectMonitor.cpp b/src/hotspot/share/runtime/objectMonitor.cpp index aff48edd7d9..e280de6baf1 100644 --- a/src/hotspot/share/runtime/objectMonitor.cpp +++ b/src/hotspot/share/runtime/objectMonitor.cpp @@ -1258,32 +1258,69 @@ void ObjectMonitor::vthread_epilog(JavaThread* current, ObjectWaiter* node) { } } +// Convert entry_list into a doubly linked list by assigning the prev +// pointers and the entry_list_tail pointer (if needed). Within the +// entry_list the next pointers always form a consistent singly linked +// list. When this function is called, the entry_list will be either +// singly linked, or starting as singly linked (at the head), but +// ending as doubly linked (at the tail). +void ObjectMonitor::entry_list_build_dll(JavaThread* current) { + assert(has_owner(current), "invariant"); + ObjectWaiter* prev = nullptr; + // Need acquire here to match the implicit release of the cmpxchg + // that updated entry_list, so we can access w->prev(). + ObjectWaiter* w = Atomic::load_acquire(&_entry_list); + assert(w != nullptr, "should only be called when entry list is not empty"); + while (w != nullptr) { + assert(w->TState == ObjectWaiter::TS_ENTER, "invariant"); + assert(w->prev() == nullptr || w->prev() == prev, "invariant"); + if (w->prev() != nullptr) { + break; + } + w->_prev = prev; + prev = w; + w = w->next(); + } + if (w == nullptr) { + // We converted the entire entry_list from a singly linked list + // into a doubly linked list. Now we just need to set the tail + // pointer. + assert(prev != nullptr && prev->next() == nullptr, "invariant"); + assert(_entry_list_tail == nullptr || _entry_list_tail == prev, "invariant"); + _entry_list_tail = prev; + } else { +#ifdef ASSERT + // We stopped iterating through the _entry_list when we found a + // node that had its prev pointer set. I.e. we converted the first + // part of the entry_list from a singly linked list into a doubly + // linked list. Now we just want to make sure the rest of the list + // is doubly linked. But first we check that we have a tail + // pointer, because if the end of the entry_list is doubly linked + // and we don't have the tail pointer, something is broken. + assert(_entry_list_tail != nullptr, "invariant"); + while (w != nullptr) { + assert(w->TState == ObjectWaiter::TS_ENTER, "invariant"); + assert(w->prev() == prev, "invariant"); + prev = w; + w = w->next(); + } + assert(_entry_list_tail == prev, "invariant"); +#endif + } +} + // Return the tail of the _entry_list. If the tail is currently not -// known, find it by walking from the head of _entry_list, and while -// doing so assign the _prev pointers to create a doubly linked list. +// known, it can be found by first calling entry_list_build_dll(). ObjectWaiter* ObjectMonitor::entry_list_tail(JavaThread* current) { assert(has_owner(current), "invariant"); ObjectWaiter* w = _entry_list_tail; if (w != nullptr) { return w; } - // Need acquire here to match the implicit release of the cmpxchg - // that updated _entry_list, so we can access w->_next. - w = Atomic::load_acquire(&_entry_list); + entry_list_build_dll(current); + w = _entry_list_tail; assert(w != nullptr, "invariant"); - if (w->next() == nullptr) { - _entry_list_tail = w; - return w; - } - ObjectWaiter* prev = nullptr; - while (w != nullptr) { - assert(w->TState == ObjectWaiter::TS_ENTER, "invariant"); - w->_prev = prev; - prev = w; - w = w->next(); - } - _entry_list_tail = prev; - return prev; + return w; } // By convention we unlink a contending thread from _entry_list @@ -1317,9 +1354,9 @@ void ObjectMonitor::unlink_after_acquire(JavaThread* current, ObjectWaiter* curr if (currentNode->prev() == nullptr) { // Build the doubly linked list to get hold of // currentNode->prev(). - _entry_list_tail = nullptr; - entry_list_tail(current); + entry_list_build_dll(current); assert(currentNode->prev() != nullptr, "must be"); + assert(_entry_list_tail == currentNode, "must be"); } // The currentNode is the last element in _entry_list and we know // which element is the previous one. @@ -1363,8 +1400,7 @@ void ObjectMonitor::unlink_after_acquire(JavaThread* current, ObjectWaiter* curr } } // Build the doubly linked list to get hold of currentNode->prev(). - _entry_list_tail = nullptr; - entry_list_tail(current); + entry_list_build_dll(current); assert(currentNode->prev() != nullptr, "must be"); } diff --git a/src/hotspot/share/runtime/objectMonitor.hpp b/src/hotspot/share/runtime/objectMonitor.hpp index aa32b3cfbf1..54662ffab7b 100644 --- a/src/hotspot/share/runtime/objectMonitor.hpp +++ b/src/hotspot/share/runtime/objectMonitor.hpp @@ -402,6 +402,7 @@ class ObjectMonitor : public CHeapObj { void dequeue_specific_waiter(ObjectWaiter* waiter); void enter_internal(JavaThread* current); void reenter_internal(JavaThread* current, ObjectWaiter* current_node); + void entry_list_build_dll(JavaThread* current); void unlink_after_acquire(JavaThread* current, ObjectWaiter* current_node); ObjectWaiter* entry_list_tail(JavaThread* current);