# HG changeset patch
# User dcubed
# Date 1560351165 14400
# Node ID 492b644bb9c2ef87c2715f209d672591cc7bc2a3
# Parent  0c20dbc3d5476acac16334d436f1bf55db7abd07
8225453: is_busy diagnostics and other baseline cleanups from Async Monitor Deflation project
Reviewed-by: dholmes, rehn, coleenp

diff -r 0c20dbc3d547 -r 492b644bb9c2 src/hotspot/share/runtime/objectMonitor.cpp
--- a/src/hotspot/share/runtime/objectMonitor.cpp	Wed Jun 12 14:24:11 2019 +0200
+++ b/src/hotspot/share/runtime/objectMonitor.cpp	Wed Jun 12 10:52:45 2019 -0400
@@ -245,9 +245,7 @@
 
   void * cur = Atomic::cmpxchg(Self, &_owner, (void*)NULL);
   if (cur == NULL) {
-    // Either ASSERT _recursions == 0 or explicitly set _recursions = 0.
     assert(_recursions == 0, "invariant");
-    assert(_owner == Self, "invariant");
     return;
   }
 
@@ -405,9 +403,7 @@
   void * own = _owner;
   if (own != NULL) return 0;
   if (Atomic::replace_if_null(Self, &_owner)) {
-    // Either guarantee _recursions == 0 or set _recursions = 0.
     assert(_recursions == 0, "invariant");
-    assert(_owner == Self, "invariant");
     return 1;
   }
   // The lock had been free momentarily, but we lost the race to the lock.
@@ -417,6 +413,15 @@
   return -1;
 }
 
+// Convert the fields used by is_busy() to a string that can be
+// used for diagnostic output.
+const char* ObjectMonitor::is_busy_to_string(stringStream* ss) {
+  ss->print("is_busy: contentions=%d, waiters=%d, owner=" INTPTR_FORMAT
+            ", cxq=" INTPTR_FORMAT ", EntryList=" INTPTR_FORMAT, _contentions,
+            _waiters, p2i(_owner), p2i(_cxq), p2i(_EntryList));
+  return ss->base();
+}
+
 #define MAX_RECHECK_INTERVAL 1000
 
 void ObjectMonitor::EnterI(TRAPS) {
diff -r 0c20dbc3d547 -r 492b644bb9c2 src/hotspot/share/runtime/objectMonitor.hpp
--- a/src/hotspot/share/runtime/objectMonitor.hpp	Wed Jun 12 14:24:11 2019 +0200
+++ b/src/hotspot/share/runtime/objectMonitor.hpp	Wed Jun 12 10:52:45 2019 -0400
@@ -235,6 +235,7 @@
     // TODO-FIXME: assert _owner == null implies _recursions = 0
     return _contentions|_waiters|intptr_t(_owner)|intptr_t(_cxq)|intptr_t(_EntryList);
   }
+  const char* is_busy_to_string(stringStream* ss);
 
   intptr_t  is_entered(Thread* current) const;
 
@@ -268,7 +269,9 @@
     // _cxq == 0 _succ == NULL _owner == NULL _waiters == 0
     // _contentions == 0 EntryList  == NULL
     // _recursions == 0 _WaitSet == NULL
-    assert(((is_busy()|_recursions) == 0), "freeing inuse monitor");
+    DEBUG_ONLY(stringStream ss;)
+    assert((is_busy() | _recursions) == 0, "freeing in-use monitor: %s, "
+           "recursions=" INTPTR_FORMAT, is_busy_to_string(&ss), _recursions);
     _succ          = NULL;
     _EntryList     = NULL;
     _cxq           = NULL;
diff -r 0c20dbc3d547 -r 492b644bb9c2 src/hotspot/share/runtime/objectMonitor.inline.hpp
--- a/src/hotspot/share/runtime/objectMonitor.inline.hpp	Wed Jun 12 14:24:11 2019 +0200
+++ b/src/hotspot/share/runtime/objectMonitor.inline.hpp	Wed Jun 12 10:52:45 2019 -0400
@@ -95,12 +95,8 @@
   return _contentions;
 }
 
-// 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) {
   _owner = owner;
-  _recursions = 0;
 }
 
 #endif // SHARE_RUNTIME_OBJECTMONITOR_INLINE_HPP
diff -r 0c20dbc3d547 -r 492b644bb9c2 src/hotspot/share/runtime/synchronizer.cpp
--- a/src/hotspot/share/runtime/synchronizer.cpp	Wed Jun 12 14:24:11 2019 +0200
+++ b/src/hotspot/share/runtime/synchronizer.cpp	Wed Jun 12 10:52:45 2019 -0400
@@ -242,7 +242,6 @@
 
     if (owner == NULL && Atomic::replace_if_null(Self, &(m->_owner))) {
       assert(m->_recursions == 0, "invariant");
-      assert(m->_owner == Self, "invariant");
       return true;
     }
   }
@@ -1030,6 +1029,7 @@
   // scavenge costs.  As usual, we lean toward time in space-time
   // tradeoffs.
   const int MAXPRIVATE = 1024;
+  stringStream ss;
   for (;;) {
     ObjectMonitor * m;
 
@@ -1065,7 +1065,6 @@
         ObjectMonitor * take = gFreeList;
         gFreeList = take->FreeNext;
         guarantee(take->object() == NULL, "invariant");
-        guarantee(!take->is_busy(), "invariant");
         take->Recycle();
         omRelease(Self, take, false);
       }
