Skip to content

Commit

Permalink
8268347: C2: nested locks optimization may create unbalanced monitor …
Browse files Browse the repository at this point in the history
…enter/exit code

Reviewed-by: mdoerr
Backport-of: 4d8b5c7
  • Loading branch information
rwestrel committed Jul 15, 2021
1 parent 93f952c commit 7a61e03
Show file tree
Hide file tree
Showing 11 changed files with 368 additions and 39 deletions.
16 changes: 15 additions & 1 deletion src/hotspot/share/opto/c2compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ const char* C2Compiler::retry_no_subsuming_loads() {
const char* C2Compiler::retry_no_escape_analysis() {
return "retry without escape analysis";
}
const char* C2Compiler::retry_no_locks_coarsening() {
return "retry without locks coarsening";
}
const char* C2Compiler::retry_class_loading_during_parsing() {
return "retry class loading during parsing";
}
Expand Down Expand Up @@ -104,10 +107,11 @@ void C2Compiler::compile_method(ciEnv* env, ciMethod* target, int entry_bci, Dir
bool do_escape_analysis = DoEscapeAnalysis && !env->should_retain_local_variables()
&& !env->jvmti_can_get_owned_monitor_info();
bool eliminate_boxing = EliminateAutoBox;
bool do_locks_coarsening = EliminateLocks;

while (!env->failing()) {
// Attempt to compile while subsuming loads into machine instructions.
Compile C(env, this, target, entry_bci, subsume_loads, do_escape_analysis, eliminate_boxing, directive);
Compile C(env, this, target, entry_bci, subsume_loads, do_escape_analysis, eliminate_boxing, do_locks_coarsening, directive);

// Check result and retry if appropriate.
if (C.failure_reason() != NULL) {
Expand All @@ -127,6 +131,12 @@ void C2Compiler::compile_method(ciEnv* env, ciMethod* target, int entry_bci, Dir
env->report_failure(C.failure_reason());
continue; // retry
}
if (C.failure_reason_is(retry_no_locks_coarsening())) {
assert(do_locks_coarsening, "must make progress");
do_locks_coarsening = false;
env->report_failure(C.failure_reason());
continue; // retry
}
if (C.has_boxed_value()) {
// Recompile without boxing elimination regardless failure reason.
assert(eliminate_boxing, "must make progress");
Expand All @@ -148,6 +158,10 @@ void C2Compiler::compile_method(ciEnv* env, ciMethod* target, int entry_bci, Dir
do_escape_analysis = false;
continue; // retry
}
if (do_locks_coarsening) {
do_locks_coarsening = false;
continue; // retry
}
}

// print inlining for last compilation only
Expand Down
3 changes: 2 additions & 1 deletion src/hotspot/share/opto/c2compiler.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1999, 2015, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1999, 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
Expand Down Expand Up @@ -48,6 +48,7 @@ class C2Compiler : public AbstractCompiler {
// sentinel value used to trigger backtracking in compile_method().
static const char* retry_no_subsuming_loads();
static const char* retry_no_escape_analysis();
static const char* retry_no_locks_coarsening();
static const char* retry_class_loading_during_parsing();

// Print compilation timers and statistics
Expand Down
66 changes: 48 additions & 18 deletions src/hotspot/share/opto/callnode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1772,6 +1772,12 @@ bool AbstractLockNode::find_unlocks_for_region(const RegionNode* region, LockNod

}

const char* AbstractLockNode::_kind_names[] = {"Regular", "NonEscObj", "Coarsened", "Nested"};

const char * AbstractLockNode::kind_as_string() const {
return _kind_names[_kind];
}

#ifndef PRODUCT
//
// Create a counter which counts the number of times this lock is acquired
Expand All @@ -1789,8 +1795,6 @@ void AbstractLockNode::set_eliminated_lock_counter() {
}
}

const char* AbstractLockNode::_kind_names[] = {"Regular", "NonEscObj", "Coarsened", "Nested"};

void AbstractLockNode::dump_spec(outputStream* st) const {
st->print("%s ", _kind_names[_kind]);
CallNode::dump_spec(st);
Expand Down Expand Up @@ -1842,6 +1846,9 @@ Node *LockNode::Ideal(PhaseGVN *phase, bool can_reshape) {
return result;
}

if (!phase->C->do_locks_coarsening()) {
return result; // Compiling without locks coarsening
}
//
// Try lock coarsening
//
Expand Down Expand Up @@ -1879,17 +1886,21 @@ Node *LockNode::Ideal(PhaseGVN *phase, bool can_reshape) {
if (PrintEliminateLocks) {
int locks = 0;
int unlocks = 0;
if (Verbose) {
tty->print_cr("=== Locks coarsening ===");
}
for (int i = 0; i < lock_ops.length(); i++) {
AbstractLockNode* lock = lock_ops.at(i);
if (lock->Opcode() == Op_Lock)
locks++;
else
unlocks++;
if (Verbose) {
lock->dump(1);
tty->print(" %d: ", i);
lock->dump();
}
}
tty->print_cr("***Eliminated %d unlocks and %d locks", unlocks, locks);
tty->print_cr("=== Coarsened %d unlocks and %d locks", unlocks, locks);
}
#endif

Expand All @@ -1904,6 +1915,8 @@ Node *LockNode::Ideal(PhaseGVN *phase, bool can_reshape) {
#endif
lock->set_coarsened();
}
// Record this coarsened group.
phase->C->add_coarsened_locks(lock_ops);
} else if (ctrl->is_Region() &&
iter->_worklist.member(ctrl)) {
// We weren't able to find any opportunities but the region this
Expand Down Expand Up @@ -1937,15 +1950,34 @@ bool LockNode::is_nested_lock_region(Compile * c) {
// Ignore complex cases: merged locks or multiple locks.
Node* obj = obj_node();
LockNode* unique_lock = NULL;
if (!box->is_simple_lock_region(&unique_lock, obj)) {
Node* bad_lock = NULL;
if (!box->is_simple_lock_region(&unique_lock, obj, &bad_lock)) {
#ifdef ASSERT
this->log_lock_optimization(c, "eliminate_lock_INLR_2a");
this->log_lock_optimization(c, "eliminate_lock_INLR_2a", bad_lock);
#endif
return false;
}
if (unique_lock != this) {
#ifdef ASSERT
this->log_lock_optimization(c, "eliminate_lock_INLR_2b");
this->log_lock_optimization(c, "eliminate_lock_INLR_2b", (unique_lock != NULL ? unique_lock : bad_lock));
if (PrintEliminateLocks && Verbose) {
tty->print_cr("=============== unique_lock != this ============");
tty->print(" this: ");
this->dump();
tty->print(" box: ");
box->dump();
tty->print(" obj: ");
obj->dump();
if (unique_lock != NULL) {
tty->print(" unique_lock: ");
unique_lock->dump();
}
if (bad_lock != NULL) {
tty->print(" bad_lock: ");
bad_lock->dump();
}
tty->print_cr("===============");
}
#endif
return false;
}
Expand Down Expand Up @@ -2012,23 +2044,21 @@ Node *UnlockNode::Ideal(PhaseGVN *phase, bool can_reshape) {
return result;
}

const char * AbstractLockNode::kind_as_string() const {
return is_coarsened() ? "coarsened" :
is_nested() ? "nested" :
is_non_esc_obj() ? "non_escaping" :
"?";
}

void AbstractLockNode::log_lock_optimization(Compile *C, const char * tag) const {
void AbstractLockNode::log_lock_optimization(Compile *C, const char * tag, Node* bad_lock) const {
if (C == NULL) {
return;
}
CompileLog* log = C->log();
if (log != NULL) {
log->begin_head("%s lock='%d' compile_id='%d' class_id='%s' kind='%s'",
tag, is_Lock(), C->compile_id(),
Node* box = box_node();
Node* obj = obj_node();
int box_id = box != NULL ? box->_idx : -1;
int obj_id = obj != NULL ? obj->_idx : -1;

log->begin_head("%s compile_id='%d' lock_id='%d' class='%s' kind='%s' box_id='%d' obj_id='%d' bad_id='%d'",
tag, C->compile_id(), this->_idx,
is_Unlock() ? "unlock" : is_Lock() ? "lock" : "?",
kind_as_string());
kind_as_string(), box_id, obj_id, (bad_lock != NULL ? bad_lock->_idx : -1));
log->stamp();
log->end_head();
JVMState* p = is_Unlock() ? (as_Unlock()->dbg_jvms()) : jvms();
Expand Down
6 changes: 4 additions & 2 deletions src/hotspot/share/opto/callnode.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -985,9 +985,11 @@ class AbstractLockNode: public CallNode {
Coarsened, // Lock was coarsened
Nested // Nested lock
} _kind;

static const char* _kind_names[Nested+1];

#ifndef PRODUCT
NamedCounter* _counter;
static const char* _kind_names[Nested+1];
#endif

protected:
Expand Down Expand Up @@ -1030,7 +1032,7 @@ class AbstractLockNode: public CallNode {
bool is_nested() const { return (_kind == Nested); }

const char * kind_as_string() const;
void log_lock_optimization(Compile* c, const char * tag) const;
void log_lock_optimization(Compile* c, const char * tag, Node* bad_lock = NULL) const;

void set_non_esc_obj() { _kind = NonEscObj; set_eliminated_lock_counter(); }
void set_coarsened() { _kind = Coarsened; set_eliminated_lock_counter(); }
Expand Down
108 changes: 107 additions & 1 deletion src/hotspot/share/opto/compile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,7 @@ void Compile::remove_useless_nodes(Unique_Node_List &useful) {
remove_opaque4_node(opaq);
}
}
remove_useless_coarsened_locks(useful); // remove useless coarsened locks nodes
BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2();
bs->eliminate_useless_gc_barriers(useful);
// clean up the late inline lists
Expand Down Expand Up @@ -512,6 +513,12 @@ void Compile::print_compile_messages() {
tty->print_cr("** Bailout: Recompile without boxing elimination **");
tty->print_cr("*********************************************************");
}
if ((_do_locks_coarsening != EliminateLocks) && PrintOpto) {
// Recompiling without locks coarsening
tty->print_cr("*********************************************************");
tty->print_cr("** Bailout: Recompile without locks coarsening **");
tty->print_cr("*********************************************************");
}
if (C->directive()->BreakAtCompileOption) {
// Open the debugger when compiling this method.
tty->print("### Breaking when compiling: ");
Expand Down Expand Up @@ -637,13 +644,15 @@ debug_only( int Compile::_debug_idx = 100000; )


Compile::Compile( ciEnv* ci_env, C2Compiler* compiler, ciMethod* target, int osr_bci,
bool subsume_loads, bool do_escape_analysis, bool eliminate_boxing, DirectiveSet* directive)
bool subsume_loads, bool do_escape_analysis, bool eliminate_boxing,
bool do_locks_coarsening, DirectiveSet* directive)
: Phase(Compiler),
_env(ci_env),
_directive(directive),
_log(ci_env->log()),
_compile_id(ci_env->compile_id()),
_save_argument_registers(false),
_do_locks_coarsening(do_locks_coarsening),
_stub_name(NULL),
_stub_function(NULL),
_stub_entry_point(NULL),
Expand All @@ -668,6 +677,7 @@ Compile::Compile( ciEnv* ci_env, C2Compiler* compiler, ciMethod* target, int osr
_inner_loops(0),
_scratch_const_size(-1),
_in_scratch_emit_size(false),
_coarsened_locks (comp_arena(), 8, 0, NULL),
_dead_node_list(comp_arena()),
_dead_node_count(0),
#ifndef PRODUCT
Expand Down Expand Up @@ -967,6 +977,7 @@ Compile::Compile( ciEnv* ci_env,
_log(ci_env->log()),
_compile_id(0),
_save_argument_registers(save_arg_registers),
_do_locks_coarsening(false),
_method(NULL),
_stub_name(stub_name),
_stub_function(stub_function),
Expand Down Expand Up @@ -4719,6 +4730,101 @@ void Compile::add_expensive_node(Node * n) {
}
}

/**
* Track coarsened Lock and Unlock nodes.
*/

class Lock_List : public Node_List {
uint _origin_cnt;
public:
Lock_List(Arena *a, uint cnt) : Node_List(a), _origin_cnt(cnt) {}
uint origin_cnt() const { return _origin_cnt; }
};

void Compile::add_coarsened_locks(GrowableArray<AbstractLockNode*>& locks) {
int length = locks.length();
if (length > 0) {
// Have to keep this list until locks elimination during Macro nodes elimination.
Lock_List* locks_list = new (comp_arena()) Lock_List(comp_arena(), length);
for (int i = 0; i < length; i++) {
AbstractLockNode* lock = locks.at(i);
assert(lock->is_coarsened(), "expecting only coarsened AbstractLock nodes, but got '%s'[%d] node", lock->Name(), lock->_idx);
locks_list->push(lock);
}
_coarsened_locks.append(locks_list);
}
}

void Compile::remove_useless_coarsened_locks(Unique_Node_List& useful) {
int count = coarsened_count();
for (int i = 0; i < count; i++) {
Node_List* locks_list = _coarsened_locks.at(i);
for (uint j = 0; j < locks_list->size(); j++) {
Node* lock = locks_list->at(j);
assert(lock->is_AbstractLock(), "sanity");
if (!useful.member(lock)) {
locks_list->yank(lock);
}
}
}
}

void Compile::remove_coarsened_lock(Node* n) {
if (n->is_AbstractLock()) {
int count = coarsened_count();
for (int i = 0; i < count; i++) {
Node_List* locks_list = _coarsened_locks.at(i);
locks_list->yank(n);
}
}
}

bool Compile::coarsened_locks_consistent() {
int count = coarsened_count();
for (int i = 0; i < count; i++) {
bool unbalanced = false;
bool modified = false; // track locks kind modifications
Lock_List* locks_list = (Lock_List*)_coarsened_locks.at(i);
uint size = locks_list->size();
if (size != locks_list->origin_cnt()) {
unbalanced = true; // Some locks were removed from list
} else {
for (uint j = 0; j < size; j++) {
Node* lock = locks_list->at(j);
// All nodes in group should have the same state (modified or not)
if (!lock->as_AbstractLock()->is_coarsened()) {
if (j == 0) {
// first on list was modified, the rest should be too for consistency
modified = true;
} else if (!modified) {
// this lock was modified but previous locks on the list were not
unbalanced = true;
break;
}
} else if (modified) {
// previous locks on list were modified but not this lock
unbalanced = true;
break;
}
}
}
if (unbalanced) {
// unbalanced monitor enter/exit - only some [un]lock nodes were removed or modified
#ifdef ASSERT
if (PrintEliminateLocks) {
tty->print_cr("=== unbalanced coarsened locks ===");
for (uint l = 0; l < size; l++) {
locks_list->at(l)->dump();
}
}
#endif
record_failure(C2Compiler::retry_no_locks_coarsening());
return false;
}
}
return true;
}

/**
* Remove the speculative part of types and clean up the graph
*/
Expand Down

1 comment on commit 7a61e03

@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.