8152358: code and comment cleanups found during the hunt for 8077392
Reviewed-by: gthornbr, kvn, cvarming
--- a/hotspot/src/cpu/x86/vm/c1_LIRAssembler_x86.cpp Mon Apr 04 14:41:00 2016 -0700
+++ b/hotspot/src/cpu/x86/vm/c1_LIRAssembler_x86.cpp Mon Apr 04 14:49:19 2016 -0700
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2000, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2016, 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
@@ -312,7 +312,7 @@
Register OSR_buf = osrBufferPointer()->as_pointer_register();
{ assert(frame::interpreter_frame_monitor_size() == BasicObjectLock::size(), "adjust code below");
int monitor_offset = BytesPerWord * method()->max_locals() +
- (2 * BytesPerWord) * (number_of_locks - 1);
+ (BasicObjectLock::size() * BytesPerWord) * (number_of_locks - 1);
// SharedRuntime::OSR_migration_begin() packs BasicObjectLocks in
// the OSR buffer using 2 word entries: first the lock and then
// the oop.
--- a/hotspot/src/cpu/x86/vm/interp_masm_x86.cpp Mon Apr 04 14:41:00 2016 -0700
+++ b/hotspot/src/cpu/x86/vm/interp_masm_x86.cpp Mon Apr 04 14:49:19 2016 -0700
@@ -1099,7 +1099,7 @@
movptr(Address(lock_reg, mark_offset), swap_reg);
assert(lock_offset == 0,
- "displached header must be first word in BasicObjectLock");
+ "displaced header must be first word in BasicObjectLock");
if (os::is_MP()) lock();
cmpxchgptr(lock_reg, Address(obj_reg, 0));
@@ -1154,7 +1154,7 @@
// Kills:
// rax
// c_rarg0, c_rarg1, c_rarg2, c_rarg3, ... (param regs)
-// rscratch1, rscratch2 (scratch regs)
+// rscratch1 (scratch reg)
// rax, rbx, rcx, rdx
void InterpreterMacroAssembler::unlock_object(Register lock_reg) {
assert(lock_reg == LP64_ONLY(c_rarg1) NOT_LP64(rdx),
@@ -1201,7 +1201,7 @@
if (os::is_MP()) lock();
cmpxchgptr(header_reg, Address(obj_reg, 0));
- // zero for recursive case
+ // zero for simple unlock of a stack-lock case
jcc(Assembler::zero, done);
// Call the runtime routine for slow case.
--- a/hotspot/src/cpu/x86/vm/macroAssembler_x86.cpp Mon Apr 04 14:41:00 2016 -0700
+++ b/hotspot/src/cpu/x86/vm/macroAssembler_x86.cpp Mon Apr 04 14:49:19 2016 -0700
@@ -1106,7 +1106,7 @@
assert_different_registers(lock_reg, obj_reg, swap_reg, tmp_reg);
assert(markOopDesc::age_shift == markOopDesc::lock_bits + markOopDesc::biased_lock_bits, "biased locking makes assumptions about bit layout");
Address mark_addr (obj_reg, oopDesc::mark_offset_in_bytes());
- Address saved_mark_addr(lock_reg, 0);
+ NOT_LP64( Address saved_mark_addr(lock_reg, 0); )
if (PrintBiasedLockingStatistics && counters == NULL) {
counters = BiasedLocking::counters();
@@ -1695,7 +1695,7 @@
RTMLockingCounters* stack_rtm_counters,
Metadata* method_data,
bool use_rtm, bool profile_rtm) {
- // Ensure the register assignents are disjoint
+ // Ensure the register assignments are disjoint
assert(tmpReg == rax, "");
if (use_rtm) {
@@ -2194,8 +2194,8 @@
cmpptr(Address(tmpReg, OM_OFFSET_NO_MONITOR_VALUE_TAG(succ)), (int32_t)NULL_WORD);
jccb (Assembler::zero, LGoSlowPath);
+ xorptr(boxReg, boxReg);
if ((EmitSync & 16) && os::is_MP()) {
- orptr(boxReg, boxReg);
xchgptr(boxReg, Address(tmpReg, OM_OFFSET_NO_MONITOR_VALUE_TAG(owner)));
} else {
movptr(Address(tmpReg, OM_OFFSET_NO_MONITOR_VALUE_TAG(owner)), (int32_t)NULL_WORD);
@@ -2227,7 +2227,6 @@
// box is really RAX -- the following CMPXCHG depends on that binding
// cmpxchg R,[M] is equivalent to rax = CAS(M,rax,R)
- movptr(boxReg, (int32_t)NULL_WORD);
if (os::is_MP()) { lock(); }
cmpxchgptr(r15_thread, Address(tmpReg, OM_OFFSET_NO_MONITOR_VALUE_TAG(owner)));
// There's no successor so we tried to regrab the lock.
--- a/hotspot/src/share/vm/runtime/biasedLocking.cpp Mon Apr 04 14:41:00 2016 -0700
+++ b/hotspot/src/share/vm/runtime/biasedLocking.cpp Mon Apr 04 14:49:19 2016 -0700
@@ -149,9 +149,13 @@
if (!mark->has_bias_pattern()) {
if (log_is_enabled(Info, biasedlocking)) {
ResourceMark rm;
- log_info(biasedlocking)(" (Skipping revocation of object of type %s "
- "because it's no longer biased)",
- obj->klass()->external_name());
+ log_info(biasedlocking)(" (Skipping revocation of object " INTPTR_FORMAT
+ ", mark " INTPTR_FORMAT ", type %s"
+ ", requesting thread " INTPTR_FORMAT
+ " because it's no longer biased)",
+ p2i((void *)obj), (intptr_t) mark,
+ obj->klass()->external_name(),
+ (intptr_t) requesting_thread);
}
return BiasedLocking::NOT_BIASED;
}
@@ -163,9 +167,9 @@
// Log at "info" level if not bulk, else "trace" level
if (!is_bulk) {
ResourceMark rm;
- log_info(biasedlocking)("Revoking bias of object " INTPTR_FORMAT " , mark "
- INTPTR_FORMAT " , type %s , prototype header " INTPTR_FORMAT
- " , allow rebias %d , requesting thread " INTPTR_FORMAT,
+ log_info(biasedlocking)("Revoking bias of object " INTPTR_FORMAT ", mark "
+ INTPTR_FORMAT ", type %s, prototype header " INTPTR_FORMAT
+ ", allow rebias %d, requesting thread " INTPTR_FORMAT,
p2i((void *)obj),
(intptr_t) mark,
obj->klass()->external_name(),
@@ -222,13 +226,24 @@
}
// Log at "info" level if not bulk, else "trace" level
if (!is_bulk) {
- log_info(biasedlocking)(" Revoked bias of object biased toward dead thread");
+ log_info(biasedlocking)(" Revoked bias of object biased toward dead thread ("
+ PTR_FORMAT ")", p2i(biased_thread));
} else {
- log_trace(biasedlocking)(" Revoked bias of object biased toward dead thread");
+ log_trace(biasedlocking)(" Revoked bias of object biased toward dead thread ("
+ PTR_FORMAT ")", p2i(biased_thread));
}
return BiasedLocking::BIAS_REVOKED;
}
+ // Log at "info" level if not bulk, else "trace" level
+ if (!is_bulk) {
+ log_info(biasedlocking)(" Revoked bias of object biased toward live thread ("
+ PTR_FORMAT ")", p2i(biased_thread));
+ } else {
+ log_trace(biasedlocking)(" Revoked bias of object biased toward live thread ("
+ PTR_FORMAT ")", p2i(biased_thread));
+ }
+
// Thread owning bias is alive.
// Check to see whether it currently owns the lock and, if so,
// write down the needed displaced headers to the thread's stack.
--- a/hotspot/src/share/vm/runtime/objectMonitor.cpp Mon Apr 04 14:41:00 2016 -0700
+++ b/hotspot/src/share/vm/runtime/objectMonitor.cpp Mon Apr 04 14:49:19 2016 -0700
@@ -850,7 +850,7 @@
// ~~~~~~~~
// ::exit() uses a canonical 1-1 idiom with a MEMBAR although some of
// the fast-path operators have been optimized so the common ::exit()
-// operation is 1-0. See i486.ad fast_unlock(), for instance.
+// operation is 1-0, e.g., see macroAssembler_x86.cpp: fast_unlock().
// The code emitted by fast_unlock() elides the usual MEMBAR. This
// greatly improves latency -- MEMBAR and CAS having considerable local
// latency on modern processors -- but at the cost of "stranding". Absent the
@@ -863,7 +863,7 @@
//
// The CAS() in enter provides for safety and exclusion, while the CAS or
// MEMBAR in exit provides for progress and avoids stranding. 1-0 locking
-// eliminates the CAS/MEMBAR from the exist path, but it admits stranding.
+// eliminates the CAS/MEMBAR from the exit path, but it admits stranding.
// We detect and recover from stranding with timers.
//
// If a thread transiently strands it'll park until (a) another
@@ -936,7 +936,6 @@
for (;;) {
assert(THREAD == _owner, "invariant");
-
if (Knob_ExitPolicy == 0) {
// release semantics: prior loads and stores from within the critical section
// must not float (reorder) past the following store that drops the lock.
--- a/hotspot/src/share/vm/runtime/os.cpp Mon Apr 04 14:41:00 2016 -0700
+++ b/hotspot/src/share/vm/runtime/os.cpp Mon Apr 04 14:49:19 2016 -0700
@@ -1284,8 +1284,8 @@
_mem_serialize_page = (volatile int32_t *)page;
// We initialize the serialization page shift count here
// We assume a cache line size of 64 bytes
- assert(SerializePageShiftCount == count,
- "thread size changed, fix SerializePageShiftCount constant");
+ assert(SerializePageShiftCount == count, "JavaThread size changed; "
+ "SerializePageShiftCount constant should be %d", count);
set_serialize_page_mask((uintptr_t)(vm_page_size() - sizeof(int32_t)));
}
--- a/hotspot/src/share/vm/runtime/sharedRuntime.cpp Mon Apr 04 14:41:00 2016 -0700
+++ b/hotspot/src/share/vm/runtime/sharedRuntime.cpp Mon Apr 04 14:49:19 2016 -0700
@@ -2954,7 +2954,7 @@
Method* moop = fr.interpreter_frame_method();
int max_locals = moop->max_locals();
// Allocate temp buffer, 1 word per local & 2 per active monitor
- int buf_size_words = max_locals + active_monitor_count*2;
+ int buf_size_words = max_locals + active_monitor_count * BasicObjectLock::size();
intptr_t *buf = NEW_C_HEAP_ARRAY(intptr_t,buf_size_words, mtCode);
// Copy the locals. Order is preserved so that loading of longs works.
--- a/hotspot/src/share/vm/runtime/synchronizer.cpp Mon Apr 04 14:41:00 2016 -0700
+++ b/hotspot/src/share/vm/runtime/synchronizer.cpp Mon Apr 04 14:49:19 2016 -0700
@@ -283,38 +283,52 @@
}
void ObjectSynchronizer::fast_exit(oop object, BasicLock* lock, TRAPS) {
- assert(!object->mark()->has_bias_pattern(), "should not see bias pattern here");
- // if displaced header is null, the previous enter is recursive enter, no-op
+ markOop mark = object->mark();
+ // We cannot check for Biased Locking if we are racing an inflation.
+ assert(mark == markOopDesc::INFLATING() ||
+ !mark->has_bias_pattern(), "should not see bias pattern here");
+
markOop dhw = lock->displaced_header();
- markOop mark;
if (dhw == NULL) {
- // Recursive stack-lock.
- // Diagnostics -- Could be: stack-locked, inflating, inflated.
- mark = object->mark();
- assert(!mark->is_neutral(), "invariant");
- if (mark->has_locker() && mark != markOopDesc::INFLATING()) {
- assert(THREAD->is_lock_owned((address)mark->locker()), "invariant");
+ // If the displaced header is NULL, then this exit matches up with
+ // a recursive enter. No real work to do here except for diagnostics.
+#ifndef PRODUCT
+ if (mark != markOopDesc::INFLATING()) {
+ // Only do diagnostics if we are not racing an inflation. Simply
+ // exiting a recursive enter of a Java Monitor that is being
+ // inflated is safe; see the has_monitor() comment below.
+ assert(!mark->is_neutral(), "invariant");
+ assert(!mark->has_locker() ||
+ THREAD->is_lock_owned((address)mark->locker()), "invariant");
+ if (mark->has_monitor()) {
+ // The BasicLock's displaced_header is marked as a recursive
+ // enter and we have an inflated Java Monitor (ObjectMonitor).
+ // This is a special case where the Java Monitor was inflated
+ // after this thread entered the stack-lock recursively. When a
+ // Java Monitor is inflated, we cannot safely walk the Java
+ // Monitor owner's stack and update the BasicLocks because a
+ // Java Monitor can be asynchronously inflated by a thread that
+ // does not own the Java Monitor.
+ ObjectMonitor * m = mark->monitor();
+ assert(((oop)(m->object()))->mark() == mark, "invariant");
+ assert(m->is_entered(THREAD), "invariant");
+ }
}
- if (mark->has_monitor()) {
- ObjectMonitor * m = mark->monitor();
- assert(((oop)(m->object()))->mark() == mark, "invariant");
- assert(m->is_entered(THREAD), "invariant");
- }
+#endif
return;
}
- mark = object->mark();
-
- // If the object is stack-locked by the current thread, try to
- // swing the displaced header from the box back to the mark.
if (mark == (markOop) lock) {
+ // If the object is stack-locked by the current thread, try to
+ // swing the displaced header from the BasicLock back to the mark.
assert(dhw->is_neutral(), "invariant");
- if ((markOop) Atomic::cmpxchg_ptr (dhw, object->mark_addr(), mark) == mark) {
- TEVENT(fast_exit: release stacklock);
+ if ((markOop) Atomic::cmpxchg_ptr(dhw, object->mark_addr(), mark) == mark) {
+ TEVENT(fast_exit: release stack-lock);
return;
}
}
+ // We have to take the slow-path of possible inflation and then exit.
ObjectSynchronizer::inflate(THREAD,
object,
inflate_cause_vm_internal)->exit(true, THREAD);
@@ -1747,6 +1761,7 @@
void do_monitor(ObjectMonitor* mid) {
if (mid->owner() == THREAD) {
if (ObjectMonitor::Knob_VerifyMatch != 0) {
+ ResourceMark rm;
Handle obj((oop) mid->object());
tty->print("INFO: unexpected locked object:");
javaVFrame::print_locked_object_class_name(tty, obj, "locked");