@@ -1167,7 +1166,10 @@
                                    bool fromPerThreadAlloc) {
   guarantee(m->header() == NULL, "invariant");
   guarantee(m->object() == NULL, "invariant");
-  guarantee(((m->is_busy()|m->_recursions) == 0), "freeing in-use monitor");
+  stringStream ss;
+  guarantee((m->is_busy() | m->_recursions) == 0, "freeing in-use monitor: "
+            "%s, recursions=" INTPTR_FORMAT, m->is_busy_to_string(&ss),
+            m->_recursions);
   // Remove from omInUseList
   if (fromPerThreadAlloc) {
     ObjectMonitor* cur_mid_in_use = NULL;
@@ -1220,16 +1222,14 @@
   int tally = 0;
   if (list != NULL) {
     ObjectMonitor * s;
-    // The thread is going away, the per-thread free monitors
-    // are freed via set_owner(NULL)
-    // Link them to tail, which will be linked into the global free list
-    // gFreeList below, under the gListLock
+    // The thread is going away. Set 'tail' to the last per-thread free
+    // monitor which will be linked to gFreeList below under the gListLock.
+    stringStream ss;
     for (s = list; s != NULL; s = s->FreeNext) {
       tally++;
       tail = s;
       guarantee(s->object() == NULL, "invariant");
-      guarantee(!s->is_busy(), "invariant");
-      s->set_owner(NULL);   // redundant but good hygiene
+      guarantee(!s->is_busy(), "must be !is_busy: %s", s->is_busy_to_string(&ss));
     }
     guarantee(tail != NULL, "invariant");
     assert(Self->omFreeCount == tally, "free-count off");
@@ -1379,7 +1379,6 @@
       // in which INFLATING appears in the mark.
       m->Recycle();
       m->_Responsible  = NULL;
-      m->_recursions   = 0;
       m->_SpinDuration = ObjectMonitor::Knob_SpinLimit;   // Consider: maintain by type/class
 
       markOop cmp = object->cas_set_mark(markOopDesc::INFLATING(), mark);
@@ -1472,9 +1471,7 @@
     // prepare m for installation - set monitor to initial state
     m->Recycle();
     m->set_header(mark);
-    m->set_owner(NULL);
     m->set_object(object);
-    m->_recursions   = 0;
     m->_Responsible  = NULL;
     m->_SpinDuration = ObjectMonitor::Knob_SpinLimit;       // consider: keep metastats by type/class
 
@@ -1925,14 +1922,15 @@
 // Check a free monitor entry; log any errors.
 void ObjectSynchronizer::chk_free_entry(JavaThread * jt, ObjectMonitor * n,
                                         outputStream * out, int *error_cnt_p) {
+  stringStream ss;
   if (n->is_busy()) {
     if (jt != NULL) {
       out->print_cr("ERROR: jt=" INTPTR_FORMAT ", monitor=" INTPTR_FORMAT
-                    ": free per-thread monitor must not be busy.", p2i(jt),
-                    p2i(n));
+                    ": free per-thread monitor must not be busy: %s", p2i(jt),
+                    p2i(n), n->is_busy_to_string(&ss));
     } else {
       out->print_cr("ERROR: monitor=" INTPTR_FORMAT ": free global monitor "
-                    "must not be busy.", p2i(n));
+                    "must not be busy: %s", p2i(n), n->is_busy_to_string(&ss));
     }
     *error_cnt_p = *error_cnt_p + 1;
   }
@@ -2111,6 +2109,7 @@
     Thread::muxAcquire(&gListLock, "log_in_use_monitor_details");
   }
 
+  stringStream ss;
   if (gOmInUseCount > 0) {
     out->print_cr("In-use global monitor info:");
     out->print_cr("(B -> is_busy, H -> has hash code, L -> lock status)");
@@ -2121,9 +2120,14 @@
       const oop obj = (oop) n->object();
       const markOop mark = n->header();
       ResourceMark rm;
-      out->print_cr(INTPTR_FORMAT "  %d%d%d  " INTPTR_FORMAT "  %s", p2i(n),
-                    n->is_busy() != 0, mark->hash() != 0, n->owner() != NULL,
-                    p2i(obj), obj->klass()->external_name());
+      out->print(INTPTR_FORMAT "  %d%d%d  " INTPTR_FORMAT "  %s", p2i(n),
+                 n->is_busy() != 0, mark->hash() != 0, n->owner() != NULL,
+                 p2i(obj), obj->klass()->external_name());
+      if (n->is_busy() != 0) {
+        out->print(" (%s)", n->is_busy_to_string(&ss));
+        ss.reset();
+      }
+      out->cr();
     }
   }
 
@@ -2141,10 +2145,15 @@
       const oop obj = (oop) n->object();
       const markOop mark = n->header();
       ResourceMark rm;
-      out->print_cr(INTPTR_FORMAT "  " INTPTR_FORMAT "  %d%d%d  " INTPTR_FORMAT
-                    "  %s", p2i(jt), p2i(n), n->is_busy() != 0,
-                    mark->hash() != 0, n->owner() != NULL, p2i(obj),
-                    obj->klass()->external_name());
+      out->print(INTPTR_FORMAT "  " INTPTR_FORMAT "  %d%d%d  " INTPTR_FORMAT
+                 "  %s", p2i(jt), p2i(n), n->is_busy() != 0,
+                 mark->hash() != 0, n->owner() != NULL, p2i(obj),
+                 obj->klass()->external_name());
+      if (n->is_busy() != 0) {
+        out->print(" (%s)", n->is_busy_to_string(&ss));
+        ss.reset();
+      }
+      out->cr();
     }
   }