--- old/src/hotspot/share/runtime/synchronizer.cpp 2019-05-02 15:04:28.054393008 -0400 +++ new/src/hotspot/share/runtime/synchronizer.cpp 2019-05-02 15:04:27.594393024 -0400 @@ -350,36 +350,33 @@ // We don't need to use fast path here, because it must have been // failed in the interpreter/compiler code. void ObjectSynchronizer::slow_enter(Handle obj, BasicLock* lock, TRAPS) { - bool do_loop = true; - while (do_loop) { - markOop mark = obj->mark(); - assert(!mark->has_bias_pattern(), "should not see bias pattern here"); + markOop mark = obj->mark(); + assert(!mark->has_bias_pattern(), "should not see bias pattern here"); - if (mark->is_neutral()) { - // Anticipate successful CAS -- the ST of the displaced mark must - // be visible <= the ST performed by the CAS. - lock->set_displaced_header(mark); - if (mark == obj()->cas_set_mark((markOop) lock, mark)) { - return; - } - // Fall through to inflate() ... - } else if (mark->has_locker() && - THREAD->is_lock_owned((address)mark->locker())) { - assert(lock != mark->locker(), "must not re-lock the same lock"); - assert(lock != (BasicLock*)obj->mark(), "don't relock with same BasicLock"); - lock->set_displaced_header(NULL); + if (mark->is_neutral()) { + // Anticipate successful CAS -- the ST of the displaced mark must + // be visible <= the ST performed by the CAS. + lock->set_displaced_header(mark); + if (mark == obj()->cas_set_mark((markOop) lock, mark)) { return; } - - // The object header will never be displaced to this lock, - // so it does not matter what the value is, except that it - // must be non-zero to avoid looking like a re-entrant lock, - // and must not look locked either. - lock->set_displaced_header(markOopDesc::unused_mark()); - ObjectMonitorHandle omh; - inflate(&omh, THREAD, obj(), inflate_cause_monitor_enter); - do_loop = !omh.om_ptr()->enter(THREAD); + // Fall through to inflate() ... + } else if (mark->has_locker() && + THREAD->is_lock_owned((address)mark->locker())) { + assert(lock != mark->locker(), "must not re-lock the same lock"); + assert(lock != (BasicLock*)obj->mark(), "don't relock with same BasicLock"); + lock->set_displaced_header(NULL); + return; } + + // The object header will never be displaced to this lock, + // so it does not matter what the value is, except that it + // must be non-zero to avoid looking like a re-entrant lock, + // and must not look locked either. + lock->set_displaced_header(markOopDesc::unused_mark()); + ObjectMonitorHandle omh; + inflate(&omh, THREAD, obj(), inflate_cause_monitor_enter); + omh.om_ptr()->enter(THREAD); } // This routine is used to handle interpreter/compiler slow case @@ -421,12 +418,9 @@ assert(!obj->mark()->has_bias_pattern(), "biases should be revoked by now"); } - bool do_loop = true; - while (do_loop) { - ObjectMonitorHandle omh; - inflate(&omh, THREAD, obj(), inflate_cause_vm_internal); - do_loop = !omh.om_ptr()->reenter(recursion, THREAD); - } + ObjectMonitorHandle omh; + inflate(&omh, THREAD, obj(), inflate_cause_vm_internal); + omh.om_ptr()->reenter(recursion, THREAD); } // ----------------------------------------------------------------------------- // JNI locks on java objects @@ -438,12 +432,9 @@ assert(!obj->mark()->has_bias_pattern(), "biases should be revoked by now"); } THREAD->set_current_pending_monitor_is_from_java(false); - bool do_loop = true; - while (do_loop) { - ObjectMonitorHandle omh; - inflate(&omh, THREAD, obj(), inflate_cause_jni_enter); - do_loop = !omh.om_ptr()->enter(THREAD); - } + ObjectMonitorHandle omh; + inflate(&omh, THREAD, obj(), inflate_cause_jni_enter); + omh.om_ptr()->enter(THREAD); THREAD->set_current_pending_monitor_is_from_java(true); } @@ -1174,7 +1165,6 @@ // Clear any values we allowed to linger during async deflation. take->_header = NULL; take->set_owner(NULL); - take->_contentions = 0; if (take->ref_count() < 0) { // Add back max_jint to restore the ref_count field to its @@ -1782,14 +1772,14 @@ // Returns true if it was deflated and false otherwise. // // The async deflation protocol sets owner to DEFLATER_MARKER and -// makes contentions negative as signals to contending threads that +// makes ref_count negative as signals to contending threads that // an async deflation is in progress. There are a number of checks // as part of the protocol to make sure that the calling thread has // not lost the race to a contending thread or to a thread that just // wants to use the ObjectMonitor*. // // The ObjectMonitor has been successfully async deflated when: -// (owner == DEFLATER_MARKER && contentions < 0 && ref_count < 0). +// (owner == DEFLATER_MARKER && ref_count < 0) // Contending threads or ObjectMonitor* using threads that see those // values know to retry their operation. // @@ -1815,41 +1805,33 @@ // the slow path. This is just the first part of the async // deflation dance. - if (mid->_waiters != 0 || mid->ref_count() != 0) { + if (mid->_contentions != 0 || mid->_waiters != 0) { // Another thread has raced to enter the ObjectMonitor after - // mid->is_busy() above and has already waited on it which - // makes it busy so no deflation. Or the ObjectMonitor* is - // in use for some other operation like inflate(). Restore - // owner to NULL if it is still DEFLATER_MARKER. + // mid->is_busy() above or has already entered and waited on + // it which makes it busy so no deflation. Restore owner to + // NULL if it is still DEFLATER_MARKER. Atomic::cmpxchg((void*)NULL, &mid->_owner, DEFLATER_MARKER); return false; } - if (Atomic::cmpxchg(-max_jint, &mid->_contentions, (jint)0) == 0) { - // Make contentions negative to force any contending threads to - // retry. This is the second part of the async deflation dance. + if (Atomic::cmpxchg(-max_jint, &mid->_ref_count, (jint)0) == 0) { + // Make ref_count negative to force any contending threads or + // ObjectMonitor* using threads to retry. This is the second + // part of the async deflation dance. - if (mid->_owner == DEFLATER_MARKER && - Atomic::cmpxchg(-max_jint, &mid->_ref_count, (jint)0) == 0) { + if (mid->_owner == DEFLATER_MARKER) { // If owner is still DEFLATER_MARKER, then we have successfully // signaled any contending threads to retry. If it is not, then we // have lost the race to an entering thread and the ObjectMonitor - // is now busy. If we cannot make ref_count negative (because the - // ObjectMonitor* is in use), then we have lost that race instead. - // This is the third and final part of the async deflation dance. - // Note: This owner check solves the ABA problem with contentions + // is now busy. This is the third and final part of the async + // deflation dance. + // Note: This owner check solves the ABA problem with ref_count // where another thread acquired the ObjectMonitor, finished - // using it and restored the contentions to zero. - // Note: Making ref_count negative solves the race with - // ObjectMonitor::save_om_ptr() where its ref_count increment - // happens after the first ref_count check in this function. - // Note: Making ref_count negative must happen after the third - // part check of "owner == DEFLATER_MARKER". When save_om_ptr() - // retries, it will call install_displaced_markword_in_object() - // which will disconnect the object from the ObjectMonitor so - // deflation must happen. + // using it and restored the ref_count to zero. // Sanity checks for the races: + guarantee(mid->_contentions == 0, "must be 0: contentions=%d", + mid->_contentions); guarantee(mid->_waiters == 0, "must be 0: waiters=%d", mid->_waiters); guarantee(mid->_cxq == NULL, "must be no contending threads: cxq=" INTPTR_FORMAT, p2i(mid->_cxq)); @@ -1898,29 +1880,27 @@ // refers to this ObjectMonitor. Those linkages have to be // cleaned up by the caller who has the complete context. - // We leave owner == DEFLATER_MARKER and contentions < 0 + // We leave owner == DEFLATER_MARKER and ref_count < 0 // to force any racing threads to retry. return true; // Success, ObjectMonitor has been deflated. } - // The owner was changed from DEFLATER_MARKER or ObjectMonitor* - // is in use so we lost the race since the ObjectMonitor is now - // busy. - - // Restore owner to NULL if it is still DEFLATER_MARKER: - Atomic::cmpxchg((void*)NULL, &mid->_owner, DEFLATER_MARKER); + // The owner was changed from DEFLATER_MARKER so we lost the + // race since the ObjectMonitor is now busy. - // Add back max_jint to restore the contentions field to its + // Add back max_jint to restore the ref_count field to its // proper value (which may not be what we saw above): - Atomic::add(max_jint, &mid->_contentions); + Atomic::add(max_jint, &mid->_ref_count); - assert(mid->_contentions >= 0, "must not be negative: contentions=%d", - mid->_contentions); + assert(mid->ref_count() >= 0, "must not be negative: ref_count=%d", + mid->ref_count()); + return false; } - // The contentions was no longer 0 so we lost the race since the - // ObjectMonitor is now busy. - assert(mid->_owner != DEFLATER_MARKER, "must not be DEFLATER_MARKER"); + // The ref_count was no longer 0 so we lost the race since the + // ObjectMonitor is now busy or the ObjectMonitor* is now is use. + // Restore owner to NULL if it is still DEFLATER_MARKER: + Atomic::cmpxchg((void*)NULL, &mid->_owner, DEFLATER_MARKER); } // The owner field is no longer NULL so we lost the race since the