Skip to content

8356783: CompilerTask hot_method is redundant #25185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

8356783: CompilerTask hot_method is redundant #25185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/hotspot/share/ci/ciReplay.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,7 @@ class CompileReplay : public StackObj {
}
replay_state = this;
CompileBroker::compile_method(methodHandle(THREAD, method), entry_bci, comp_level,
methodHandle(), 0, CompileTask::Reason_Replay, THREAD);
0, CompileTask::Reason_Replay, THREAD);
replay_state = nullptr;
}

Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/compiler/compilationPolicy.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ void CompilationPolicy::compile_if_required(const methodHandle& m, TRAPS) {
if (PrintTieredEvents) {
print_event(COMPILE, m(), m(), InvocationEntryBci, level);
}
CompileBroker::compile_method(m, InvocationEntryBci, level, methodHandle(), 0, CompileTask::Reason_MustBeCompiled, THREAD);
CompileBroker::compile_method(m, InvocationEntryBci, level, 0, CompileTask::Reason_MustBeCompiled, THREAD);
}
}

Expand Down Expand Up @@ -812,7 +812,7 @@ void CompilationPolicy::compile(const methodHandle& mh, int bci, CompLevel level
}
int hot_count = (bci == InvocationEntryBci) ? mh->invocation_count() : mh->backedge_count();
update_rate(nanos_to_millis(os::javaTimeNanos()), mh);
CompileBroker::compile_method(mh, bci, level, mh, hot_count, CompileTask::Reason_Tiered, THREAD);
CompileBroker::compile_method(mh, bci, level, hot_count, CompileTask::Reason_Tiered, THREAD);
}
}

