Skip to content
Permalink
Browse files

8236035: refactor ObjectMonitor::set_owner() and _owner field setting

Reviewed-by: dholmes, kbarrett, rehn
  • Loading branch information
Daniel D. Daugherty
Daniel D. Daugherty committed Feb 5, 2020
1 parent b9e3a4e commit 8ff24c55ef37c61549b48f67b9d93be8d24bd2ce
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2015, 2020, 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
@@ -117,6 +117,7 @@
LOG_TAG(oops) \
LOG_TAG(oopstorage) \
LOG_TAG(os) \
LOG_TAG(owner) \
LOG_TAG(pagesize) \
LOG_TAG(patch) \
LOG_TAG(path) \
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1998, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1998, 2020, 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
@@ -245,7 +245,7 @@ void ObjectMonitor::enter(TRAPS) {
// and to reduce RTS->RTO cache line upgrades on SPARC and IA32 processors.
Thread * const Self = THREAD;

void * cur = Atomic::cmpxchg(&_owner, (void*)NULL, Self);
void* cur = try_set_owner_from(NULL, Self);
if (cur == NULL) {
assert(_recursions == 0, "invariant");
return;
@@ -260,9 +260,7 @@ void ObjectMonitor::enter(TRAPS) {
if (Self->is_lock_owned((address)cur)) {
assert(_recursions == 0, "internal state error");
_recursions = 1;
// Commute owner from a thread-specific on-stack BasicLockObject address to
// a full-fledged "Thread *".
_owner = Self;
set_owner_from_BasicLock(cur, Self); // Convert from BasicLock* to Thread*.
return;
}

@@ -403,7 +401,7 @@ void ObjectMonitor::enter(TRAPS) {
int ObjectMonitor::TryLock(Thread * Self) {
void * own = _owner;
if (own != NULL) return 0;
if (Atomic::replace_if_null(&_owner, Self)) {
if (try_set_owner_from(NULL, Self) == NULL) {
assert(_recursions == 0, "invariant");
return 1;
}
@@ -862,15 +860,12 @@ void ObjectMonitor::UnlinkAfterAcquire(Thread *Self, ObjectWaiter *SelfNode) {
// of such futile wakups is low.

void ObjectMonitor::exit(bool not_suspended, TRAPS) {
Thread * const Self = THREAD;
if (THREAD != _owner) {
if (THREAD->is_lock_owned((address) _owner)) {
// Transmute _owner from a BasicLock pointer to a Thread address.
// We don't need to hold _mutex for this transition.
// Non-null to Non-null is safe as long as all readers can
// tolerate either flavor.
Thread* const Self = THREAD;
void* cur = Atomic::load(&_owner);
if (THREAD != cur) {
if (THREAD->is_lock_owned((address)cur)) {
assert(_recursions == 0, "invariant");
_owner = THREAD;
set_owner_from_BasicLock(cur, Self); // Convert from BasicLock* to Thread*.
_recursions = 0;
} else {
// Apparent unbalanced locking ...
@@ -914,10 +909,15 @@ void ObjectMonitor::exit(bool not_suspended, TRAPS) {
for (;;) {
assert(THREAD == _owner, "invariant");

// Drop the lock.
// release semantics: prior loads and stores from within the critical section
// must not float (reorder) past the following store that drops the lock.
Atomic::release_store(&_owner, (void*)NULL); // drop the lock
OrderAccess::storeload(); // See if we need to wake a successor
// Uses a storeload to separate release_store(owner) from the
// successor check. The try_set_owner() below uses cmpxchg() so
// we get the fence down there.
release_clear_owner(Self);
OrderAccess::storeload();

if ((intptr_t(_EntryList)|intptr_t(_cxq)) == 0 || _succ != NULL) {
return;
}
@@ -959,7 +959,7 @@ void ObjectMonitor::exit(bool not_suspended, TRAPS) {
// to reacquire the lock the responsibility for ensuring succession
// falls to the new owner.
//
if (!Atomic::replace_if_null(&_owner, THREAD)) {
if (try_set_owner_from(NULL, Self) != NULL) {
return;
}

@@ -1092,8 +1092,9 @@ void ObjectMonitor::ExitEpilog(Thread * Self, ObjectWaiter * Wakee) {
Wakee = NULL;

// Drop the lock
Atomic::release_store(&_owner, (void*)NULL);
OrderAccess::fence(); // ST _owner vs LD in unpark()
// Uses a fence to separate release_store(owner) from the LD in unpark().
release_clear_owner(Self);
OrderAccess::fence();

DTRACE_MONITOR_PROBE(contended__exit, this, object(), Self);
Trigger->unpark();
@@ -1119,10 +1120,11 @@ intx ObjectMonitor::complete_exit(TRAPS) {

assert(InitDone, "Unexpectedly not initialized");

if (THREAD != _owner) {
if (THREAD->is_lock_owned ((address)_owner)) {
void* cur = Atomic::load(&_owner);
if (THREAD != cur) {
if (THREAD->is_lock_owned((address)cur)) {
assert(_recursions == 0, "internal state error");
_owner = THREAD; // Convert from basiclock addr to Thread addr
set_owner_from_BasicLock(cur, Self); // Convert from BasicLock* to Thread*.
_recursions = 0;
}
}
@@ -1167,11 +1169,12 @@ void ObjectMonitor::reenter(intx recursions, TRAPS) {
// (IMSE). If there is a pending exception and the specified thread
// is not the owner, that exception will be replaced by the IMSE.
bool ObjectMonitor::check_owner(Thread* THREAD) {
if (_owner == THREAD) {
void* cur = Atomic::load(&_owner);
if (cur == THREAD) {
return true;
}
if (THREAD->is_lock_owned((address)_owner)) {
_owner = THREAD; // convert from BasicLock addr to Thread addr
if (THREAD->is_lock_owned((address)cur)) {
set_owner_from_BasicLock(cur, THREAD); // Convert from BasicLock* to Thread*.
_recursions = 0;
return true;
}
@@ -1680,7 +1683,7 @@ int ObjectMonitor::TrySpin(Thread * Self) {

Thread * ox = (Thread *) _owner;
if (ox == NULL) {
ox = (Thread*)Atomic::cmpxchg(&_owner, (void*)NULL, Self);
ox = (Thread*)try_set_owner_from(NULL, Self);
if (ox == NULL) {
// The CAS succeeded -- this thread acquired ownership
// Take care of some bookkeeping to exit spin state.
@@ -238,7 +238,16 @@ class ObjectMonitor {
intptr_t is_entered(Thread* current) const;

void* owner() const;
void set_owner(void* owner);
// Clear _owner field; current value must match old_value.
void release_clear_owner(void* old_value);
// Simply set _owner field to new_value; current value must match old_value.
void set_owner_from(void* old_value, void* new_value);
// Simply set _owner field to self; current value must match basic_lock_p.
void set_owner_from_BasicLock(void* basic_lock_p, Thread* self);
// Try to set _owner field to new_value if the current value matches
// old_value, using Atomic::cmpxchg(). Otherwise, does not change the
// _owner field. Returns the prior value of the _owner field.
void* try_set_owner_from(void* old_value, void* new_value);

jint waiters() const;

@@ -1,5 +1,5 @@
/*
* Copyright (c) 1998, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1998, 2020, 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
@@ -25,6 +25,7 @@
#ifndef SHARE_RUNTIME_OBJECTMONITOR_INLINE_HPP
#define SHARE_RUNTIME_OBJECTMONITOR_INLINE_HPP

#include "logging/log.hpp"
#include "runtime/atomic.hpp"

inline intptr_t ObjectMonitor::is_entered(TRAPS) const {
@@ -84,8 +85,56 @@ inline jint ObjectMonitor::contentions() const {
return _contentions;
}

inline void ObjectMonitor::set_owner(void* owner) {
_owner = owner;
// Clear _owner field; current value must match old_value.
inline void ObjectMonitor::release_clear_owner(void* old_value) {
DEBUG_ONLY(void* prev = Atomic::load(&_owner);)
assert(prev == old_value, "unexpected prev owner=" INTPTR_FORMAT
", expected=" INTPTR_FORMAT, p2i(prev), p2i(old_value));
Atomic::release_store(&_owner, (void*)NULL);
log_trace(monitorinflation, owner)("release_clear_owner(): mid="
INTPTR_FORMAT ", old_value=" INTPTR_FORMAT,
p2i(this), p2i(old_value));
}

// Simply set _owner field to new_value; current value must match old_value.
// (Simple means no memory sync needed.)
inline void ObjectMonitor::set_owner_from(void* old_value, void* new_value) {
DEBUG_ONLY(void* prev = Atomic::load(&_owner);)
assert(prev == old_value, "unexpected prev owner=" INTPTR_FORMAT
", expected=" INTPTR_FORMAT, p2i(prev), p2i(old_value));
Atomic::store(&_owner, new_value);
log_trace(monitorinflation, owner)("set_owner_from(): mid="
INTPTR_FORMAT ", old_value=" INTPTR_FORMAT
", new_value=" INTPTR_FORMAT, p2i(this),
p2i(old_value), p2i(new_value));
}

// Simply set _owner field to self; current value must match basic_lock_p.
inline void ObjectMonitor::set_owner_from_BasicLock(void* basic_lock_p, Thread* self) {
DEBUG_ONLY(void* prev = Atomic::load(&_owner);)
assert(prev == basic_lock_p, "unexpected prev owner=" INTPTR_FORMAT
", expected=" INTPTR_FORMAT, p2i(prev), p2i(basic_lock_p));
// Non-null owner field to non-null owner field is safe without
// cmpxchg() as long as all readers can tolerate either flavor.
Atomic::store(&_owner, self);
log_trace(monitorinflation, owner)("set_owner_from_BasicLock(): mid="
INTPTR_FORMAT ", basic_lock_p="
INTPTR_FORMAT ", new_value=" INTPTR_FORMAT,
p2i(this), p2i(basic_lock_p), p2i(self));
}

// Try to set _owner field to new_value if the current value matches
// old_value. Otherwise, does not change the _owner field. Returns
// the prior value of the _owner field.
inline void* ObjectMonitor::try_set_owner_from(void* old_value, void* new_value) {
void* prev = Atomic::cmpxchg(&_owner, old_value, new_value);
if (prev == old_value) {
log_trace(monitorinflation, owner)("try_set_owner_from(): mid="
INTPTR_FORMAT ", prev=" INTPTR_FORMAT
", new=" INTPTR_FORMAT, p2i(this),
p2i(prev), p2i(new_value));
}
return prev;
}

#endif // SHARE_RUNTIME_OBJECTMONITOR_INLINE_HPP
@@ -240,7 +240,7 @@ bool ObjectSynchronizer::quick_enter(oop obj, Thread* self,
// and last are the inflated Java Monitor (ObjectMonitor) checks.
lock->set_displaced_header(markWord::unused_mark());

if (owner == NULL && Atomic::replace_if_null(&(m->_owner), self)) {
if (owner == NULL && m->try_set_owner_from(NULL, self) == NULL) {
assert(m->_recursions == 0, "invariant");
return true;
}
@@ -1404,7 +1404,7 @@ ObjectMonitor* ObjectSynchronizer::inflate(Thread* self,
// Note that a thread can inflate an object
// that it has stack-locked -- as might happen in wait() -- directly
// with CAS. That is, we can avoid the xchg-NULL .... ST idiom.
m->set_owner(mark.locker());
m->set_owner_from(NULL, mark.locker());
m->set_object(object);
// TODO-FIXME: assert BasicLock->dhw != 0.

0 comments on commit 8ff24c5

Please sign in to comment.