From 32235be4ddc2162c6b2ae94467ed88a25231cfc6 Mon Sep 17 00:00:00 2001 From: Roman Kennke Date: Tue, 18 Mar 2025 12:40:39 +0000 Subject: [PATCH 1/5] 8339114: DaCapo xalan performance with -XX:+UseObjectMonitorTable --- src/hotspot/share/runtime/basicLock.hpp | 2 + .../share/runtime/lightweightSynchronizer.cpp | 65 +++++++++++++------ .../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 +- 6 files changed, 53 insertions(+), 24 deletions(-) diff --git a/src/hotspot/share/runtime/basicLock.hpp b/src/hotspot/share/runtime/basicLock.hpp index cbdb65c8ec6a2..4329129ce78c9 100644 --- a/src/hotspot/share/runtime/basicLock.hpp +++ b/src/hotspot/share/runtime/basicLock.hpp @@ -50,6 +50,8 @@ 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); } diff --git a/src/hotspot/share/runtime/lightweightSynchronizer.cpp b/src/hotspot/share/runtime/lightweightSynchronizer.cpp index 44d0cb32b5693..7673f23575145 100644 --- a/src/hotspot/share/runtime/lightweightSynchronizer.cpp +++ b/src/hotspot/share/runtime/lightweightSynchronizer.cpp @@ -509,6 +509,7 @@ class LightweightSynchronizer::CacheSetter : StackObj { JavaThread* const _thread; BasicLock* const _lock; ObjectMonitor* _monitor; + bool _hit; NONCOPYABLE(CacheSetter); @@ -516,13 +517,15 @@ class LightweightSynchronizer::CacheSetter : StackObj { CacheSetter(JavaThread* thread, BasicLock* lock) : _thread(thread), _lock(lock), - _monitor(nullptr) {} + _monitor(nullptr), _hit(false) {} ~CacheSetter() { // Only use the cache if using the table. if (UseObjectMonitorTable) { if (_monitor != nullptr) { - _thread->om_set_monitor_cache(_monitor); + if (!_hit) { + _thread->om_set_monitor_cache(_monitor); + } _lock->set_object_monitor_cache(_monitor); } else { _lock->clear_object_monitor_cache(); @@ -530,8 +533,9 @@ class LightweightSynchronizer::CacheSetter : StackObj { } } - void set_monitor(ObjectMonitor* monitor) { + void set_monitor(ObjectMonitor* monitor, bool hit = false) { assert(_monitor == nullptr, "only set once"); + _hit = hit; _monitor = monitor; } @@ -637,7 +641,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(), ObjectSynchronizer::inflate_cause_monitor_enter, locking_thread, current); + monitor = inflate_and_enter(obj(), lock, ObjectSynchronizer::inflate_cause_monitor_enter, locking_thread, current); // But there may still be a race with deflation. } while (monitor == nullptr); } @@ -693,7 +697,7 @@ void LightweightSynchronizer::enter(Handle obj, BasicLock* lock, JavaThread* cur spin_yield.wait(); } - ObjectMonitor* monitor = inflate_and_enter(obj(), ObjectSynchronizer::inflate_cause_monitor_enter, current, current); + ObjectMonitor* monitor = inflate_and_enter(obj(), lock, ObjectSynchronizer::inflate_cause_monitor_enter, current, current); if (monitor != nullptr) { cache_setter.set_monitor(monitor); return; @@ -707,7 +711,7 @@ void LightweightSynchronizer::enter(Handle obj, BasicLock* lock, JavaThread* cur } } -void LightweightSynchronizer::exit(oop object, JavaThread* current) { +void LightweightSynchronizer::exit(oop object, BasicLock* lock, JavaThread* current) { assert(LockingMode == LM_LIGHTWEIGHT, "must be"); assert(current == Thread::current(), "must be"); @@ -742,7 +746,18 @@ void LightweightSynchronizer::exit(oop object, JavaThread* current) { assert(mark.has_monitor(), "must be"); // The monitor exists - ObjectMonitor* monitor = ObjectSynchronizer::read_monitor(current, object, mark); + ObjectMonitor* monitor; + if (UseObjectMonitorTable) { + monitor = lock->object_monitor_cache(); + if (monitor == nullptr) { + monitor = JavaThread::cast(current)->om_get_from_monitor_cache(object); + if (monitor == nullptr) { + monitor = get_monitor_from_table(current, object); + } + } + } else { + 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); @@ -987,7 +1002,7 @@ ObjectMonitor* LightweightSynchronizer::inflate_fast_locked_object(oop object, O return monitor; } -ObjectMonitor* LightweightSynchronizer::inflate_and_enter(oop object, ObjectSynchronizer::InflateCause cause, JavaThread* locking_thread, JavaThread* current) { +ObjectMonitor* LightweightSynchronizer::inflate_and_enter(oop object, BasicLock* lock, ObjectSynchronizer::InflateCause cause, JavaThread* locking_thread, JavaThread* current) { assert(LockingMode == LM_LIGHTWEIGHT, "only used for lightweight"); VerifyThreadState vts(locking_thread, current); @@ -1013,18 +1028,20 @@ ObjectMonitor* LightweightSynchronizer::inflate_and_enter(oop object, ObjectSync 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 = current->om_get_from_monitor_cache(object); + monitor = lock->object_monitor_cache(); + if (monitor == nullptr) { + 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); } @@ -1172,9 +1189,6 @@ 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,17 +1215,28 @@ bool LightweightSynchronizer::quick_enter(oop obj, BasicLock* lock, JavaThread* #endif if (mark.has_monitor()) { - ObjectMonitor* const monitor = UseObjectMonitorTable ? current->om_get_from_monitor_cache(obj) : - ObjectSynchronizer::read_monitor(mark); + 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); + } if (monitor == nullptr) { // Take the slow-path on a cache miss. return false; } - if (monitor->try_enter(current)) { - // ObjectMonitor enter successful. - cache_setter.set_monitor(monitor); + if (UseObjectMonitorTable) { + lock->set_object_monitor_cache(monitor); + } + + if (monitor->spin_enter(current)) { return true; } } diff --git a/src/hotspot/share/runtime/lightweightSynchronizer.hpp b/src/hotspot/share/runtime/lightweightSynchronizer.hpp index fdd753e9b9c3f..b10e639a67cf3 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, JavaThread* current); + static void exit(oop object, BasicLock* lock, 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, 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 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 e7f964ee4e594..72d70e5cea185 100644 --- a/src/hotspot/share/runtime/objectMonitor.hpp +++ b/src/hotspot/share/runtime/objectMonitor.hpp @@ -149,6 +149,7 @@ 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 2a97dba7119de..b91612fd98f47 100644 --- a/src/hotspot/share/runtime/synchronizer.cpp +++ b/src/hotspot/share/runtime/synchronizer.cpp @@ -682,7 +682,8 @@ void ObjectSynchronizer::jni_enter(Handle obj, JavaThread* current) { ObjectMonitor* monitor; bool entered; if (LockingMode == LM_LIGHTWEIGHT) { - entered = LightweightSynchronizer::inflate_and_enter(obj(), inflate_cause_jni_enter, current, current) != nullptr; + BasicLock lock; + entered = LightweightSynchronizer::inflate_and_enter(obj(), &lock, 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 53a9f99a39eae..ac9116088f71c 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, current); + LightweightSynchronizer::exit(object, lock, current); } else { exit_legacy(object, lock, current); } From 9a394d296b4a7215a113e6d6c2de6827bbf3c613 Mon Sep 17 00:00:00 2001 From: Roman Kennke Date: Tue, 18 Mar 2025 14:37:06 +0000 Subject: [PATCH 2/5] Remove _hit variable --- src/hotspot/share/runtime/lightweightSynchronizer.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/hotspot/share/runtime/lightweightSynchronizer.cpp b/src/hotspot/share/runtime/lightweightSynchronizer.cpp index 7673f23575145..176b4d07a1d17 100644 --- a/src/hotspot/share/runtime/lightweightSynchronizer.cpp +++ b/src/hotspot/share/runtime/lightweightSynchronizer.cpp @@ -509,7 +509,6 @@ class LightweightSynchronizer::CacheSetter : StackObj { JavaThread* const _thread; BasicLock* const _lock; ObjectMonitor* _monitor; - bool _hit; NONCOPYABLE(CacheSetter); @@ -517,15 +516,13 @@ class LightweightSynchronizer::CacheSetter : StackObj { CacheSetter(JavaThread* thread, BasicLock* lock) : _thread(thread), _lock(lock), - _monitor(nullptr), _hit(false) {} + _monitor(nullptr) {} ~CacheSetter() { // Only use the cache if using the table. if (UseObjectMonitorTable) { if (_monitor != nullptr) { - if (!_hit) { - _thread->om_set_monitor_cache(_monitor); - } + _thread->om_set_monitor_cache(_monitor); _lock->set_object_monitor_cache(_monitor); } else { _lock->clear_object_monitor_cache(); @@ -533,9 +530,8 @@ class LightweightSynchronizer::CacheSetter : StackObj { } } - void set_monitor(ObjectMonitor* monitor, bool hit = false) { + void set_monitor(ObjectMonitor* monitor) { assert(_monitor == nullptr, "only set once"); - _hit = hit; _monitor = monitor; } From 6407833c1a98ddf645f82ff7c3d85f654888835e Mon Sep 17 00:00:00 2001 From: Roman Kennke Date: Wed, 19 Mar 2025 06:51:50 +0000 Subject: [PATCH 3/5] Use read_monitor(mark) --- src/hotspot/share/runtime/lightweightSynchronizer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/runtime/lightweightSynchronizer.cpp b/src/hotspot/share/runtime/lightweightSynchronizer.cpp index 176b4d07a1d17..ecc2085690fb3 100644 --- a/src/hotspot/share/runtime/lightweightSynchronizer.cpp +++ b/src/hotspot/share/runtime/lightweightSynchronizer.cpp @@ -752,7 +752,7 @@ void LightweightSynchronizer::exit(oop object, BasicLock* lock, JavaThread* curr } } } else { - monitor = ObjectSynchronizer::read_monitor(current, object, mark); + monitor = ObjectSynchronizer::read_monitor(mark); } if (monitor->has_anonymous_owner()) { assert(current->lock_stack().contains(object), "current must have object on its lock stack"); From 15a54aa01bde340e38f6d3be47d196f6e986d2e5 Mon Sep 17 00:00:00 2001 From: Roman Kennke Date: Wed, 26 Mar 2025 07:21:18 +0000 Subject: [PATCH 4/5] Clear BasicLock OM cache when monitor is being deflated --- src/hotspot/share/runtime/basicLock.cpp | 2 +- src/hotspot/share/runtime/basicLock.hpp | 4 ++-- src/hotspot/share/runtime/basicLock.inline.hpp | 10 ++++++++-- src/hotspot/share/runtime/lightweightSynchronizer.cpp | 2 +- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/hotspot/share/runtime/basicLock.cpp b/src/hotspot/share/runtime/basicLock.cpp index 384fc493e0b7c..7f2cc856e5888 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) const { +void BasicLock::print_on(outputStream* st, oop owner) { 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 4329129ce78c9..6e69dbe8ce91a 100644 --- a/src/hotspot/share/runtime/basicLock.hpp +++ b/src/hotspot/share/runtime/basicLock.hpp @@ -61,12 +61,12 @@ class BasicLock { static int displaced_header_offset_in_bytes() { return metadata_offset_in_bytes(); } // LM_LIGHTWEIGHT - inline ObjectMonitor* object_monitor_cache() const; + inline ObjectMonitor* object_monitor_cache(); 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) const; + void print_on(outputStream* st, oop owner); // 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 1090241c3e1f0..251ff52872046 100644 --- a/src/hotspot/share/runtime/basicLock.inline.hpp +++ b/src/hotspot/share/runtime/basicLock.inline.hpp @@ -26,6 +26,7 @@ #define SHARE_RUNTIME_BASICLOCK_INLINE_HPP #include "runtime/basicLock.hpp" +#include "runtime/objectMonitor.hpp" inline markWord BasicLock::displaced_header() const { assert(LockingMode == LM_LEGACY, "must be"); @@ -37,10 +38,15 @@ inline void BasicLock::set_displaced_header(markWord header) { Atomic::store(&_metadata, header.value()); } -inline ObjectMonitor* BasicLock::object_monitor_cache() const { +inline ObjectMonitor* BasicLock::object_monitor_cache() { assert(UseObjectMonitorTable, "must be"); #if !defined(ZERO) && (defined(X86) || defined(AARCH64) || defined(RISCV64) || defined(PPC64) || defined(S390)) - return reinterpret_cast(get_metadata()); + ObjectMonitor* monitor = reinterpret_cast(get_metadata()); + if (monitor != nullptr && monitor->is_being_async_deflated()) { + clear_object_monitor_cache(); + return nullptr; + } + return monitor; #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 ecc2085690fb3..908cbe080a3de 100644 --- a/src/hotspot/share/runtime/lightweightSynchronizer.cpp +++ b/src/hotspot/share/runtime/lightweightSynchronizer.cpp @@ -746,7 +746,7 @@ void LightweightSynchronizer::exit(oop object, BasicLock* lock, JavaThread* curr if (UseObjectMonitorTable) { monitor = lock->object_monitor_cache(); if (monitor == nullptr) { - monitor = JavaThread::cast(current)->om_get_from_monitor_cache(object); + monitor = current->om_get_from_monitor_cache(object); if (monitor == nullptr) { monitor = get_monitor_from_table(current, object); } From fc5062a48767646414f9fd7e57a9370c766bda3e Mon Sep 17 00:00:00 2001 From: Roman Kennke Date: Wed, 26 Mar 2025 07:51:09 +0000 Subject: [PATCH 5/5] Include objectMonitor.inline.hpp --- src/hotspot/share/runtime/basicLock.inline.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/runtime/basicLock.inline.hpp b/src/hotspot/share/runtime/basicLock.inline.hpp index 251ff52872046..70e150901850e 100644 --- a/src/hotspot/share/runtime/basicLock.inline.hpp +++ b/src/hotspot/share/runtime/basicLock.inline.hpp @@ -26,7 +26,7 @@ #define SHARE_RUNTIME_BASICLOCK_INLINE_HPP #include "runtime/basicLock.hpp" -#include "runtime/objectMonitor.hpp" +#include "runtime/objectMonitor.inline.hpp" inline markWord BasicLock::displaced_header() const { assert(LockingMode == LM_LEGACY, "must be");