8351655: Optimize ObjectMonitor::unlink_after_acquire()

Reviewed-by: pchilanomate, dholmes, coleenp

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");
   }
 

src/hotspot/share/runtime/objectMonitor.hpp

@@ -402,6 +402,7 @@ class ObjectMonitor : public CHeapObj<mtObjectMonitor> {
   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);