Skip to content

Commit

Permalink
8332111: [BACKOUT] A way to align already compiled methods with compi…
Browse files Browse the repository at this point in the history
…ler directives

Reviewed-by: shade, kvn
  • Loading branch information
Evgeny Astigeevich committed May 15, 2024
1 parent 957eb61 commit 1a94447
Show file tree
Hide file tree
Showing 15 changed files with 33 additions and 377 deletions.
20 changes: 11 additions & 9 deletions src/hotspot/share/ci/ciEnv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1139,15 +1139,17 @@ void ciEnv::register_method(ciMethod* target,
#endif

if (entry_bci == InvocationEntryBci) {
// If there is an old version we're done with it
nmethod* old = method->code();
if (TraceMethodReplacement && old != nullptr) {
ResourceMark rm;
char *method_name = method->name_and_sig_as_C_string();
tty->print_cr("Replacing method %s", method_name);
}
if (old != nullptr) {
old->make_not_used();
if (TieredCompilation) {
// If there is an old version we're done with it
nmethod* old = method->code();
if (TraceMethodReplacement && old != nullptr) {
ResourceMark rm;
char *method_name = method->name_and_sig_as_C_string();
tty->print_cr("Replacing method %s", method_name);
}
if (old != nullptr) {
old->make_not_used();
}
}

LogTarget(Info, nmethod, install) lt;
Expand Down
62 changes: 0 additions & 62 deletions src/hotspot/share/code/codeCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
#include "compiler/compilationPolicy.hpp"
#include "compiler/compileBroker.hpp"
#include "compiler/compilerDefinitions.inline.hpp"
#include "compiler/compilerDirectives.hpp"
#include "compiler/oopMap.hpp"
#include "gc/shared/barrierSetNMethod.hpp"
#include "gc/shared/classUnloadingContext.hpp"
Expand Down Expand Up @@ -1330,67 +1329,6 @@ void CodeCache::mark_all_nmethods_for_evol_deoptimization(DeoptimizationScope* d

#endif // INCLUDE_JVMTI

void CodeCache::mark_directives_matches(bool top_only) {
MutexLocker mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
Thread *thread = Thread::current();
HandleMark hm(thread);

NMethodIterator iter(NMethodIterator::not_unloading);
while(iter.next()) {
nmethod* nm = iter.method();
methodHandle mh(thread, nm->method());
if (DirectivesStack::hasMatchingDirectives(mh, top_only)) {
ResourceMark rm;
log_trace(codecache)("Mark because of matching directives %s", mh->external_name());
mh->set_has_matching_directives();
}
}
}

void CodeCache::recompile_marked_directives_matches() {
Thread *thread = Thread::current();
HandleMark hm(thread);

// Try the max level and let the directives be applied during the compilation.
int comp_level = CompilationPolicy::highest_compile_level();
RelaxedNMethodIterator iter(RelaxedNMethodIterator::not_unloading);
while(iter.next()) {
nmethod* nm = iter.method();
methodHandle mh(thread, nm->method());
if (mh->has_matching_directives()) {
ResourceMark rm;
mh->clear_directive_flags();
bool deopt = false;

if (!nm->is_osr_method()) {
log_trace(codecache)("Recompile to level %d because of matching directives %s",
comp_level, mh->external_name());
nmethod * comp_nm = CompileBroker::compile_method(mh, InvocationEntryBci, comp_level,
methodHandle(), 0,
CompileTask::Reason_DirectivesChanged,
(JavaThread*)thread);
if (comp_nm == nullptr) {
log_trace(codecache)("Recompilation to level %d failed, deoptimize %s",
comp_level, mh->external_name());
deopt = true;
}
} else {
log_trace(codecache)("Deoptimize OSR %s", mh->external_name());
deopt = true;
}
// For some reason the method cannot be compiled by C2, e.g. the new directives forbid it.
// Deoptimize the method and let the usual hotspot logic do the rest.
if (deopt) {
if (!nm->has_been_deoptimized() && nm->can_be_deoptimized()) {
nm->make_not_entrant();
nm->make_deoptimized();
}
}
gc_on_allocation(); // Flush unused methods from CodeCache if required.
}
}
}

// Mark methods for deopt (if safe or possible).
void CodeCache::mark_all_nmethods_for_deoptimization(DeoptimizationScope* deopt_scope) {
MutexLocker mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
Expand Down
3 changes: 0 additions & 3 deletions src/hotspot/share/code/codeCache.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -295,9 +295,6 @@ class CodeCache : AllStatic {
static void mark_for_deoptimization(DeoptimizationScope* deopt_scope, Method* dependee);
static void make_marked_nmethods_deoptimized();

static void mark_directives_matches(bool top_only = false);
static void recompile_marked_directives_matches();

// Marks dependents during classloading
static void mark_dependents_on(DeoptimizationScope* deopt_scope, InstanceKlass* dependee);

Expand Down
26 changes: 11 additions & 15 deletions src/hotspot/share/compiler/compileBroker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1172,13 +1172,11 @@ void CompileBroker::compile_method_base(const methodHandle& method,
tty->cr();
}

if (compile_reason != CompileTask::Reason_DirectivesChanged) {
// A request has been made for compilation. Before we do any
// real work, check to see if the method has been compiled
// in the meantime with a definitive result.
if (compilation_is_complete(method, osr_bci, comp_level)) {
return;
}
// A request has been made for compilation. Before we do any
// real work, check to see if the method has been compiled
// in the meantime with a definitive result.
if (compilation_is_complete(method, osr_bci, comp_level)) {
return;
}

#ifndef PRODUCT
Expand Down Expand Up @@ -1223,13 +1221,11 @@ void CompileBroker::compile_method_base(const methodHandle& method,
return;
}

if (compile_reason != CompileTask::Reason_DirectivesChanged) {
// We need to check again to see if the compilation has
// completed. A previous compilation may have registered
// some result.
if (compilation_is_complete(method, osr_bci, comp_level)) {
return;
}
// We need to check again to see if the compilation has
// completed. A previous compilation may have registered
// some result.
if (compilation_is_complete(method, osr_bci, comp_level)) {
return;
}

// We now know that this compilation is not pending, complete,
Expand Down Expand Up @@ -1378,7 +1374,7 @@ nmethod* CompileBroker::compile_method(const methodHandle& method, int osr_bci,
if (osr_bci == InvocationEntryBci) {
// standard compilation
nmethod* method_code = method->code();
if (method_code != nullptr && (compile_reason != CompileTask::Reason_DirectivesChanged)) {
if (method_code != nullptr) {
if (compilation_is_complete(method, osr_bci, comp_level)) {
return method_code;
}
Expand Down
4 changes: 1 addition & 3 deletions src/hotspot/share/compiler/compileTask.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ class CompileTask : public CHeapObj<mtCompiler> {
Reason_Whitebox, // Whitebox API
Reason_MustBeCompiled, // Used for -Xcomp or AlwaysCompileLoopMethods (see CompilationPolicy::must_be_compiled())
Reason_Bootstrap, // JVMCI bootstrap
Reason_DirectivesChanged, // Changed CompilerDirectivesStack
Reason_Count
};

Expand All @@ -75,8 +74,7 @@ class CompileTask : public CHeapObj<mtCompiler> {
"replay",
"whitebox",
"must_be_compiled",
"bootstrap",
"directives_changed"
"bootstrap"
};
return reason_names[compile_reason];
}
Expand Down
19 changes: 0 additions & 19 deletions src/hotspot/share/compiler/compilerDirectives.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -747,25 +747,6 @@ void DirectivesStack::release(CompilerDirectives* dir) {
}
}

bool DirectivesStack::hasMatchingDirectives(const methodHandle& method, bool top_only) {
assert(_depth > 0, "Must never be empty");
MutexLocker locker(DirectivesStack_lock, Mutex::_no_safepoint_check_flag);

CompilerDirectives* dir = _top;
assert(dir != nullptr, "Must be initialized");

while (dir != nullptr) {
if (!dir->is_default_directive() && dir->match(method)) {
return true;
}
if (top_only) {
break;
}
dir = dir->next();
}
return false;
}

DirectiveSet* DirectivesStack::getMatchingDirective(const methodHandle& method, AbstractCompiler *comp) {
assert(_depth > 0, "Must never be empty");

Expand Down
1 change: 0 additions & 1 deletion src/hotspot/share/compiler/compilerDirectives.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ class DirectivesStack : AllStatic {
public:
static void init();
static DirectiveSet* getMatchingDirective(const methodHandle& mh, AbstractCompiler* comp);
static bool hasMatchingDirectives(const methodHandle& method, bool top_only = false);
static DirectiveSet* getDefaultDirective(AbstractCompiler* comp);
static void push(CompilerDirectives* directive);
static void pop(int count);
Expand Down
8 changes: 0 additions & 8 deletions src/hotspot/share/oops/method.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -811,14 +811,6 @@ class Method : public Metadata {
return _method_counters;
}

// Clear the flags related to compiler directives that were set by the compilerBroker,
// because the directives can be updated.
void clear_directive_flags() {
set_has_matching_directives(false);
clear_is_not_c1_compilable();
clear_is_not_c2_compilable();
}

void clear_is_not_c1_compilable() { set_is_not_c1_compilable(false); }
void clear_is_not_c2_compilable() { set_is_not_c2_compilable(false); }
void clear_is_not_c2_osr_compilable() { set_is_not_c2_osr_compilable(false); }
Expand Down
1 change: 0 additions & 1 deletion src/hotspot/share/oops/methodFlags.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ class MethodFlags {
status(has_loops_flag , 1 << 13) /* Method has loops */ \
status(has_loops_flag_init , 1 << 14) /* The loop flag has been initialized */ \
status(on_stack_flag , 1 << 15) /* RedefineClasses support to keep Metadata from being cleaned */ \
status(has_matching_directives , 1 << 16) /* Temporary mark, used only when methods are to be refreshed to reflect a compiler directives update */ \
/* end of list */

#define M_STATUS_ENUM_NAME(name, value) _misc_##name = value,
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/runtime/mutexLocker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ void mutex_init() {
MUTEX_DEFN(CompiledIC_lock , PaddedMutex , nosafepoint); // locks VtableStubs_lock, InlineCacheBuffer_lock
MUTEX_DEFN(MethodCompileQueue_lock , PaddedMonitor, safepoint);
MUTEX_DEFN(CompileStatistics_lock , PaddedMutex , safepoint);
MUTEX_DEFN(DirectivesStack_lock , PaddedMutex , nosafepoint);

MUTEX_DEFN(JvmtiThreadState_lock , PaddedMutex , safepoint); // Used by JvmtiThreadState/JvmtiEventController
MUTEX_DEFN(EscapeBarrier_lock , PaddedMonitor, nosafepoint); // Used to synchronize object reallocation/relocking triggered by JVMTI
Expand Down Expand Up @@ -325,7 +326,6 @@ void mutex_init() {
MUTEX_DEFL(InlineCacheBuffer_lock , PaddedMutex , CompiledIC_lock);
MUTEX_DEFL(VtableStubs_lock , PaddedMutex , CompiledIC_lock); // Also holds DumpTimeTable_lock
MUTEX_DEFL(CodeCache_lock , PaddedMonitor, VtableStubs_lock);
MUTEX_DEFL(DirectivesStack_lock , PaddedMutex , CodeCache_lock);
MUTEX_DEFL(NMethodState_lock , PaddedMutex , CodeCache_lock);

MUTEX_DEFL(Threads_lock , PaddedMonitor, CompileThread_lock, true);
Expand Down
67 changes: 3 additions & 64 deletions src/hotspot/share/services/diagnosticCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ void DCmd::register_dcmds(){

DCmdFactory::register_DCmdFactory(new DCmdFactoryImpl<CompilerDirectivesPrintDCmd>(full_export, true, false));
DCmdFactory::register_DCmdFactory(new DCmdFactoryImpl<CompilerDirectivesAddDCmd>(full_export, true, false));
DCmdFactory::register_DCmdFactory(new DCmdFactoryImpl<CompilerDirectivesReplaceDCmd>(full_export, true, false));
DCmdFactory::register_DCmdFactory(new DCmdFactoryImpl<CompilerDirectivesRemoveDCmd>(full_export, true, false));
DCmdFactory::register_DCmdFactory(new DCmdFactoryImpl<CompilerDirectivesClearDCmd>(full_export, true, false));
DCmdFactory::register_DCmdFactory(new DCmdFactoryImpl<CompilationMemoryStatisticDCmd>(full_export, true, false));
Expand Down Expand Up @@ -924,81 +923,21 @@ void CompilerDirectivesPrintDCmd::execute(DCmdSource source, TRAPS) {

CompilerDirectivesAddDCmd::CompilerDirectivesAddDCmd(outputStream* output, bool heap) :
DCmdWithParser(output, heap),
_filename("filename", "Name of the directives file", "STRING", true),
_refresh("-r", "Refresh affected methods", "BOOLEAN", false, "false") {

_filename("filename","Name of the directives file", "STRING",true) {
_dcmdparser.add_dcmd_argument(&_filename);
_dcmdparser.add_dcmd_option(&_refresh);
}

void CompilerDirectivesAddDCmd::execute(DCmdSource source, TRAPS) {
DirectivesParser::parse_from_file(_filename.value(), output(), true);
if (_refresh.value()) {
CodeCache::mark_directives_matches(true);
CodeCache::recompile_marked_directives_matches();
}
}

CompilerDirectivesReplaceDCmd::CompilerDirectivesReplaceDCmd(outputStream* output, bool heap) :
DCmdWithParser(output, heap),
_filename("filename", "Name of the directives file", "STRING", true),
_refresh("-r", "Refresh affected methods", "BOOLEAN", false, "false") {

_dcmdparser.add_dcmd_argument(&_filename);
_dcmdparser.add_dcmd_option(&_refresh);
}

void CompilerDirectivesReplaceDCmd::execute(DCmdSource source, TRAPS) {
// Need to mark the methods twice, to account for the method that doesn't match
// the directives anymore
if (_refresh.value()) {
CodeCache::mark_directives_matches();

DirectivesStack::clear();
DirectivesParser::parse_from_file(_filename.value(), output(), true);

CodeCache::mark_directives_matches();
CodeCache::recompile_marked_directives_matches();
} else {
DirectivesStack::clear();
DirectivesParser::parse_from_file(_filename.value(), output(), true);
}
}

CompilerDirectivesRemoveDCmd::CompilerDirectivesRemoveDCmd(outputStream* output, bool heap) :
DCmdWithParser(output, heap),
_refresh("-r", "Refresh affected methods", "BOOLEAN", false, "false") {

_dcmdparser.add_dcmd_option(&_refresh);
}

void CompilerDirectivesRemoveDCmd::execute(DCmdSource source, TRAPS) {
if (_refresh.value()) {
CodeCache::mark_directives_matches(true);
DirectivesStack::pop(1);
CodeCache::recompile_marked_directives_matches();
} else {
DirectivesStack::pop(1);
}
}

CompilerDirectivesClearDCmd::CompilerDirectivesClearDCmd(outputStream* output, bool heap) :
DCmdWithParser(output, heap),
_refresh("-r", "Refresh affected methods", "BOOLEAN", false, "false") {

_dcmdparser.add_dcmd_option(&_refresh);
DirectivesStack::pop(1);
}

void CompilerDirectivesClearDCmd::execute(DCmdSource source, TRAPS) {
if (_refresh.value()) {
CodeCache::mark_directives_matches();
DirectivesStack::clear();
CodeCache::recompile_marked_directives_matches();
} else {
DirectivesStack::clear();
}
DirectivesStack::clear();
}

#if INCLUDE_SERVICES
ClassHierarchyDCmd::ClassHierarchyDCmd(outputStream* output, bool heap) :
DCmdWithParser(output, heap),
Expand Down
Loading

1 comment on commit 1a94447

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