8234544: ObjectSynchronizer::FastHashCode() cleanups from Async Monitor Deflation project
Reviewed-by: dholmes
--- 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;
}