--- old/src/hotspot/share/runtime/objectMonitor.cpp 2019-05-02 15:04:25.010393114 -0400 +++ new/src/hotspot/share/runtime/objectMonitor.cpp 2019-05-02 15:04:24.562393130 -0400 @@ -238,7 +238,9 @@ // ----------------------------------------------------------------------------- // Enter support -bool ObjectMonitor::enter(TRAPS) { +void ObjectMonitor::enter(TRAPS) { + ADIM_guarantee(_ref_count > 0, "must be positive: ref_count=%d", _ref_count); + // The following code is ordered to check the most common cases first // and to reduce RTS->RTO cache line upgrades on SPARC and IA32 processors. Thread * const Self = THREAD; @@ -248,13 +250,13 @@ // Either ASSERT _recursions == 0 or explicitly set _recursions = 0. assert(_recursions == 0, "invariant"); assert(_owner == Self, "invariant"); - return true; + return; } if (cur == Self) { // TODO-FIXME: check for integer overflow! BUGID 6557169. _recursions++; - return true; + return; } if (Self->is_lock_owned ((address)cur)) { @@ -263,7 +265,7 @@ // Commute owner from a thread-specific on-stack BasicLockObject address to // a full-fledged "Thread *". _owner = Self; - return true; + return; } // We've encountered genuine contention. @@ -284,7 +286,7 @@ ", encoded this=" INTPTR_FORMAT, p2i(((oop)object())->mark()), p2i(markOopDesc::encode(this))); Self->_Stalled = 0; - return true; + return; } assert(_owner != Self, "invariant"); @@ -294,21 +296,13 @@ assert(!SafepointSynchronize::is_at_safepoint(), "invariant"); assert(jt->thread_state() != _thread_blocked, "invariant"); assert(AsyncDeflateIdleMonitors || this->object() != NULL, "invariant"); - assert(AsyncDeflateIdleMonitors || _contentions >= 0, "invariant"); + assert(_contentions >= 0, "must not be negative: contentions=%d", _contentions); - // Prevent deflation. See ObjectSynchronizer::deflate_monitor() and is_busy(). - // Ensure the object-monitor relationship remains stable while there's contention. - const jint contentions = Atomic::add(1, &_contentions); - if (contentions <= 0 && _owner == DEFLATER_MARKER) { - // Async deflation is in progress. Attempt to restore the - // header/dmw to the object's header so that we only retry once - // if the deflater thread happens to be slow. - const oop obj = (oop) object(); - install_displaced_markword_in_object(obj); - Self->_Stalled = 0; - return false; // Caller should retry. Never mind about _contentions as this monitor has been deflated. - } - // The deflater thread will not deflate this monitor and the monitor is contended, continue. + // Prevent deflation. See ObjectSynchronizer::deflate_monitor(), + // ObjectSynchronizer::deflate_monitor_using_JT() and is_busy(). + // Ensure the object <-> monitor relationship remains stable while + // there's contention. + Atomic::add(1, &_contentions); JFR_ONLY(JfrConditionalFlushWithStacktrace flush(jt);) EventJavaMonitorEnter event; @@ -370,7 +364,7 @@ } Atomic::dec(&_contentions); - assert(AsyncDeflateIdleMonitors || _contentions >= 0, "invariant"); + assert(_contentions >= 0, "must not be negative: contentions=%d", _contentions); Self->_Stalled = 0; // Must either set _recursions = 0 or ASSERT _recursions == 0. @@ -406,7 +400,6 @@ event.commit(); } OM_PERFDATA_OP(ContendedLockAttempts, inc()); - return true; } // Caveat: TryLock() is not necessarily serializing if it returns failure. @@ -435,7 +428,7 @@ // deflation process. void ObjectMonitor::install_displaced_markword_in_object(const oop obj) { // This function must only be called when (owner == DEFLATER_MARKER - // && contentions <= 0), but we can't guarantee that here because + // && ref_count <= 0), but we can't guarantee that here because // those values could change when the ObjectMonitor gets moved from // the global free list to a per-thread free list. @@ -510,6 +503,8 @@ #define MAX_RECHECK_INTERVAL 1000 void ObjectMonitor::EnterI(TRAPS) { + ADIM_guarantee(_ref_count > 0, "must be positive: ref_count=%d", _ref_count); + Thread * const Self = THREAD; assert(Self->is_Java_thread(), "invariant"); assert(((JavaThread *) Self)->thread_state() == _thread_blocked, "invariant"); @@ -523,12 +518,8 @@ } if (_owner == DEFLATER_MARKER) { - // The deflation protocol finished the first part (setting _owner), but - // it failed the second part (making _contentions negative) and bailed. - // Because we're called from enter() we have at least one contention. - guarantee(_contentions > 0, "owner == DEFLATER_MARKER && contentions <= 0 " - "should have been handled by the caller: contentions=%d", - _contentions); + // The deflation protocol finished the first part (setting owner), but + // it failed the second part (making ref_count negative) and bailed. if (Atomic::cmpxchg(Self, &_owner, DEFLATER_MARKER) == DEFLATER_MARKER) { // Acquired the monitor. assert(_succ != Self, "invariant"); @@ -654,12 +645,8 @@ if (TryLock(Self) > 0) break; if (_owner == DEFLATER_MARKER) { - // The deflation protocol finished the first part (setting _owner), but - // it failed the second part (making _contentions negative) and bailed. - // Because we're called from enter() we have at least one contention. - guarantee(_contentions > 0, "owner == DEFLATER_MARKER && contentions <= 0 " - "should have been handled by the caller: contentions=%d", - _contentions); + // The deflation protocol finished the first part (setting owner), but + // it failed the second part (making ref_count negative) and bailed. if (Atomic::cmpxchg(Self, &_owner, DEFLATER_MARKER) == DEFLATER_MARKER) { // Acquired the monitor. break; @@ -770,6 +757,8 @@ // In the future we should reconcile EnterI() and ReenterI(). void ObjectMonitor::ReenterI(Thread * Self, ObjectWaiter * SelfNode) { + ADIM_guarantee(_ref_count > 0, "must be positive: ref_count=%d", _ref_count); + assert(Self != NULL, "invariant"); assert(SelfNode != NULL, "invariant"); assert(SelfNode->_thread == Self, "invariant"); @@ -788,12 +777,8 @@ if (TrySpin(Self) > 0) break; if (_owner == DEFLATER_MARKER) { - // The deflation protocol finished the first part (setting _owner), - // but it will observe _waiters != 0 and will bail out. Because we're - // called from wait() we may or may not have any contentions. - guarantee(_contentions >= 0, "owner == DEFLATER_MARKER && contentions < 0 " - "should have been handled by the caller: contentions=%d", - _contentions); + // The deflation protocol finished the first part (setting owner), but + // it failed the second part (making ref_count negative) and bailed. if (Atomic::cmpxchg(Self, &_owner, DEFLATER_MARKER) == DEFLATER_MARKER) { // Acquired the monitor. break; @@ -1258,20 +1243,16 @@ // reenter() enters a lock and sets recursion count // complete_exit/reenter operate as a wait without waiting -bool ObjectMonitor::reenter(intptr_t recursions, TRAPS) { +void ObjectMonitor::reenter(intptr_t recursions, TRAPS) { Thread * const Self = THREAD; assert(Self->is_Java_thread(), "Must be Java thread!"); JavaThread *jt = (JavaThread *)THREAD; guarantee(_owner != Self, "reenter already owner"); - if (!enter(THREAD)) { - // Failed to enter the monitor so return for a retry. - return false; - } + enter(THREAD); // Entered the monitor. guarantee(_recursions == 0, "reenter recursion"); _recursions = recursions; - return true; } @@ -1499,8 +1480,7 @@ assert(_owner != Self, "invariant"); ObjectWaiter::TStates v = node.TState; if (v == ObjectWaiter::TS_RUN) { - const bool success = enter(Self); - ADIM_guarantee(success, "enter signaled for a retry, but monitor should not have been deflated as waiters > 0"); + enter(Self); } else { guarantee(v == ObjectWaiter::TS_ENTER || v == ObjectWaiter::TS_CXQ, "invariant"); ReenterI(Self, &node); @@ -2064,7 +2044,7 @@ DEBUG_ONLY(InitDone = true;) } -// For internal used by ObjectSynchronizer::monitors_iterate(). +// For internal use by ObjectSynchronizer::monitors_iterate(). ObjectMonitorHandle::ObjectMonitorHandle(ObjectMonitor * om_ptr) { om_ptr->inc_ref_count(); _om_ptr = om_ptr; @@ -2098,17 +2078,14 @@ // Race here if monitor is not owned! The above ref_count bump // will cause subsequent async deflation to skip it. However, // previous or concurrent async deflation is a race. - if (om_ptr->_owner == DEFLATER_MARKER && om_ptr->_contentions <= 0) { - // Async deflation is in progress. - if (om_ptr->ref_count() <= 0) { - // And our ref_count increment above lost the race to async - // deflation. Attempt to restore the header/dmw to the - // object's header so that we only retry once if the deflater - // thread happens to be slow. - om_ptr->install_displaced_markword_in_object(object); - om_ptr->dec_ref_count(); - return false; - } + if (om_ptr->_owner == DEFLATER_MARKER && om_ptr->ref_count() <= 0) { + // Async deflation is in progress and our ref_count increment + // above lost the race to async deflation. Attempt to restore + // the header/dmw to the object's header so that we only retry + // once if the deflater thread happens to be slow. + om_ptr->install_displaced_markword_in_object(object); + om_ptr->dec_ref_count(); + return false; } // The ObjectMonitor could have been deflated and reused for // another object before we bumped the ref_count so make sure @@ -2122,8 +2099,8 @@ } } - guarantee(_om_ptr == NULL, "sanity check: _om_ptr=" INTPTR_FORMAT, - p2i(_om_ptr)); + ADIM_guarantee(_om_ptr == NULL, "sanity check: _om_ptr=" INTPTR_FORMAT, + p2i(_om_ptr)); _om_ptr = om_ptr; return true; } @@ -2131,11 +2108,11 @@ // For internal use by ObjectSynchronizer::inflate(). void ObjectMonitorHandle::set_om_ptr(ObjectMonitor * om_ptr) { if (_om_ptr == NULL) { - guarantee(om_ptr != NULL, "cannot clear an unset om_ptr"); + ADIM_guarantee(om_ptr != NULL, "cannot clear an unset om_ptr"); om_ptr->inc_ref_count(); _om_ptr = om_ptr; } else { - guarantee(om_ptr == NULL, "can only clear a set om_ptr"); + ADIM_guarantee(om_ptr == NULL, "can only clear a set om_ptr"); _om_ptr->dec_ref_count(); _om_ptr = NULL; }