Expand Down
25 changes: 8 additions & 17 deletions src/hotspot/share/compiler/compileBroker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,6 @@ CompileTask* CompileQueue::get(CompilerThread* thread) {
// save methods from RedefineClasses across safepoint
// across MethodCompileQueue_lock below.
methodHandle save_method;
methodHandle save_hot_method;

MonitorLocker locker(MethodCompileQueue_lock);
// If _first is null we have no more compile jobs. There are two reasons for
Expand Down Expand Up @@ -453,7 +452,6 @@ CompileTask* CompileQueue::get(CompilerThread* thread) {
// the compilation queue, which is walked during RedefineClasses.
Thread* thread = Thread::current();
save_method = methodHandle(thread, task->method());
save_hot_method = methodHandle(thread, task->hot_method());

remove(task);
}
Expand Down Expand Up @@ -1154,7 +1152,6 @@ void CompileBroker::mark_on_stack() {
void CompileBroker::compile_method_base(const methodHandle& method,
int osr_bci,
int comp_level,
const methodHandle& hot_method,
int hot_count,
CompileTask::CompileReason compile_reason,
bool blocking,
Expand All @@ -1173,13 +1170,8 @@ void CompileBroker::compile_method_base(const methodHandle& method,
tty->print(" osr_bci: %d", osr_bci);
}
tty->print(" level: %d comment: %s count: %d", comp_level, CompileTask::reason_name(compile_reason), hot_count);
if (!hot_method.is_null()) {
tty->print(" hot: ");
if (hot_method() != method()) {
hot_method->print_short_name(tty);
} else {
tty->print("yes");
}
if (hot_count > 0) {
tty->print(" hot: yes");
}
tty->cr();
}
Expand Down Expand Up @@ -1326,7 +1318,7 @@ void CompileBroker::compile_method_base(const methodHandle& method,
task = create_compile_task(queue,
compile_id, method,
osr_bci, comp_level,
hot_method, hot_count, compile_reason,
hot_count, compile_reason,
blocking);
}

Expand All @@ -1337,7 +1329,7 @@ void CompileBroker::compile_method_base(const methodHandle& method,

nmethod* CompileBroker::compile_method(const methodHandle& method, int osr_bci,
int comp_level,
const methodHandle& hot_method, int hot_count,
int hot_count,
CompileTask::CompileReason compile_reason,
TRAPS) {
// Do nothing if compilebroker is not initialized or compiles are submitted on level none
Expand All @@ -1357,14 +1349,14 @@ nmethod* CompileBroker::compile_method(const methodHandle& method, int osr_bci,

DirectiveSet* directive = DirectivesStack::getMatchingDirective(method, comp);
// CompileBroker::compile_method can trap and can have pending async exception.
nmethod* nm = CompileBroker::compile_method(method, osr_bci, comp_level, hot_method, hot_count, compile_reason, directive, THREAD);
nmethod* nm = CompileBroker::compile_method(method, osr_bci, comp_level, hot_count, compile_reason, directive, THREAD);
DirectivesStack::release(directive);
return nm;
}

nmethod* CompileBroker::compile_method(const methodHandle& method, int osr_bci,
int comp_level,
const methodHandle& hot_method, int hot_count,
int hot_count,
CompileTask::CompileReason compile_reason,
DirectiveSet* directive,
TRAPS) {
Expand Down Expand Up @@ -1460,7 +1452,7 @@ nmethod* CompileBroker::compile_method(const methodHandle& method, int osr_bci,
return nullptr;
}
bool is_blocking = !directive->BackgroundCompilationOption || ReplayCompiles;
compile_method_base(method, osr_bci, comp_level, hot_method, hot_count, compile_reason, is_blocking, THREAD);
compile_method_base(method, osr_bci, comp_level, hot_count, compile_reason, is_blocking, THREAD);
}

// return requested nmethod
Expand Down Expand Up @@ -1607,13 +1599,12 @@ CompileTask* CompileBroker::create_compile_task(CompileQueue* queue,
const methodHandle& method,
int osr_bci,
int comp_level,
const methodHandle& hot_method,
int hot_count,
CompileTask::CompileReason compile_reason,
bool blocking) {
CompileTask* new_task = CompileTask::allocate();
new_task->initialize(compile_id, method, osr_bci, comp_level,
hot_method, hot_count, compile_reason,
hot_count, compile_reason,
blocking);
queue->add(new_task);
return new_task;
Expand Down
4 changes: 0 additions & 4 deletions src/hotspot/share/compiler/compileBroker.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,6 @@ class CompileBroker: AllStatic {
const methodHandle& method,
int osr_bci,
int comp_level,
const methodHandle& hot_method,
int hot_count,
CompileTask::CompileReason compile_reason,
bool blocking);
Expand All @@ -288,7 +287,6 @@ class CompileBroker: AllStatic {
static void compile_method_base(const methodHandle& method,
int osr_bci,
int comp_level,
const methodHandle& hot_method,
int hot_count,
CompileTask::CompileReason compile_reason,
bool blocking,
Expand Down Expand Up @@ -322,7 +320,6 @@ class CompileBroker: AllStatic {
static nmethod* compile_method(const methodHandle& method,
int osr_bci,
int comp_level,
const methodHandle& hot_method,
int hot_count,
CompileTask::CompileReason compile_reason,
TRAPS);
Expand All @@ -333,7 +330,6 @@ class CompileBroker: AllStatic {
static nmethod* compile_method(const methodHandle& method,
int osr_bci,
int comp_level,
const methodHandle& hot_method,
int hot_count,
CompileTask::CompileReason compile_reason,
DirectiveSet* directive,
Expand Down
33 changes: 1 addition & 32 deletions src/hotspot/share/compiler/compileTask.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,10 @@ void CompileTask::free(CompileTask* task) {
MutexLocker locker(CompileTaskAlloc_lock);
if (!task->is_free()) {
assert(!task->lock()->is_locked(), "Should not be locked when freed");
if ((task->_method_holder != nullptr && JNIHandles::is_weak_global_handle(task->_method_holder)) ||
(task->_hot_method_holder != nullptr && JNIHandles::is_weak_global_handle(task->_hot_method_holder))) {
if ((task->_method_holder != nullptr && JNIHandles::is_weak_global_handle(task->_method_holder))) {
JNIHandles::destroy_weak_global(task->_method_holder);
JNIHandles::destroy_weak_global(task->_hot_method_holder);
} else {
JNIHandles::destroy_global(task->_method_holder);
JNIHandles::destroy_global(task->_hot_method_holder);
}
if (task->_failure_reason_on_C_heap && task->_failure_reason != nullptr) {
os::free((void*) task->_failure_reason);
Expand All @@ -90,7 +87,6 @@ void CompileTask::initialize(int compile_id,
const methodHandle& method,
int osr_bci,
int comp_level,
const methodHandle& hot_method,
int hot_count,
CompileTask::CompileReason compile_reason,
bool is_blocking) {
Expand All @@ -112,8 +108,6 @@ void CompileTask::initialize(int compile_id,
_is_complete = false;
_is_success = false;

_hot_method = nullptr;
_hot_method_holder = nullptr;
_hot_count = hot_count;
_time_queued = os::elapsed_counter();
_time_started = 0;
Expand All @@ -127,18 +121,6 @@ void CompileTask::initialize(int compile_id,
_failure_reason_on_C_heap = false;
_arena_bytes = 0;

if (LogCompilation) {
if (hot_method.not_null()) {
if (hot_method == method) {
_hot_method = _method;
} else {
_hot_method = hot_method();
// only add loader or mirror if different from _method_holder
_hot_method_holder = JNIHandles::make_weak_global(Handle(thread, hot_method->method_holder()->klass_holder()));
}
}
}

_next = nullptr;
}

Expand All @@ -159,11 +141,7 @@ CompileTask* CompileTask::select_for_compilation() {
assert(_method->method_holder()->is_loader_alive(), "should be alive");
Handle method_holder(thread, _method->method_holder()->klass_holder());
JNIHandles::destroy_weak_global(_method_holder);
JNIHandles::destroy_weak_global(_hot_method_holder);
_method_holder = JNIHandles::make_global(method_holder);
if (_hot_method != nullptr) {
_hot_method_holder = JNIHandles::make_global(Handle(thread, _hot_method->method_holder()->klass_holder()));
}
return this;
}

Expand All @@ -173,9 +151,6 @@ void CompileTask::mark_on_stack() {
}
// Mark these methods as something redefine classes cannot remove.
_method->set_on_stack(true);
if (_hot_method != nullptr) {
_hot_method->set_on_stack(true);
}
}

bool CompileTask::is_unloaded() const {
Expand All @@ -188,9 +163,6 @@ void CompileTask::metadata_do(MetadataClosure* f) {
return;
}
f->do_metadata(method());
if (hot_method() != nullptr && hot_method() != method()) {
f->do_metadata(hot_method());
}
}

// ------------------------------------------------------------------
Expand Down Expand Up @@ -329,9 +301,6 @@ void CompileTask::log_task_queued() {
assert(_compile_reason > CompileTask::Reason_None && _compile_reason < CompileTask::Reason_Count, "Valid values");
xtty->print(" comment='%s'", reason_name(_compile_reason));

if (_hot_method != nullptr && _hot_method != _method) {
xtty->method(_hot_method);
}
if (_hot_count != 0) {
xtty->print(" hot_count='%d'", _hot_count);
}
Expand Down
5 changes: 1 addition & 4 deletions src/hotspot/share/compiler/compileTask.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,6 @@ class CompileTask : public CHeapObj<mtCompiler> {
// Fields used for logging why the compilation was initiated:
jlong _time_queued; // time when task was enqueued
jlong _time_started; // time when compilation started
Method* _hot_method; // which method actually triggered this task
jobject _hot_method_holder;
int _hot_count; // information about its invocation counter
CompileReason _compile_reason; // more info about the task
const char* _failure_reason;
Expand All @@ -122,15 +120,14 @@ class CompileTask : public CHeapObj<mtCompiler> {
}

void initialize(int compile_id, const methodHandle& method, int osr_bci, int comp_level,
const methodHandle& hot_method, int hot_count,
int hot_count,
CompileTask::CompileReason compile_reason, bool is_blocking);

static CompileTask* allocate();
static void free(CompileTask* task);

int compile_id() const { return _compile_id; }
Method* method() const { return _method; }
Method* hot_method() const { return _hot_method; }
int osr_bci() const { return _osr_bci; }
bool is_complete() const { return _is_complete; }
bool is_blocking() const { return _is_blocking; }
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/jvmci/jvmciCompiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ void JVMCICompiler::bootstrap(TRAPS) {
if (!mh->is_native() && !mh->is_static() && !mh->is_object_initializer() && !mh->is_static_initializer()) {
ResourceMark rm;
int hot_count = 10; // TODO: what's the appropriate value?
CompileBroker::compile_method(mh, InvocationEntryBci, CompLevel_full_optimization, mh, hot_count, CompileTask::Reason_Bootstrap, CHECK);
CompileBroker::compile_method(mh, InvocationEntryBci, CompLevel_full_optimization, hot_count, CompileTask::Reason_Bootstrap, CHECK);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/prims/whitebox.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1124,7 +1124,7 @@ bool WhiteBox::compile_method(Method* method, int comp_level, int bci, JavaThrea
DirectivesStack::release(directive);

// Compile method and check result
nmethod* nm = CompileBroker::compile_method(mh, bci, comp_level, mh, mh->invocation_count(), CompileTask::Reason_Whitebox, CHECK_false);
nmethod* nm = CompileBroker::compile_method(mh, bci, comp_level, mh->invocation_count(), CompileTask::Reason_Whitebox, CHECK_false);
MutexLocker mu(THREAD, Compile_lock);
bool is_queued = mh->queued_for_compilation();
if ((!is_blocking && is_queued) || nm != nullptr) {
Expand Down