From 9ead2b75cefa42732d3445f086dcf8d51452af2c Mon Sep 17 00:00:00 2001 From: Coleen Phillimore Date: Fri, 11 Apr 2025 13:12:16 +0000 Subject: [PATCH] 8354180: Clean up uses of ObjectMonitor caches Co-authored-by: Axel Boldt-Christmas Reviewed-by: aboldtch, fbredberg --- src/hotspot/share/runtime/basicLock.cpp | 2 +- src/hotspot/share/runtime/basicLock.hpp | 6 +-- .../share/runtime/basicLock.inline.hpp | 11 ++-- src/hotspot/share/runtime/deoptimization.cpp | 3 ++ .../share/runtime/lightweightSynchronizer.cpp | 50 +++++++++++-------- src/hotspot/share/runtime/objectMonitor.cpp | 2 +- 6 files changed, 41 insertions(+), 33 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..50de8db6bc6 100644 --- a/src/hotspot/share/runtime/basicLock.hpp +++ b/src/hotspot/share/runtime/basicLock.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1998, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1998, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -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(); + 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..ee4791edecf 100644 --- a/src/hotspot/share/runtime/basicLock.inline.hpp +++ b/src/hotspot/share/runtime/basicLock.inline.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2024, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -38,15 +38,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/deoptimization.cpp b/src/hotspot/share/runtime/deoptimization.cpp index b27d6c89e91..5e4aaf31a3b 100644 --- a/src/hotspot/share/runtime/deoptimization.cpp +++ b/src/hotspot/share/runtime/deoptimization.cpp @@ -1666,6 +1666,9 @@ bool Deoptimization::relock_objects(JavaThread* thread, GrowableArrayclear_object_monitor_cache(); } ObjectSynchronizer::enter_for(obj, lock, deoptee_thread); diff --git a/src/hotspot/share/runtime/lightweightSynchronizer.cpp b/src/hotspot/share/runtime/lightweightSynchronizer.cpp index c2a4e73f034..423ca990abd 100644 --- a/src/hotspot/share/runtime/lightweightSynchronizer.cpp +++ b/src/hotspot/share/runtime/lightweightSynchronizer.cpp @@ -519,8 +519,12 @@ class LightweightSynchronizer::CacheSetter : StackObj { // Only use the cache if using the table. if (UseObjectMonitorTable) { if (_monitor != nullptr) { - _thread->om_set_monitor_cache(_monitor); - _lock->set_object_monitor_cache(_monitor); + // If the monitor is already in the BasicLock cache then it is most + // likely in the thread cache, do not set it again to avoid reordering. + if (_monitor != _lock->object_monitor_cache()) { + _thread->om_set_monitor_cache(_monitor); + _lock->set_object_monitor_cache(_monitor); + } } else { _lock->clear_object_monitor_cache(); } @@ -534,6 +538,16 @@ class LightweightSynchronizer::CacheSetter : StackObj { }; +// Reads first from the BasicLock cache then from the OMCache in the current thread. +// C2 fast-path may have put the monitor in the cache in the BasicLock. +inline static ObjectMonitor* read_caches(JavaThread* current, BasicLock* lock, oop object) { + ObjectMonitor* monitor = lock->object_monitor_cache(); + if (monitor == nullptr) { + monitor = current->om_get_from_monitor_cache(object); + } + return monitor; +} + class LightweightSynchronizer::VerifyThreadState { bool _no_safepoint; @@ -615,6 +629,7 @@ bool LightweightSynchronizer::fast_lock_spin_enter(oop obj, LockStack& lock_stac void LightweightSynchronizer::enter_for(Handle obj, BasicLock* lock, JavaThread* locking_thread) { assert(LockingMode == LM_LIGHTWEIGHT, "must be"); + assert(!UseObjectMonitorTable || lock->object_monitor_cache() == nullptr, "must be cleared"); JavaThread* current = JavaThread::current(); VerifyThreadState vts(locking_thread, current); @@ -622,8 +637,6 @@ void LightweightSynchronizer::enter_for(Handle obj, BasicLock* lock, JavaThread* ObjectSynchronizer::handle_sync_on_value_based_class(obj, locking_thread); } - CacheSetter cache_setter(locking_thread, lock); - LockStack& lock_stack = locking_thread->lock_stack(); ObjectMonitor* monitor = nullptr; @@ -640,7 +653,7 @@ void LightweightSynchronizer::enter_for(Handle obj, BasicLock* lock, JavaThread* } assert(monitor != nullptr, "LightweightSynchronizer::enter_for must succeed"); - cache_setter.set_monitor(monitor); + assert(!UseObjectMonitorTable || lock->object_monitor_cache() == nullptr, "unused. already cleared"); } void LightweightSynchronizer::enter(Handle obj, BasicLock* lock, JavaThread* current) { @@ -741,12 +754,9 @@ void LightweightSynchronizer::exit(oop object, BasicLock* lock, JavaThread* curr // The monitor exists ObjectMonitor* monitor; if (UseObjectMonitorTable) { - monitor = lock->object_monitor_cache(); + monitor = read_caches(current, lock, object); if (monitor == nullptr) { - monitor = current->om_get_from_monitor_cache(object); - if (monitor == nullptr) { - monitor = get_monitor_from_table(current, object); - } + monitor = get_monitor_from_table(current, object); } } else { monitor = ObjectSynchronizer::read_monitor(mark); @@ -1019,10 +1029,7 @@ ObjectMonitor* LightweightSynchronizer::inflate_and_enter(oop object, BasicLock* // 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 = read_caches(current, lock, object); } // Get or create the monitor @@ -1044,6 +1051,9 @@ ObjectMonitor* LightweightSynchronizer::inflate_and_enter(oop object, BasicLock* // The MonitorDeflation thread is deflating the monitor. The locking thread // must spin until further progress has been made. + // Clear the BasicLock cache as it may contain this monitor. + lock->clear_object_monitor_cache(); + const markWord mark = object->mark_acquire(); if (mark.has_monitor()) { @@ -1204,12 +1214,7 @@ bool LightweightSynchronizer::quick_enter(oop obj, BasicLock* lock, JavaThread* 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); - } + monitor = read_caches(current, lock, obj); } else { monitor = ObjectSynchronizer::read_monitor(mark); } @@ -1220,6 +1225,11 @@ bool LightweightSynchronizer::quick_enter(oop obj, BasicLock* lock, JavaThread* } if (UseObjectMonitorTable) { + // Set the monitor regardless of success. + // Either we successfully lock on the monitor, or we retry with the + // monitor in the slow path. If the monitor gets deflated, it will be + // cleared, either by the CacheSetter if we fast lock in enter or in + // inflate_and_enter when we see that the monitor is deflated. lock->set_object_monitor_cache(monitor); } diff --git a/src/hotspot/share/runtime/objectMonitor.cpp b/src/hotspot/share/runtime/objectMonitor.cpp index 03228d0cb12..48e068714f5 100644 --- a/src/hotspot/share/runtime/objectMonitor.cpp +++ b/src/hotspot/share/runtime/objectMonitor.cpp @@ -2571,7 +2571,7 @@ void ObjectMonitor::print_on(outputStream* st) const { st->print("{contentions=0x%08x,waiters=0x%08x" ",recursions=%zd,owner=" INT64_FORMAT "}", contentions(), waiters(), recursions(), - owner()); + owner_raw()); } void ObjectMonitor::print() const { print_on(tty); }