From e2e1598ecc19a8c39bc68c05d5982da1fec106ea Mon Sep 17 00:00:00 2001 From: Coleen Phillimore Date: Wed, 2 Apr 2025 23:37:56 +0000 Subject: [PATCH] 8353584: [BACKOUT] DaCapo xalan performance with -XX:+UseObjectMonitorTable Reviewed-by: rkennke --- src/hotspot/share/runtime/basicLock.cpp | 2 +- src/hotspot/share/runtime/basicLock.hpp | 6 +- .../share/runtime/basicLock.inline.hpp | 10 +--- .../share/runtime/lightweightSynchronizer.cpp | 55 ++++++------------- .../share/runtime/lightweightSynchronizer.hpp | 4 +- src/hotspot/share/runtime/objectMonitor.hpp | 1 - src/hotspot/share/runtime/synchronizer.cpp | 3 +- .../share/runtime/synchronizer.inline.hpp | 2 +- 8 files changed, 26 insertions(+), 57 deletions(-) diff --git a/src/hotspot/share/runtime/basicLock.cpp b/src/hotspot/share/runtime/basicLock.cpp index 7f2cc856e58..384fc493e0b 100644 --- a/src/hotspot/share/runtime/basicLock.cpp +++ b/src/hotspot/share/runtime/basicLock.cpp @@ -27,7 +27,7 @@ #include "runtime/objectMonitor.hpp" #include "runtime/synchronizer.hpp" -void BasicLock::print_on(outputStream* st, oop owner) { +void BasicLock::print_on(outputStream* st, oop owner) const { st->print("monitor"); if (UseObjectMonitorTable) { ObjectMonitor* mon = object_monitor_cache(); diff --git a/src/hotspot/share/runtime/basicLock.hpp b/src/hotspot/share/runtime/basicLock.hpp index 6e69dbe8ce9..cbdb65c8ec6 100644 --- a/src/hotspot/share/runtime/basicLock.hpp +++ b/src/hotspot/share/runtime/basicLock.hpp @@ -50,8 +50,6 @@ class BasicLock { static int metadata_offset_in_bytes() { return (int)offset_of(BasicLock, _metadata); } public: - BasicLock() : _metadata(0) {} - // LM_MONITOR void set_bad_metadata_deopt() { set_metadata(badDispHeaderDeopt); } @@ -61,12 +59,12 @@ class BasicLock { static int displaced_header_offset_in_bytes() { return metadata_offset_in_bytes(); } // LM_LIGHTWEIGHT - inline ObjectMonitor* object_monitor_cache(); + inline ObjectMonitor* object_monitor_cache() const; inline void clear_object_monitor_cache(); inline void set_object_monitor_cache(ObjectMonitor* mon); static int object_monitor_cache_offset_in_bytes() { return metadata_offset_in_bytes(); } - void print_on(outputStream* st, oop owner); + void print_on(outputStream* st, oop owner) const; // move a basic lock (used during deoptimization) void move_to(oop obj, BasicLock* dest); diff --git a/src/hotspot/share/runtime/basicLock.inline.hpp b/src/hotspot/share/runtime/basicLock.inline.hpp index 70e15090185..1090241c3e1 100644 --- a/src/hotspot/share/runtime/basicLock.inline.hpp +++ b/src/hotspot/share/runtime/basicLock.inline.hpp @@ -26,7 +26,6 @@ #define SHARE_RUNTIME_BASICLOCK_INLINE_HPP #include "runtime/basicLock.hpp" -#include "runtime/objectMonitor.inline.hpp" inline markWord BasicLock::displaced_header() const { assert(LockingMode == LM_LEGACY, "must be"); @@ -38,15 +37,10 @@ inline void BasicLock::set_displaced_header(markWord header) { Atomic::store(&_metadata, header.value()); } -inline ObjectMonitor* BasicLock::object_monitor_cache() { +inline ObjectMonitor* BasicLock::object_monitor_cache() const { assert(UseObjectMonitorTable, "must be"); #if !defined(ZERO) && (defined(X86) || defined(AARCH64) || defined(RISCV64) || defined(PPC64) || defined(S390)) - ObjectMonitor* monitor = reinterpret_cast(get_metadata()); - if (monitor != nullptr && monitor->is_being_async_deflated()) { - clear_object_monitor_cache(); - return nullptr; - } - return monitor; + return reinterpret_cast(get_metadata()); #else // Other platforms do not make use of the cache yet, // and are not as careful with maintaining the invariant diff --git a/src/hotspot/share/runtime/lightweightSynchronizer.cpp b/src/hotspot/share/runtime/lightweightSynchronizer.cpp index 471a5c00e20..cefd46d900d 100644 --- a/src/hotspot/share/runtime/lightweightSynchronizer.cpp +++ b/src/hotspot/share/runtime/lightweightSynchronizer.cpp @@ -633,7 +633,7 @@ void LightweightSynchronizer::enter_for(Handle obj, BasicLock* lock, JavaThread* } else { do { // It is assumed that enter_for must enter on an object without contention. - monitor = inflate_and_enter(obj(), lock, ObjectSynchronizer::inflate_cause_monitor_enter, locking_thread, current); + monitor = inflate_and_enter(obj(), ObjectSynchronizer::inflate_cause_monitor_enter, locking_thread, current); // But there may still be a race with deflation. } while (monitor == nullptr); } @@ -689,7 +689,7 @@ void LightweightSynchronizer::enter(Handle obj, BasicLock* lock, JavaThread* cur spin_yield.wait(); } - ObjectMonitor* monitor = inflate_and_enter(obj(), lock, ObjectSynchronizer::inflate_cause_monitor_enter, current, current); + ObjectMonitor* monitor = inflate_and_enter(obj(), ObjectSynchronizer::inflate_cause_monitor_enter, current, current); if (monitor != nullptr) { cache_setter.set_monitor(monitor); return; @@ -703,7 +703,7 @@ void LightweightSynchronizer::enter(Handle obj, BasicLock* lock, JavaThread* cur } } -void LightweightSynchronizer::exit(oop object, BasicLock* lock, JavaThread* current) { +void LightweightSynchronizer::exit(oop object, JavaThread* current) { assert(LockingMode == LM_LIGHTWEIGHT, "must be"); assert(current == Thread::current(), "must be"); @@ -738,18 +738,7 @@ void LightweightSynchronizer::exit(oop object, BasicLock* lock, JavaThread* curr assert(mark.has_monitor(), "must be"); // The monitor exists - ObjectMonitor* monitor; - if (UseObjectMonitorTable) { - monitor = lock->object_monitor_cache(); - if (monitor == nullptr) { - monitor = current->om_get_from_monitor_cache(object); - if (monitor == nullptr) { - monitor = get_monitor_from_table(current, object); - } - } - } else { - monitor = ObjectSynchronizer::read_monitor(mark); - } + ObjectMonitor* monitor = ObjectSynchronizer::read_monitor(current, object, mark); if (monitor->has_anonymous_owner()) { assert(current->lock_stack().contains(object), "current must have object on its lock stack"); monitor->set_owner_from_anonymous(current); @@ -988,7 +977,7 @@ ObjectMonitor* LightweightSynchronizer::inflate_fast_locked_object(oop object, O return monitor; } -ObjectMonitor* LightweightSynchronizer::inflate_and_enter(oop object, BasicLock* lock, ObjectSynchronizer::InflateCause cause, JavaThread* locking_thread, JavaThread* current) { +ObjectMonitor* LightweightSynchronizer::inflate_and_enter(oop object, ObjectSynchronizer::InflateCause cause, JavaThread* locking_thread, JavaThread* current) { assert(LockingMode == LM_LIGHTWEIGHT, "only used for lightweight"); VerifyThreadState vts(locking_thread, current); @@ -1014,20 +1003,18 @@ ObjectMonitor* LightweightSynchronizer::inflate_and_enter(oop object, BasicLock* NoSafepointVerifier nsv; + // Lightweight monitors require that hash codes are installed first + ObjectSynchronizer::FastHashCode(locking_thread, object); + // Try to get the monitor from the thread-local cache. // There's no need to use the cache if we are locking // on behalf of another thread. if (current == locking_thread) { - monitor = lock->object_monitor_cache(); - if (monitor == nullptr) { - monitor = current->om_get_from_monitor_cache(object); - } + monitor = current->om_get_from_monitor_cache(object); } // Get or create the monitor if (monitor == nullptr) { - // Lightweight monitors require that hash codes are installed first - ObjectSynchronizer::FastHashCode(locking_thread, object); monitor = get_or_insert_monitor(object, current, cause); } @@ -1175,6 +1162,9 @@ bool LightweightSynchronizer::quick_enter(oop obj, BasicLock* lock, JavaThread* assert(obj != nullptr, "must be"); NoSafepointVerifier nsv; + // If quick_enter succeeds with entering, the cache should be in a valid initialized state. + CacheSetter cache_setter(current, lock); + LockStack& lock_stack = current->lock_stack(); if (lock_stack.is_full()) { // Always go into runtime if the lock stack is full. @@ -1201,28 +1191,17 @@ bool LightweightSynchronizer::quick_enter(oop obj, BasicLock* lock, JavaThread* #endif if (mark.has_monitor()) { - ObjectMonitor* monitor; - if (UseObjectMonitorTable) { - // C2 fast-path may have put the monitor in the cache in the BasicLock. - monitor = lock->object_monitor_cache(); - if (monitor == nullptr) { - // Otherwise look up the monitor in the thread's OMCache. - monitor = current->om_get_from_monitor_cache(obj); - } - } else { - monitor = ObjectSynchronizer::read_monitor(mark); - } + ObjectMonitor* const monitor = UseObjectMonitorTable ? current->om_get_from_monitor_cache(obj) : + ObjectSynchronizer::read_monitor(mark); if (monitor == nullptr) { // Take the slow-path on a cache miss. return false; } - if (UseObjectMonitorTable) { - lock->set_object_monitor_cache(monitor); - } - - if (monitor->spin_enter(current)) { + if (monitor->try_enter(current)) { + // ObjectMonitor enter successful. + cache_setter.set_monitor(monitor); return true; } } diff --git a/src/hotspot/share/runtime/lightweightSynchronizer.hpp b/src/hotspot/share/runtime/lightweightSynchronizer.hpp index b10e639a67c..fdd753e9b9c 100644 --- a/src/hotspot/share/runtime/lightweightSynchronizer.hpp +++ b/src/hotspot/share/runtime/lightweightSynchronizer.hpp @@ -61,12 +61,12 @@ class LightweightSynchronizer : AllStatic { public: static void enter_for(Handle obj, BasicLock* lock, JavaThread* locking_thread); static void enter(Handle obj, BasicLock* lock, JavaThread* current); - static void exit(oop object, BasicLock* lock, JavaThread* current); + static void exit(oop object, JavaThread* current); static ObjectMonitor* inflate_into_object_header(oop object, ObjectSynchronizer::InflateCause cause, JavaThread* locking_thread, Thread* current); static ObjectMonitor* inflate_locked_or_imse(oop object, ObjectSynchronizer::InflateCause cause, TRAPS); static ObjectMonitor* inflate_fast_locked_object(oop object, ObjectSynchronizer::InflateCause cause, JavaThread* locking_thread, JavaThread* current); - static ObjectMonitor* inflate_and_enter(oop object, BasicLock* lock, ObjectSynchronizer::InflateCause cause, JavaThread* locking_thread, JavaThread* current); + static ObjectMonitor* inflate_and_enter(oop object, ObjectSynchronizer::InflateCause cause, JavaThread* locking_thread, JavaThread* current); static void deflate_monitor(Thread* current, oop obj, ObjectMonitor* monitor); diff --git a/src/hotspot/share/runtime/objectMonitor.hpp b/src/hotspot/share/runtime/objectMonitor.hpp index d24201fafe4..54662ffab7b 100644 --- a/src/hotspot/share/runtime/objectMonitor.hpp +++ b/src/hotspot/share/runtime/objectMonitor.hpp @@ -148,7 +148,6 @@ class ObjectWaiter : public CHeapObj { #define OM_CACHE_LINE_SIZE DEFAULT_CACHE_LINE_SIZE class ObjectMonitor : public CHeapObj { - friend class LightweightSynchronizer; friend class ObjectSynchronizer; friend class ObjectWaiter; friend class VMStructs; diff --git a/src/hotspot/share/runtime/synchronizer.cpp b/src/hotspot/share/runtime/synchronizer.cpp index 73dd9d5330e..3cd708e9718 100644 --- a/src/hotspot/share/runtime/synchronizer.cpp +++ b/src/hotspot/share/runtime/synchronizer.cpp @@ -678,8 +678,7 @@ void ObjectSynchronizer::jni_enter(Handle obj, JavaThread* current) { ObjectMonitor* monitor; bool entered; if (LockingMode == LM_LIGHTWEIGHT) { - BasicLock lock; - entered = LightweightSynchronizer::inflate_and_enter(obj(), &lock, inflate_cause_jni_enter, current, current) != nullptr; + entered = LightweightSynchronizer::inflate_and_enter(obj(), inflate_cause_jni_enter, current, current) != nullptr; } else { monitor = inflate(current, obj(), inflate_cause_jni_enter); entered = monitor->enter(current); diff --git a/src/hotspot/share/runtime/synchronizer.inline.hpp b/src/hotspot/share/runtime/synchronizer.inline.hpp index ac9116088f7..53a9f99a39e 100644 --- a/src/hotspot/share/runtime/synchronizer.inline.hpp +++ b/src/hotspot/share/runtime/synchronizer.inline.hpp @@ -72,7 +72,7 @@ inline void ObjectSynchronizer::exit(oop object, BasicLock* lock, JavaThread* cu current->dec_held_monitor_count(); if (LockingMode == LM_LIGHTWEIGHT) { - LightweightSynchronizer::exit(object, lock, current); + LightweightSynchronizer::exit(object, current); } else { exit_legacy(object, lock, current); }