# HG changeset patch # User dcubed # Date 1556115625 14400 # Node ID 04857e2cd5095e3b76eabb031bb00318916b3e69 # Parent c604234be658417bf05cc8736cc76e154f396d25 8222295: more baseline cleanups from Async Monitor Deflation project Reviewed-by: coleenp, acorn, dholmes diff -r c604234be658 -r 04857e2cd509 src/hotspot/share/oops/markOop.cpp --- a/src/hotspot/share/oops/markOop.cpp Wed Apr 24 15:37:55 2019 +0200 +++ b/src/hotspot/share/oops/markOop.cpp Wed Apr 24 10:20:25 2019 -0400 @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2015, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2019, 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 @@ -37,9 +37,9 @@ if (mon == NULL) { st->print("NULL (this should never be seen!)"); } else { - st->print("{count=0x%08x,waiters=0x%08x" + st->print("{contentions=0x%08x,waiters=0x%08x" ",recursions=" INTPTR_FORMAT ",owner=" INTPTR_FORMAT "}", - mon->count(), mon->waiters(), mon->recursions(), + mon->contentions(), mon->waiters(), mon->recursions(), p2i(mon->owner())); } } else if (is_locked()) { diff -r c604234be658 -r 04857e2cd509 src/hotspot/share/runtime/objectMonitor.cpp --- a/src/hotspot/share/runtime/objectMonitor.cpp Wed Apr 24 15:37:55 2019 +0200 +++ b/src/hotspot/share/runtime/objectMonitor.cpp Wed Apr 24 10:20:25 2019 -0400 @@ -276,9 +276,13 @@ // Note that if we acquire the monitor from an initial spin // we forgo posting JVMTI events and firing DTRACE probes. if (TrySpin(Self) > 0) { - assert(_owner == Self, "invariant"); - assert(_recursions == 0, "invariant"); - assert(((oop)(object()))->mark() == markOopDesc::encode(this), "invariant"); + assert(_owner == Self, "must be Self: owner=" INTPTR_FORMAT, p2i(_owner)); + assert(_recursions == 0, "must be 0: recursions=" INTPTR_FORMAT, + _recursions); + assert(((oop)object())->mark() == markOopDesc::encode(this), + "object mark must match encoded this: mark=" INTPTR_FORMAT + ", encoded this=" INTPTR_FORMAT, p2i(((oop)object())->mark()), + p2i(markOopDesc::encode(this))); Self->_Stalled = 0; return; } @@ -290,11 +294,11 @@ assert(!SafepointSynchronize::is_at_safepoint(), "invariant"); assert(jt->thread_state() != _thread_blocked, "invariant"); assert(this->object() != NULL, "invariant"); - assert(_count >= 0, "invariant"); + assert(_contentions >= 0, "invariant"); // Prevent deflation at STW-time. See deflate_idle_monitors() and is_busy(). // Ensure the object-monitor relationship remains stable while there's contention. - Atomic::inc(&_count); + Atomic::inc(&_contentions); JFR_ONLY(JfrConditionalFlushWithStacktrace flush(jt);) EventJavaMonitorEnter event; @@ -355,8 +359,8 @@ // acquire it. } - Atomic::dec(&_count); - assert(_count >= 0, "invariant"); + Atomic::dec(&_contentions); + assert(_contentions >= 0, "invariant"); Self->_Stalled = 0; // Must either set _recursions = 0 or ASSERT _recursions == 0. @@ -809,7 +813,7 @@ // There's one exception to the claim above, however. EnterI() can call // exit() to drop a lock if the acquirer has been externally suspended. // In that case exit() is called with _thread_state as _thread_blocked, -// but the monitor's _count field is > 0, which inhibits reclamation. +// but the monitor's _contentions field is > 0, which inhibits reclamation. // // 1-0 exit // ~~~~~~~~ diff -r c604234be658 -r 04857e2cd509 src/hotspot/share/runtime/objectMonitor.hpp --- a/src/hotspot/share/runtime/objectMonitor.hpp Wed Apr 24 15:37:55 2019 +0200 +++ b/src/hotspot/share/runtime/objectMonitor.hpp Wed Apr 24 10:20:25 2019 -0400 @@ -163,9 +163,9 @@ volatile int _Spinner; // for exit->spinner handoff optimization volatile int _SpinDuration; - volatile jint _count; // reference count to prevent reclamation/deflation - // at stop-the-world time. See ObjectSynchronizer::deflate_monitor(). - // _count is approximately |_WaitSet| + |_EntryList| + volatile jint _contentions; // Number of active contentions in enter(). It is used by is_busy() + // along with other fields to determine if an ObjectMonitor can be + // deflated. See ObjectSynchronizer::deflate_monitor(). protected: ObjectWaiter * volatile _WaitSet; // LL of threads wait()ing on the monitor volatile jint _waiters; // number of waiting threads @@ -207,7 +207,6 @@ static int header_offset_in_bytes() { return offset_of(ObjectMonitor, _header); } static int object_offset_in_bytes() { return offset_of(ObjectMonitor, _object); } static int owner_offset_in_bytes() { return offset_of(ObjectMonitor, _owner); } - static int count_offset_in_bytes() { return offset_of(ObjectMonitor, _count); } static int recursions_offset_in_bytes() { return offset_of(ObjectMonitor, _recursions); } static int cxq_offset_in_bytes() { return offset_of(ObjectMonitor, _cxq); } static int succ_offset_in_bytes() { return offset_of(ObjectMonitor, _succ); } @@ -232,10 +231,8 @@ void set_header(markOop hdr); intptr_t is_busy() const { - // TODO-FIXME: merge _count and _waiters. // TODO-FIXME: assert _owner == null implies _recursions = 0 - // TODO-FIXME: assert _WaitSet != null implies _count > 0 - return _count|_waiters|intptr_t(_owner)|intptr_t(_cxq)|intptr_t(_EntryList); + return _contentions|_waiters|intptr_t(_owner)|intptr_t(_cxq)|intptr_t(_EntryList); } intptr_t is_entered(Thread* current) const; @@ -245,8 +242,6 @@ jint waiters() const; - jint count() const; - void set_count(jint count); jint contentions() const; intptr_t recursions() const { return _recursions; } @@ -263,14 +258,14 @@ ~ObjectMonitor() { // TODO: Add asserts ... // _cxq == 0 _succ == NULL _owner == NULL _waiters == 0 - // _count == 0 _EntryList == NULL etc + // _contentions == 0 _EntryList == NULL etc } private: void Recycle() { // TODO: add stronger asserts ... // _cxq == 0 _succ == NULL _owner == NULL _waiters == 0 - // _count == 0 EntryList == NULL + // _contentions == 0 EntryList == NULL // _recursions == 0 _WaitSet == NULL assert(((is_busy()|_recursions) == 0), "freeing inuse monitor"); _succ = NULL; diff -r c604234be658 -r 04857e2cd509 src/hotspot/share/runtime/objectMonitor.inline.hpp --- a/src/hotspot/share/runtime/objectMonitor.inline.hpp Wed Apr 24 15:37:55 2019 +0200 +++ b/src/hotspot/share/runtime/objectMonitor.inline.hpp Wed Apr 24 10:20:25 2019 -0400 @@ -45,10 +45,6 @@ _header = hdr; } -inline jint ObjectMonitor::count() const { - return _count; -} - inline jint ObjectMonitor::waiters() const { return _waiters; } @@ -58,12 +54,12 @@ } inline void ObjectMonitor::clear() { - assert(_header != NULL, "Fatal logic error in ObjectMonitor header!"); - assert(_count == 0, "Fatal logic error in ObjectMonitor count!"); - assert(_waiters == 0, "Fatal logic error in ObjectMonitor waiters!"); - assert(_recursions == 0, "Fatal logic error in ObjectMonitor recursions!"); - assert(_object != NULL, "Fatal logic error in ObjectMonitor object!"); - assert(_owner == NULL, "Fatal logic error in ObjectMonitor owner!"); + assert(_header != NULL, "must be non-NULL"); + assert(_contentions == 0, "must be 0: contentions=%d", _contentions); + assert(_waiters == 0, "must be 0: waiters=%d", _waiters); + assert(_recursions == 0, "must be 0: recursions=" INTPTR_FORMAT, _recursions); + assert(_object != NULL, "must be non-NULL"); + assert(_owner == NULL, "must be NULL: owner=" INTPTR_FORMAT, p2i(_owner)); _header = NULL; _object = NULL; @@ -96,10 +92,10 @@ // return number of threads contending for this monitor inline jint ObjectMonitor::contentions() const { - return _count; + return _contentions; } -// Do NOT set _count = 0. There is a race such that _count could +// Do NOT set _contentions = 0. There is a race such that _contentions could // be set while inflating prior to setting _owner // Just use Atomic::inc/dec and assert 0 when monitor put on free list inline void ObjectMonitor::set_owner(void* owner) { diff -r c604234be658 -r 04857e2cd509 src/hotspot/share/runtime/synchronizer.cpp --- a/src/hotspot/share/runtime/synchronizer.cpp Wed Apr 24 15:37:55 2019 +0200 +++ b/src/hotspot/share/runtime/synchronizer.cpp Wed Apr 24 10:20:25 2019 -0400 @@ -735,7 +735,7 @@ } else if (mark->has_monitor()) { monitor = mark->monitor(); temp = monitor->header(); - assert(temp->is_neutral(), "invariant: header=" INTPTR_FORMAT, p2i((address)temp)); + assert(temp->is_neutral(), "invariant: header=" INTPTR_FORMAT, p2i(temp)); hash = temp->hash(); if (hash != 0) { return hash; @@ -743,39 +743,38 @@ // Skip to the following code to reduce code size } else if (Self->is_lock_owned((address)mark->locker())) { temp = mark->displaced_mark_helper(); // this is a lightweight monitor owned - assert(temp->is_neutral(), "invariant: header=" INTPTR_FORMAT, p2i((address)temp)); + assert(temp->is_neutral(), "invariant: header=" INTPTR_FORMAT, p2i(temp)); hash = temp->hash(); // by current thread, check if the displaced if (hash != 0) { // header contains hash code return hash; } // WARNING: - // The displaced header is strictly immutable. - // It can NOT be changed in ANY cases. So we have - // to inflate the header into heavyweight monitor - // even the current thread owns the lock. The reason - // is the BasicLock (stack slot) will be asynchronously - // read by other threads during the inflate() function. - // Any change to stack may not propagate to other threads - // correctly. + // The displaced header in the BasicLock on a thread's stack + // is strictly immutable. It CANNOT be changed in ANY cases. + // So we have to inflate the stack lock into an ObjectMonitor + // even if the current thread owns the lock. The BasicLock on + // a thread's stack can be asynchronously read by other threads + // during an inflate() call so any change to that stack memory + // may not propagate to other threads correctly. } // Inflate the monitor to set hash code monitor = inflate(Self, obj, inflate_cause_hash_code); // Load displaced header and check it has hash code mark = monitor->header(); - assert(mark->is_neutral(), "invariant: header=" INTPTR_FORMAT, p2i((address)mark)); + assert(mark->is_neutral(), "invariant: header=" INTPTR_FORMAT, p2i(mark)); hash = mark->hash(); if (hash == 0) { hash = get_next_hash(Self, obj); temp = mark->copy_set_hash(hash); // merge hash code into header - assert(temp->is_neutral(), "invariant: header=" INTPTR_FORMAT, p2i((address)temp)); + assert(temp->is_neutral(), "invariant: header=" INTPTR_FORMAT, p2i(temp)); test = Atomic::cmpxchg(temp, monitor->header_addr(), mark); if (test != mark) { - // The only update to the header in the monitor (outside GC) - // is install the hash code. If someone add new usage of - // displaced header, please update this code + // The only update to the ObjectMonitor's header/dmw field + // is to merge in the hash code. If someone adds a new usage + // of the header/dmw field, please update this code. hash = test->hash(); - assert(test->is_neutral(), "invariant: header=" INTPTR_FORMAT, p2i((address)test)); + assert(test->is_neutral(), "invariant: header=" INTPTR_FORMAT, p2i(test)); assert(hash != 0, "Trivial unexpected object/monitor header usage."); } } @@ -1334,7 +1333,7 @@ if (mark->has_monitor()) { ObjectMonitor * inf = mark->monitor(); markOop dmw = inf->header(); - assert(dmw->is_neutral(), "invariant: header=" INTPTR_FORMAT, p2i((address)dmw)); + assert(dmw->is_neutral(), "invariant: header=" INTPTR_FORMAT, p2i(dmw)); assert(oopDesc::equals((oop) inf->object(), object), "invariant"); assert(ObjectSynchronizer::verify_objmon_isinpool(inf), "monitor is invalid"); return inf; @@ -1398,7 +1397,7 @@ // Consider what happens when a thread unlocks a stack-locked object. // It attempts to use CAS to swing the displaced header value from the // on-stack basiclock back into the object header. Recall also that the - // header value (hashcode, etc) can reside in (a) the object header, or + // header value (hash code, etc) can reside in (a) the object header, or // (b) a displaced header associated with the stack-lock, or (c) a displaced // header in an objectMonitor. The inflate() routine must copy the header // value from the basiclock on the owner's stack to the objectMonitor, all @@ -1419,7 +1418,9 @@ // object is in the mark. Furthermore the owner can't complete // an unlock on the object, either. markOop dmw = mark->displaced_mark_helper(); - assert(dmw->is_neutral(), "invariant"); + // Catch if the object's header is not neutral (not locked and + // not marked is what we care about here). + assert(dmw->is_neutral(), "invariant: header=" INTPTR_FORMAT, p2i(dmw)); // Setup monitor fields to proper values -- prepare the monitor m->set_header(dmw); @@ -1463,7 +1464,9 @@ // An inflateTry() method that we could call from fast_enter() and slow_enter() // would be useful. - assert(mark->is_neutral(), "invariant"); + // Catch if the object's header is not neutral (not locked and + // not marked is what we care about here). + assert(mark->is_neutral(), "invariant: header=" INTPTR_FORMAT, p2i(mark)); ObjectMonitor * m = omAlloc(Self); // prepare m for installation - set monitor to initial state m->Recycle(); @@ -1503,7 +1506,7 @@ } -// We create a list of in-use monitors for each thread. +// We maintain a list of in-use monitors for each thread. // // deflate_thread_local_monitors() scans a single thread's in-use list, while // deflate_idle_monitors() scans only a global list of in-use monitors which @@ -1512,7 +1515,7 @@ // These operations are called at all safepoints, immediately after mutators // are stopped, but before any objects have moved. Collectively they traverse // the population of in-use monitors, deflating where possible. The scavenged -// monitors are returned to the monitor free list. +// monitors are returned to the global monitor free list. // // Beware that we scavenge at *every* stop-the-world point. Having a large // number of monitors in-use could negatively impact performance. We also want @@ -1521,7 +1524,7 @@ // // Perversely, the heap size -- and thus the STW safepoint rate -- // typically drives the scavenge rate. Large heaps can mean infrequent GC, -// which in turn can mean large(r) numbers of objectmonitors in circulation. +// which in turn can mean large(r) numbers of ObjectMonitors in circulation. // This is an unfortunate aspect of this design. // Deflate a single monitor if not in-use @@ -1531,9 +1534,15 @@ ObjectMonitor** freeTailp) { bool deflated; // Normal case ... The monitor is associated with obj. - guarantee(obj->mark() == markOopDesc::encode(mid), "invariant"); - guarantee(mid == obj->mark()->monitor(), "invariant"); - guarantee(mid->header()->is_neutral(), "invariant"); + const markOop mark = obj->mark(); + guarantee(mark == markOopDesc::encode(mid), "should match: mark=" + INTPTR_FORMAT ", encoded mid=" INTPTR_FORMAT, p2i(mark), + p2i(markOopDesc::encode(mid))); + // Make sure that mark->monitor() and markOopDesc::encode() agree: + guarantee(mark->monitor() == mid, "should match: monitor()=" INTPTR_FORMAT + ", mid=" INTPTR_FORMAT, p2i(mark->monitor()), p2i(mid)); + const markOop dmw = mid->header(); + guarantee(dmw->is_neutral(), "invariant: header=" INTPTR_FORMAT, p2i(dmw)); if (mid->is_busy()) { deflated = false; @@ -1544,16 +1553,17 @@ if (log_is_enabled(Trace, monitorinflation)) { ResourceMark rm; log_trace(monitorinflation)("deflate_monitor: " - "object=" INTPTR_FORMAT ", mark=" INTPTR_FORMAT ", type='%s'", - p2i(obj), p2i(obj->mark()), - obj->klass()->external_name()); + "object=" INTPTR_FORMAT ", mark=" + INTPTR_FORMAT ", type='%s'", p2i(obj), + p2i(mark), obj->klass()->external_name()); } // Restore the header back to obj - obj->release_set_mark(mid->header()); + obj->release_set_mark(dmw); mid->clear(); - assert(mid->object() == NULL, "invariant"); + assert(mid->object() == NULL, "invariant: object=" INTPTR_FORMAT, + p2i(mid->object())); // Move the object to the working free list defined by freeHeadp, freeTailp if (*freeHeadp == NULL) *freeHeadp = mid; @@ -2022,13 +2032,12 @@ out->print_cr("ERROR: jt=" INTPTR_FORMAT ", monitor=" INTPTR_FORMAT ": in-use per-thread monitor's object does not think " "it has a monitor: obj=" INTPTR_FORMAT ", mark=" - INTPTR_FORMAT, p2i(jt), p2i(n), p2i((address)obj), - p2i((address)mark)); + INTPTR_FORMAT, p2i(jt), p2i(n), p2i(obj), p2i(mark)); } else { out->print_cr("ERROR: monitor=" INTPTR_FORMAT ": in-use global " "monitor's object does not think it has a monitor: obj=" INTPTR_FORMAT ", mark=" INTPTR_FORMAT, p2i(n), - p2i((address)obj), p2i((address)mark)); + p2i(obj), p2i(mark)); } *error_cnt_p = *error_cnt_p + 1; } @@ -2039,14 +2048,12 @@ ": in-use per-thread monitor's object does not refer " "to the same monitor: obj=" INTPTR_FORMAT ", mark=" INTPTR_FORMAT ", obj_mon=" INTPTR_FORMAT, p2i(jt), - p2i(n), p2i((address)obj), p2i((address)mark), - p2i((address)obj_mon)); + p2i(n), p2i(obj), p2i(mark), p2i(obj_mon)); } else { out->print_cr("ERROR: monitor=" INTPTR_FORMAT ": in-use global " "monitor's object does not refer to the same monitor: obj=" INTPTR_FORMAT ", mark=" INTPTR_FORMAT ", obj_mon=" - INTPTR_FORMAT, p2i(n), p2i((address)obj), - p2i((address)mark), p2i((address)obj_mon)); + INTPTR_FORMAT, p2i(n), p2i(obj), p2i(mark), p2i(obj_mon)); } *error_cnt_p = *error_cnt_p + 1; } @@ -2105,7 +2112,7 @@ if (gOmInUseCount > 0) { out->print_cr("In-use global monitor info:"); - out->print_cr("(B -> is_busy, H -> has hashcode, L -> lock status)"); + out->print_cr("(B -> is_busy, H -> has hash code, L -> lock status)"); out->print_cr("%18s %s %18s %18s", "monitor", "BHL", "object", "object type"); out->print_cr("================== === ================== =================="); @@ -2124,7 +2131,7 @@ } out->print_cr("In-use per-thread monitor info:"); - out->print_cr("(B -> is_busy, H -> has hashcode, L -> lock status)"); + out->print_cr("(B -> is_busy, H -> has hash code, L -> lock status)"); out->print_cr("%18s %18s %s %18s %18s", "jt", "monitor", "BHL", "object", "object type"); out->print_cr("================== ================== === ================== =================="); diff -r c604234be658 -r 04857e2cd509 src/hotspot/share/runtime/vmStructs.cpp --- a/src/hotspot/share/runtime/vmStructs.cpp Wed Apr 24 15:37:55 2019 +0200 +++ b/src/hotspot/share/runtime/vmStructs.cpp Wed Apr 24 10:20:25 2019 -0400 @@ -896,7 +896,7 @@ volatile_nonstatic_field(ObjectMonitor, _header, markOop) \ unchecked_nonstatic_field(ObjectMonitor, _object, sizeof(void *)) /* NOTE: no type */ \ unchecked_nonstatic_field(ObjectMonitor, _owner, sizeof(void *)) /* NOTE: no type */ \ - volatile_nonstatic_field(ObjectMonitor, _count, jint) \ + volatile_nonstatic_field(ObjectMonitor, _contentions, jint) \ volatile_nonstatic_field(ObjectMonitor, _waiters, jint) \ volatile_nonstatic_field(ObjectMonitor, _recursions, intptr_t) \ nonstatic_field(ObjectMonitor, FreeNext, ObjectMonitor*) \ diff -r c604234be658 -r 04857e2cd509 src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/ObjectMonitor.java --- a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/ObjectMonitor.java Wed Apr 24 15:37:55 2019 +0200 +++ b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/ObjectMonitor.java Wed Apr 24 10:20:25 2019 -0400 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2001, 2014, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2001, 2019, 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 @@ -50,7 +50,7 @@ ownerFieldOffset = f.getOffset(); f = type.getField("FreeNext"); FreeNextFieldOffset = f.getOffset(); - countField = type.getJIntField("_count"); + contentionsField = type.getJIntField("_contentions"); waitersField = type.getJIntField("_waiters"); recursionsField = type.getCIntegerField("_recursions"); } @@ -87,19 +87,14 @@ // FIXME // void set_queue(void* owner); - public int count() { return countField.getValue(addr); } - // FIXME - // void set_count(int count); - public long recursions() { return recursionsField.getValue(addr); } public OopHandle object() { return addr.getOopHandleAt(objectFieldOffset); } - // contentions is always equal to count public int contentions() { - return count(); + return contentionsField.getValue(addr); } // FIXME @@ -114,7 +109,7 @@ private static long objectFieldOffset; private static long ownerFieldOffset; private static long FreeNextFieldOffset; - private static JIntField countField; + private static JIntField contentionsField; private static JIntField waitersField; private static CIntegerField recursionsField; // FIXME: expose platform-dependent stuff diff -r c604234be658 -r 04857e2cd509 src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/ui/MonitorCacheDumpPanel.java --- a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/ui/MonitorCacheDumpPanel.java Wed Apr 24 15:37:55 2019 +0200 +++ b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/ui/MonitorCacheDumpPanel.java Wed Apr 24 10:20:25 2019 -0400 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2002, 2007, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2002, 2019, 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 @@ -83,7 +83,7 @@ } } } - tty.println(" _count: " + mon.count()); + tty.println(" _contentions: " + mon.contentions()); tty.println(" _waiters: " + mon.waiters()); tty.println(" _recursions: " + mon.recursions()); } @@ -98,7 +98,7 @@ ObjectMonitor mon; while (i.hasNext()) { mon = (ObjectMonitor)i.next(); - if (mon.count() != 0 || mon.waiters() != 0 || mon.owner() != null) { + if (mon.contentions() != 0 || mon.waiters() != 0 || mon.owner() != null) { OopHandle object = mon.object(); if (object == null) { dumpMonitor(tty, mon, true);