8234544: ObjectSynchronizer::FastHashCode() cleanups from Async Monitor Deflation project
authordcubed
Fri, 22 Nov 2019 09:06:35 -0500
changeset 59225 80e1201f6c9a
parent 59224 55fdee124e89
child 59226 a0f39cc47387
8234544: ObjectSynchronizer::FastHashCode() cleanups from Async Monitor Deflation project Reviewed-by: dholmes
src/hotspot/share/runtime/synchronizer.cpp
--- a/src/hotspot/share/runtime/synchronizer.cpp	Fri Nov 22 11:52:48 2019 +0000
+++ b/src/hotspot/share/runtime/synchronizer.cpp	Fri Nov 22 09:06:35 2019 -0500
@@ -694,35 +694,39 @@
   // object should remain ineligible for biased locking
   assert(!mark.has_bias_pattern(), "invariant");
 
-  if (mark.is_neutral()) {
-    hash = mark.hash();               // this is a normal header
-    if (hash != 0) {                  // if it has hash, just return it
+  if (mark.is_neutral()) {            // if this is a normal header
+    hash = mark.hash();
+    if (hash != 0) {                  // if it has a hash, just return it
       return hash;
     }
-    hash = get_next_hash(self, obj);  // allocate a new hash code
-    temp = mark.copy_set_hash(hash);  // merge the hash code into header
-    // use (machine word version) atomic operation to install the hash
+    hash = get_next_hash(self, obj);  // get a new hash
+    temp = mark.copy_set_hash(hash);  // merge the hash into header
+                                      // try to install the hash
     test = obj->cas_set_mark(temp, mark);
-    if (test == mark) {
+    if (test == mark) {               // if the hash was installed, return it
       return hash;
     }
-    // If atomic operation failed, we must inflate the header
-    // into heavy weight monitor. We could add more code here
-    // for fast path, but it does not worth the complexity.
+    // Failed to install the hash. It could be that another thread
+    // installed the hash just before our attempt or inflation has
+    // occurred or... so we fall thru to inflate the monitor for
+    // stability and then install the hash.
   } else if (mark.has_monitor()) {
     monitor = mark.monitor();
     temp = monitor->header();
     assert(temp.is_neutral(), "invariant: header=" INTPTR_FORMAT, temp.value());
     hash = temp.hash();
-    if (hash != 0) {
+    if (hash != 0) {                  // if it has a hash, just return it
       return hash;
     }
-    // Skip to the following code to reduce code size
+    // Fall thru so we only have one place that installs the hash in
+    // the ObjectMonitor.
   } else if (self->is_lock_owned((address)mark.locker())) {
-    temp = mark.displaced_mark_helper(); // this is a lightweight monitor owned
+    // This is a stack lock owned by the calling thread so fetch the
+    // displaced markWord from the BasicLock on the stack.
+    temp = mark.displaced_mark_helper();
     assert(temp.is_neutral(), "invariant: header=" INTPTR_FORMAT, temp.value());
-    hash = temp.hash();                  // by current thread, check if the displaced
-    if (hash != 0) {                     // header contains hash code
+    hash = temp.hash();
+    if (hash != 0) {                  // if it has a hash, just return it
       return hash;
     }
     // WARNING:
@@ -735,29 +739,30 @@
     // may not propagate to other threads correctly.
   }
 
-  // Inflate the monitor to set hash code
+  // Inflate the monitor to set the hash.
   monitor = inflate(self, obj, inflate_cause_hash_code);
-  // Load displaced header and check it has hash code
+  // Load ObjectMonitor's header/dmw field and see if it has a hash.
   mark = monitor->header();
   assert(mark.is_neutral(), "invariant: header=" INTPTR_FORMAT, mark.value());
   hash = mark.hash();
-  if (hash == 0) {
-    hash = get_next_hash(self, obj);
-    temp = mark.copy_set_hash(hash); // merge hash code into header
+  if (hash == 0) {                    // if it does not have a hash
+    hash = get_next_hash(self, obj);  // get a new hash
+    temp = mark.copy_set_hash(hash);  // merge the hash into header
     assert(temp.is_neutral(), "invariant: header=" INTPTR_FORMAT, temp.value());
     uintptr_t v = Atomic::cmpxchg(temp.value(), (volatile uintptr_t*)monitor->header_addr(), mark.value());
     test = markWord(v);
     if (test != mark) {
-      // The only non-deflation 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.
+      // The attempt to update the ObjectMonitor's header/dmw field
+      // did not work. This can happen if another thread managed to
+      // merge in the hash just before our cmpxchg().
+      // If we add any new usages of the header/dmw field, this code
+      // will need to be updated.
       hash = test.hash();
       assert(test.is_neutral(), "invariant: header=" INTPTR_FORMAT, test.value());
-      assert(hash != 0, "Trivial unexpected object/monitor header usage.");
+      assert(hash != 0, "should only have lost the race to a thread that set a non-zero hash");
     }
   }
-  // We finally get the hash
+  // We finally get the hash.
   return hash;
 }