Skip to content

Commit

Permalink
8273917: Remove 'leaf' ranking for Mutex
Browse files Browse the repository at this point in the history
Reviewed-by: eosterlund, dholmes
  • Loading branch information
coleenp committed Oct 6, 2021
1 parent c80a612 commit b8af6a9
Show file tree
Hide file tree
Showing 11 changed files with 117 additions and 87 deletions.
2 changes: 1 addition & 1 deletion src/hotspot/share/compiler/compileTask.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ class CompileTask : public CHeapObj<mtCompiler> {

public:
CompileTask() : _failure_reason(NULL), _failure_reason_on_C_heap(false) {
_lock = new Monitor(Mutex::nonleaf+2, "CompileTaskLock", Mutex::_safepoint_check_always);
_lock = new Monitor(Mutex::nonleaf, "CompileTask_lock", Mutex::_safepoint_check_always);
}

void initialize(int compile_id, const methodHandle& method, int osr_bci, int comp_level,
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/gc/shared/space.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -773,7 +773,7 @@ void OffsetTableContigSpace::alloc_block(HeapWord* start, HeapWord* end) {
OffsetTableContigSpace::OffsetTableContigSpace(BlockOffsetSharedArray* sharedOffsetArray,
MemRegion mr) :
_offsets(sharedOffsetArray, mr),
_par_alloc_lock(Mutex::leaf, "OffsetTableContigSpace par alloc lock",
_par_alloc_lock(Mutex::nonleaf, "OffsetTableContigSpaceParAlloc_lock",
Mutex::_safepoint_check_always, true)
{
_offsets.set_contig_space(this);
Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/gc/shenandoah/shenandoahControlThread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@

ShenandoahControlThread::ShenandoahControlThread() :
ConcurrentGCThread(),
_alloc_failure_waiters_lock(Mutex::leaf, "ShenandoahAllocFailureGC_lock", Monitor::_safepoint_check_always, true),
_gc_waiters_lock(Mutex::leaf, "ShenandoahRequestedGC_lock", Monitor::_safepoint_check_always, true),
_alloc_failure_waiters_lock(Mutex::nonleaf, "ShenandoahAllocFailureGC_lock", Monitor::_safepoint_check_always, true),
_gc_waiters_lock(Mutex::nonleaf, "ShenandoahRequestedGC_lock", Monitor::_safepoint_check_always, true),
_periodic_task(this),
_requested_gc_cause(GCCause::_no_cause_specified),
_degen_point(ShenandoahGC::_degenerated_outside_cycle),
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/gc/shenandoah/shenandoahPacer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class ShenandoahPacer : public CHeapObj<mtGC> {
_heap(heap),
_last_time(os::elapsedTime()),
_progress_history(new TruncatedSeq(5)),
_wait_monitor(new Monitor(Mutex::leaf, "_wait_monitor", Monitor::_safepoint_check_always, true)),
_wait_monitor(new Monitor(Mutex::nonleaf-1, "ShenandoahWaitMonitor_lock", Monitor::_safepoint_check_always, true)),
_epoch(0),
_tax_rate(1),
_budget(0),
Expand Down
3 changes: 2 additions & 1 deletion src/hotspot/share/oops/methodData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1206,7 +1206,8 @@ void MethodData::post_initialize(BytecodeStream* stream) {
// Initialize the MethodData* corresponding to a given method.
MethodData::MethodData(const methodHandle& method)
: _method(method()),
_extra_data_lock(Mutex::leaf, "MDO extra data lock", Mutex::_safepoint_check_always),
// Holds Compile_lock
_extra_data_lock(Mutex::nonleaf-2, "MDOExtraData_lock", Mutex::_safepoint_check_always),
_compiler_counters(),
_parameters_type_data_di(parameters_uninitialized) {
initialize();
Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/runtime/mutex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,8 @@ Mutex::Mutex(int Rank, const char * name, SafepointCheckRequired safepoint_check
_safepoint_check_required = safepoint_check_required;
_skip_rank_check = false;

assert(_rank >= 0 && _rank <= nonleaf, "Bad lock rank %d: %s", _rank, name);

assert(_rank > nosafepoint || _safepoint_check_required == _safepoint_check_never,
"Locks below nosafepoint rank should never safepoint: %s", name);

Expand All @@ -295,8 +297,6 @@ Mutex::Mutex(int Rank, const char * name, SafepointCheckRequired safepoint_check
// allowing Java threads to block in native.
assert(_safepoint_check_required == _safepoint_check_always || _allow_vm_block,
"Safepoint check never locks should always allow the vm to block: %s", name);

assert(_rank >= 0, "Bad lock rank: %s", name);
#endif
}

Expand Down
6 changes: 2 additions & 4 deletions src/hotspot/share/runtime/mutex.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,8 @@ class Mutex : public CHeapObj<mtSynchronizer> {
tty = stackwatermark + 3,
oopstorage = tty + 3,
nosafepoint = oopstorage + 6,
leaf = nosafepoint + 6,
barrier = leaf + 10,
nonleaf = barrier + 1,
max_nonleaf = nonleaf + 900
nonleaf = nosafepoint + 20,
max_nonleaf = nonleaf
};

private:
Expand Down
137 changes: 83 additions & 54 deletions src/hotspot/share/runtime/mutexLocker.cpp

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions src/hotspot/share/services/heapDumper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -748,7 +748,7 @@ class ParDumpWriter : public AbstractDumpWriter {

static void before_work() {
assert(_lock == NULL, "ParDumpWriter lock must be initialized only once");
_lock = new (std::nothrow) PaddedMonitor(Mutex::leaf, "ParallelHProfWriter_lock", Mutex::_safepoint_check_always);
_lock = new (std::nothrow) PaddedMonitor(Mutex::nonleaf, "ParallelHProfWriter_lock", Mutex::_safepoint_check_always);
}

static void after_work() {
Expand Down Expand Up @@ -1814,7 +1814,7 @@ class DumperController : public CHeapObj<mtInternal> {
public:
DumperController(uint number) :
_started(false),
_lock(new (std::nothrow) PaddedMonitor(Mutex::leaf, "DumperController_lock",
_lock(new (std::nothrow) PaddedMonitor(Mutex::nonleaf, "DumperController_lock",
Mutex::_safepoint_check_always)),
_dumper_number(number),
_complete_number(0) { }
Expand Down
38 changes: 20 additions & 18 deletions test/hotspot/gtest/runtime/test_mutex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,16 @@ TEST_VM(MutexName, mutex_name) {

#ifdef ASSERT

const int rankA = 50;
const int rankA = Mutex::nonleaf-5;
const int rankAplusOne = Mutex::nonleaf-4;
const int rankAplusTwo = Mutex::nonleaf-3;

TEST_OTHER_VM(MutexRank, mutex_lock_rank_in_order) {
JavaThread* THREAD = JavaThread::current();
ThreadInVMfromNative invm(THREAD);

Mutex* mutex_rankA = new Mutex(rankA, "mutex_rankA", Mutex::_safepoint_check_always);
Mutex* mutex_rankA_plus_one = new Mutex(rankA + 1, "mutex_rankA_plus_one", Mutex::_safepoint_check_always);
Mutex* mutex_rankA_plus_one = new Mutex(rankAplusOne, "mutex_rankA_plus_one", Mutex::_safepoint_check_always);

mutex_rankA_plus_one->lock();
mutex_rankA->lock();
Expand All @@ -69,12 +71,12 @@ TEST_OTHER_VM(MutexRank, mutex_lock_rank_in_order) {
}

TEST_VM_ASSERT_MSG(MutexRank, mutex_lock_rank_out_of_orderA,
".* Attempting to acquire lock mutex_rankA_plus_one/51 out of order with lock mutex_rankA/50 -- possible deadlock") {
".* Attempting to acquire lock mutex_rankA_plus_one/.* out of order with lock mutex_rankA/.* -- possible deadlock") {
JavaThread* THREAD = JavaThread::current();
ThreadInVMfromNative invm(THREAD);

Mutex* mutex_rankA = new Mutex(rankA, "mutex_rankA", Mutex::_safepoint_check_always);
Mutex* mutex_rankA_plus_one = new Mutex(rankA + 1, "mutex_rankA_plus_one", Mutex::_safepoint_check_always);
Mutex* mutex_rankA_plus_one = new Mutex(rankAplusOne, "mutex_rankA_plus_one", Mutex::_safepoint_check_always);

mutex_rankA->lock();
mutex_rankA_plus_one->lock();
Expand All @@ -83,7 +85,7 @@ TEST_VM_ASSERT_MSG(MutexRank, mutex_lock_rank_out_of_orderA,
}

TEST_VM_ASSERT_MSG(MutexRank, mutex_lock_rank_out_of_orderB,
".* Attempting to acquire lock mutex_rankB/50 out of order with lock mutex_rankA/50 -- possible deadlock") {
".* Attempting to acquire lock mutex_rankB/.* out of order with lock mutex_rankA/.* -- possible deadlock") {
JavaThread* THREAD = JavaThread::current();
ThreadInVMfromNative invm(THREAD);

Expand All @@ -101,8 +103,8 @@ TEST_OTHER_VM(MutexRank, mutex_trylock_rank_out_of_orderA) {
ThreadInVMfromNative invm(THREAD);

Mutex* mutex_rankA = new Mutex(rankA, "mutex_rankA", Mutex::_safepoint_check_always);
Mutex* mutex_rankA_plus_one = new Mutex(rankA + 1, "mutex_rankA_plus_one", Mutex::_safepoint_check_always);
Mutex* mutex_rankA_plus_two = new Mutex(rankA + 2, "mutex_rankA_plus_two", Mutex::_safepoint_check_always);
Mutex* mutex_rankA_plus_one = new Mutex(rankAplusOne, "mutex_rankA_plus_one", Mutex::_safepoint_check_always);
Mutex* mutex_rankA_plus_two = new Mutex(rankAplusTwo, "mutex_rankA_plus_two", Mutex::_safepoint_check_always);

mutex_rankA_plus_one->lock();
mutex_rankA_plus_two->try_lock_without_rank_check();
Expand All @@ -113,12 +115,12 @@ TEST_OTHER_VM(MutexRank, mutex_trylock_rank_out_of_orderA) {
}

TEST_VM_ASSERT_MSG(MutexRank, mutex_trylock_rank_out_of_orderB,
".* Attempting to acquire lock mutex_rankA_plus_one/51 out of order with lock mutex_rankA/50 -- possible deadlock") {
".* Attempting to acquire lock mutex_rankA_plus_one/.* out of order with lock mutex_rankA/.* -- possible deadlock") {
JavaThread* THREAD = JavaThread::current();
ThreadInVMfromNative invm(THREAD);

Mutex* mutex_rankA = new Mutex(rankA, "mutex_rankA", Mutex::_safepoint_check_always);
Mutex* mutex_rankA_plus_one = new Mutex(rankA + 1, "mutex_rankA_plus_one", Mutex::_safepoint_check_always);
Mutex* mutex_rankA_plus_one = new Mutex(rankAplusOne, "mutex_rankA_plus_one", Mutex::_safepoint_check_always);

mutex_rankA->lock();
mutex_rankA_plus_one->try_lock_without_rank_check();
Expand Down Expand Up @@ -163,7 +165,7 @@ TEST_OTHER_VM(MutexRank, monitor_wait_rank_in_order) {
ThreadInVMfromNative invm(THREAD);

Monitor* monitor_rankA = new Monitor(rankA, "monitor_rankA", Mutex::_safepoint_check_always);
Monitor* monitor_rankA_plus_one = new Monitor(rankA + 1, "monitor_rankA_plus_one", Mutex::_safepoint_check_always);
Monitor* monitor_rankA_plus_one = new Monitor(rankAplusOne, "monitor_rankA_plus_one", Mutex::_safepoint_check_always);

monitor_rankA_plus_one->lock();
monitor_rankA->lock();
Expand All @@ -173,13 +175,13 @@ TEST_OTHER_VM(MutexRank, monitor_wait_rank_in_order) {
}

TEST_VM_ASSERT_MSG(MutexRank, monitor_wait_rank_out_of_order,
".* Attempting to wait on monitor monitor_rankA_plus_one/51 while holding lock monitor_rankA/50 "
".* Attempting to wait on monitor monitor_rankA_plus_one/.* while holding lock monitor_rankA/.* "
"-- possible deadlock. Should wait on the least ranked monitor from all owned locks.") {
JavaThread* THREAD = JavaThread::current();
ThreadInVMfromNative invm(THREAD);

Monitor* monitor_rankA = new Monitor(rankA, "monitor_rankA", Mutex::_safepoint_check_always);
Monitor* monitor_rankA_plus_one = new Monitor(rankA + 1, "monitor_rankA_plus_one", Mutex::_safepoint_check_always);
Monitor* monitor_rankA_plus_one = new Monitor(rankAplusOne, "monitor_rankA_plus_one", Mutex::_safepoint_check_always);

monitor_rankA_plus_one->lock();
monitor_rankA->lock();
Expand All @@ -189,13 +191,13 @@ TEST_VM_ASSERT_MSG(MutexRank, monitor_wait_rank_out_of_order,
}

TEST_VM_ASSERT_MSG(MutexRank, monitor_wait_rank_out_of_order_trylock,
".* Attempting to wait on monitor monitor_rankA_plus_one/51 while holding lock monitor_rankA/50 "
".* Attempting to wait on monitor monitor_rankA_plus_one/.* while holding lock monitor_rankA/.* "
"-- possible deadlock. Should wait on the least ranked monitor from all owned locks.") {
JavaThread* THREAD = JavaThread::current();
ThreadInVMfromNative invm(THREAD);

Monitor* monitor_rankA = new Monitor(rankA, "monitor_rankA", Mutex::_safepoint_check_always);
Monitor* monitor_rankA_plus_one = new Monitor(rankA + 1, "monitor_rankA_plus_one", Mutex::_safepoint_check_always);
Monitor* monitor_rankA_plus_one = new Monitor(rankAplusOne, "monitor_rankA_plus_one", Mutex::_safepoint_check_always);

monitor_rankA->lock();
monitor_rankA_plus_one->try_lock_without_rank_check();
Expand Down Expand Up @@ -280,12 +282,12 @@ TEST_VM_ASSERT_MSG(MutexRank, monitor_negative_rank,
}

TEST_VM_ASSERT_MSG(MutexRank, monitor_nosafepoint_rank,
".*failed: Locks above nosafepoint rank should safepoint: monitor_rank_leaf") {
".*failed: Locks above nosafepoint rank should safepoint: monitor_rank_nonleaf") {
JavaThread* THREAD = JavaThread::current();
ThreadInVMfromNative invm(THREAD);

Monitor* monitor_rank_leaf = new Monitor(Mutex::leaf, "monitor_rank_leaf", Mutex::_safepoint_check_never);
monitor_rank_leaf->lock_without_safepoint_check();
monitor_rank_leaf->unlock();
Monitor* monitor_rank_nonleaf = new Monitor(Mutex::nonleaf, "monitor_rank_nonleaf", Mutex::_safepoint_check_never);
monitor_rank_nonleaf->lock_without_safepoint_check();
monitor_rank_nonleaf->unlock();
}
#endif // ASSERT
2 changes: 1 addition & 1 deletion test/hotspot/gtest/runtime/test_safepoint_locks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
// Test mismatched safepoint check flag on lock declaration vs. lock acquisition.
TEST_VM_ASSERT_MSG(SafepointLockAssertTest, always_check,
".*This lock should always have a safepoint check for Java threads: SFPT_Test_lock") {
MutexLocker ml(new Mutex(Mutex::leaf, "SFPT_Test_lock", Mutex::_safepoint_check_always),
MutexLocker ml(new Mutex(Mutex::nonleaf, "SFPT_Test_lock", Mutex::_safepoint_check_always),
Mutex::_no_safepoint_check_flag);
}

Expand Down

1 comment on commit b8af6a9

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.