From 008e9ffd3002430174cbdaa4aac9355dd8b40c15 Mon Sep 17 00:00:00 2001 From: Fredrik Bredberg Date: Mon, 8 Jan 2024 13:32:17 +0000 Subject: [PATCH] 8320317: ObjectMonitor NotRunnable is not really an optimization Reviewed-by: eosterlund, dholmes, shade, dcubed JBR-6819 Backport 8320317 --- src/hotspot/share/runtime/objectMonitor.cpp | 68 --------------------- src/hotspot/share/runtime/objectMonitor.hpp | 1 - 2 files changed, 69 deletions(-) diff --git a/src/hotspot/share/runtime/objectMonitor.cpp b/src/hotspot/share/runtime/objectMonitor.cpp index 374816fd355e..ba5c88b94daa 100644 --- a/src/hotspot/share/runtime/objectMonitor.cpp +++ b/src/hotspot/share/runtime/objectMonitor.cpp @@ -1877,10 +1877,6 @@ int ObjectMonitor::TrySpin(JavaThread* current) { ctr = _SpinDuration; if (ctr <= 0) return 0; - if (NotRunnable(current, (JavaThread*) owner_raw())) { - return 0; - } - // We're good to spin ... spin ingress. // CONSIDER: use Prefetch::write() to avoid RTS->RTO upgrades // when preparing to LD...CAS _owner, etc and the CAS is likely @@ -1963,13 +1959,6 @@ int ObjectMonitor::TrySpin(JavaThread* current) { } prv = ox; - // Abort the spin if the owner is not executing. - // The owner must be executing in order to drop the lock. - // Spinning while the owner is OFFPROC is idiocy. - // Consider: ctr -= RunnablePenalty ; - if (NotRunnable(current, ox)) { - goto Abort; - } if (_succ == NULL) { _succ = current; } @@ -2002,63 +1991,6 @@ int ObjectMonitor::TrySpin(JavaThread* current) { return 0; } -// NotRunnable() -- informed spinning -// -// Don't bother spinning if the owner is not eligible to drop the lock. -// Spin only if the owner thread is _thread_in_Java or _thread_in_vm. -// The thread must be runnable in order to drop the lock in timely fashion. -// If the _owner is not runnable then spinning will not likely be -// successful (profitable). -// -// Beware -- the thread referenced by _owner could have died -// so a simply fetch from _owner->_thread_state might trap. -// Instead, we use SafeFetchXX() to safely LD _owner->_thread_state. -// Because of the lifecycle issues, the _thread_state values -// observed by NotRunnable() might be garbage. NotRunnable must -// tolerate this and consider the observed _thread_state value -// as advisory. -// -// Beware too, that _owner is sometimes a BasicLock address and sometimes -// a thread pointer. -// Alternately, we might tag the type (thread pointer vs basiclock pointer) -// with the LSB of _owner. Another option would be to probabilistically probe -// the putative _owner->TypeTag value. -// -// Checking _thread_state isn't perfect. Even if the thread is -// in_java it might be blocked on a page-fault or have been preempted -// and sitting on a ready/dispatch queue. -// -// The return value from NotRunnable() is *advisory* -- the -// result is based on sampling and is not necessarily coherent. -// The caller must tolerate false-negative and false-positive errors. -// Spinning, in general, is probabilistic anyway. - - -int ObjectMonitor::NotRunnable(JavaThread* current, JavaThread* ox) { - // Check ox->TypeTag == 2BAD. - if (ox == NULL) return 0; - - // Avoid transitive spinning ... - // Say T1 spins or blocks trying to acquire L. T1._Stalled is set to L. - // Immediately after T1 acquires L it's possible that T2, also - // spinning on L, will see L.Owner=T1 and T1._Stalled=L. - // This occurs transiently after T1 acquired L but before - // T1 managed to clear T1.Stalled. T2 does not need to abort - // its spin in this circumstance. - intptr_t BlockedOn = SafeFetchN((intptr_t *) &ox->_Stalled, intptr_t(1)); - - if (BlockedOn == 1) return 1; - if (BlockedOn != 0) { - return BlockedOn != intptr_t(this) && owner_raw() == ox; - } - - assert(sizeof(ox->_thread_state == sizeof(int)), "invariant"); - int jst = SafeFetch32((int *) &ox->_thread_state, -1);; - // consider also: jst != _thread_in_Java -- but that's overspecific. - return jst == _thread_blocked || jst == _thread_in_native; -} - - // ----------------------------------------------------------------------------- // WaitSet management ... diff --git a/src/hotspot/share/runtime/objectMonitor.hpp b/src/hotspot/share/runtime/objectMonitor.hpp index da3a4b098279..693dd94f4082 100644 --- a/src/hotspot/share/runtime/objectMonitor.hpp +++ b/src/hotspot/share/runtime/objectMonitor.hpp @@ -340,7 +340,6 @@ class ObjectMonitor : public CHeapObj { void ReenterI(JavaThread* current, ObjectWaiter* current_node); void UnlinkAfterAcquire(JavaThread* current, ObjectWaiter* current_node); int TryLock(JavaThread* current); - int NotRunnable(JavaThread* current, JavaThread* Owner); int TrySpin(JavaThread* current); void ExitEpilog(JavaThread* current, ObjectWaiter* Wakee);