Skip to content
Permalink
Browse files
8256306: ObjectMonitor::_contentions field should not be 'jint'
Reviewed-by: dholmes, stuefe, dcubed
  • Loading branch information
coleenp committed Jun 23, 2021
1 parent 52d5d1b commit f3ba2690c5b34673ebf6836c87e45477e1ff91c3
@@ -485,7 +485,7 @@ bool ObjectMonitor::enter(JavaThread* current) {
// just exited the monitor.
}
if (event.should_commit()) {
event.set_previousOwner((uintptr_t)_previous_owner_tid);
event.set_previousOwner(_previous_owner_tid);
event.commit();
}
OM_PERFDATA_OP(ContendedLockAttempts, inc());
@@ -545,7 +545,7 @@ bool ObjectMonitor::deflate_monitor() {
// Java threads. The GC already broke the association with the object.
set_owner_from(NULL, DEFLATER_MARKER);
assert(contentions() >= 0, "must be non-negative: contentions=%d", contentions());
_contentions = -max_jint;
_contentions = INT_MIN; // minimum negative int
} else {
// Attempt async deflation protocol.

@@ -572,7 +572,7 @@ bool ObjectMonitor::deflate_monitor() {

// Make a zero contentions field negative to force any contending threads
// to retry. This is the second part of the async deflation dance.
if (Atomic::cmpxchg(&_contentions, (jint)0, -max_jint) != 0) {
if (Atomic::cmpxchg(&_contentions, 0, INT_MIN) != 0) {
// Contentions was no longer 0 so we lost the race since the
// ObjectMonitor is now busy. Restore owner to NULL if it is
// still DEFLATER_MARKER:
@@ -2243,7 +2243,7 @@ void ObjectMonitor::print_debug_style_on(outputStream* st) const {
st->print_cr(" [%d] = '\\0'", (int)sizeof(_pad_buf0) - 1);
st->print_cr(" }");
st->print_cr(" _owner = " INTPTR_FORMAT, p2i(owner_raw()));
st->print_cr(" _previous_owner_tid = " JLONG_FORMAT, _previous_owner_tid);
st->print_cr(" _previous_owner_tid = " INTPTR_FORMAT, _previous_owner_tid);
st->print_cr(" _pad_buf1 = {");
st->print_cr(" [0] = '\\0'");
st->print_cr(" ...");
@@ -148,13 +148,13 @@ class ObjectMonitor : public CHeapObj<mtInternal> {
// Used by async deflation as a marker in the _owner field:
#define DEFLATER_MARKER reinterpret_cast<void*>(-1)
void* volatile _owner; // pointer to owning thread OR BasicLock
volatile jlong _previous_owner_tid; // thread id of the previous owner of the monitor
volatile uintptr_t _previous_owner_tid; // thread id of the previous owner of the monitor
// Separate _owner and _next_om on different cache lines since
// both can have busy multi-threaded access. _previous_owner_tid is only
// changed by ObjectMonitor::exit() so it is a good choice to share the
// cache line with _owner.
DEFINE_PAD_MINUS_SIZE(1, OM_CACHE_LINE_SIZE, sizeof(void* volatile) +
sizeof(volatile jlong));
sizeof(volatile uintptr_t));
ObjectMonitor* _next_om; // Next ObjectMonitor* linkage
volatile intx _recursions; // recursion count, 0 for first entry
ObjectWaiter* volatile _EntryList; // Threads blocked on entry or reentry.
@@ -168,13 +168,13 @@ class ObjectMonitor : public CHeapObj<mtInternal> {
volatile int _Spinner; // for exit->spinner handoff optimization
volatile int _SpinDuration;

jint _contentions; // Number of active contentions in enter(). It is used by is_busy()
int _contentions; // Number of active contentions in enter(). It is used by is_busy()
// along with other fields to determine if an ObjectMonitor can be
// deflated. It is also used by the async deflation protocol. See
// ObjectMonitor::deflate_monitor().
protected:
ObjectWaiter* volatile _WaitSet; // LL of threads wait()ing on the monitor
volatile jint _waiters; // number of waiting threads
volatile int _waiters; // number of waiting threads
private:
volatile int _WaitSetLock; // protects Wait Queue - simple spinlock

@@ -238,9 +238,10 @@ class ObjectMonitor : public CHeapObj<mtInternal> {

bool is_busy() const {
// TODO-FIXME: assert _owner == null implies _recursions = 0
intptr_t ret_code = _waiters | intptr_t(_cxq) | intptr_t(_EntryList);
if (contentions() > 0) {
ret_code |= contentions();
intptr_t ret_code = intptr_t(_waiters) | intptr_t(_cxq) | intptr_t(_EntryList);
int cnts = contentions(); // read once
if (cnts > 0) {
ret_code |= intptr_t(cnts);
}
if (!owner_is_DEFLATER_MARKER()) {
ret_code |= intptr_t(owner_raw());
@@ -281,10 +282,10 @@ class ObjectMonitor : public CHeapObj<mtInternal> {
// _next_om field. Returns the prior value of the _next_om field.
ObjectMonitor* try_set_next_om(ObjectMonitor* old_value, ObjectMonitor* new_value);

jint waiters() const;
int waiters() const;

jint contentions() const;
void add_to_contentions(jint value);
int contentions() const;
void add_to_contentions(int value);
intx recursions() const { return _recursions; }

// JVM/TI GetObjectMonitorUsage() needs this:
@@ -52,7 +52,7 @@ inline void ObjectMonitor::set_header(markWord hdr) {
Atomic::store(&_header, hdr);
}

inline jint ObjectMonitor::waiters() const {
inline int ObjectMonitor::waiters() const {
return _waiters;
}

@@ -79,12 +79,12 @@ inline bool ObjectMonitor::is_being_async_deflated() {
}

// Return number of threads contending for this monitor.
inline jint ObjectMonitor::contentions() const {
inline int ObjectMonitor::contentions() const {
return Atomic::load(&_contentions);
}

// Add value to the contentions field.
inline void ObjectMonitor::add_to_contentions(jint value) {
inline void ObjectMonitor::add_to_contentions(int value) {
Atomic::add(&_contentions, value);
}

@@ -881,8 +881,8 @@ typedef HashtableEntry<InstanceKlass*, mtClass> KlassHashtableEntry;
unchecked_nonstatic_field(ObjectMonitor, _owner, sizeof(void *)) /* NOTE: no type */ \
volatile_nonstatic_field(ObjectMonitor, _next_om, ObjectMonitor*) \
volatile_nonstatic_field(BasicLock, _displaced_header, markWord) \
nonstatic_field(ObjectMonitor, _contentions, jint) \
volatile_nonstatic_field(ObjectMonitor, _waiters, jint) \
nonstatic_field(ObjectMonitor, _contentions, int) \
volatile_nonstatic_field(ObjectMonitor, _waiters, int) \
volatile_nonstatic_field(ObjectMonitor, _recursions, intx) \
nonstatic_field(BasicObjectLock, _lock, BasicLock) \
nonstatic_field(BasicObjectLock, _obj, oop) \
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2001, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2001, 2021, 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
@@ -52,9 +52,9 @@ private static synchronized void initialize(TypeDataBase db) throws WrongTypeExc
ownerFieldOffset = f.getOffset();
f = type.getField("_next_om");
nextOMFieldOffset = f.getOffset();
contentionsField = type.getJIntField("_contentions");
waitersField = type.getJIntField("_waiters");
recursionsField = type.getCIntegerField("_recursions");
contentionsField = new CIntField(type.getCIntegerField("_contentions"), 0);
waitersField = new CIntField(type.getCIntegerField("_waiters"), 0);
recursionsField = type.getCIntegerField("_recursions");
}

public ObjectMonitor(Address addr) {
@@ -83,7 +83,7 @@ public boolean isEntered(sun.jvm.hotspot.runtime.Thread current) {
// FIXME
// void set_owner(void* owner);

public int waiters() { return waitersField.getValue(addr); }
public int waiters() { return (int)waitersField.getValue(this); }

public Address nextOM() { return addr.getAddressAt(nextOMFieldOffset); }
// FIXME
@@ -100,7 +100,7 @@ public OopHandle object() {
}

public int contentions() {
return contentionsField.getValue(addr);
return (int)contentionsField.getValue(this);
}

// The following four either aren't expressed as typed fields in
@@ -111,8 +111,8 @@ public int contentions() {
private static long objectFieldOffset;
private static long ownerFieldOffset;
private static long nextOMFieldOffset;
private static JIntField contentionsField;
private static JIntField waitersField;
private static CIntField contentionsField;
private static CIntField waitersField;
private static CIntegerField recursionsField;
// FIXME: expose platform-dependent stuff
}

1 comment on commit f3ba269

@openjdk-notifier

This comment has been minimized.

Copy link

@openjdk-notifier openjdk-notifier bot commented on f3ba269 Jun 23, 2021

Please sign in to comment.