8309271: A way to align already compiled methods with compiler directives

Reviewed-by: apangin, sspitsyn, tholenstein
This commit is contained in:
Dmitry Chuyko 2024-03-14 12:38:48 +00:00
parent 954c50ed88
commit c879627dbd
15 changed files with 378 additions and 33 deletions

@ -1141,17 +1141,15 @@ void ciEnv::register_method(ciMethod* target,
#endif
if (entry_bci == InvocationEntryBci) {
if (TieredCompilation) {
// If there is an old version we're done with it
CompiledMethod* 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 there is an old version we're done with it
CompiledMethod* 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;

@ -34,6 +34,7 @@
#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"
@ -1359,6 +1360,67 @@ 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);
CompiledMethodIterator iter(CompiledMethodIterator::only_not_unloading);
while(iter.next()) {
CompiledMethod* 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();
RelaxedCompiledMethodIterator iter(RelaxedCompiledMethodIterator::only_not_unloading);
while(iter.next()) {
CompiledMethod* 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);

@ -304,6 +304,9 @@ 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);

@ -1171,11 +1171,13 @@ void CompileBroker::compile_method_base(const methodHandle& method,
tty->cr();
}
// 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;
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;
}
}
#ifndef PRODUCT
@ -1220,11 +1222,13 @@ void CompileBroker::compile_method_base(const methodHandle& method,
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;
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 now know that this compilation is not pending, complete,
@ -1373,7 +1377,8 @@ nmethod* CompileBroker::compile_method(const methodHandle& method, int osr_bci,
if (osr_bci == InvocationEntryBci) {
// standard compilation
CompiledMethod* method_code = method->code();
if (method_code != nullptr && method_code->is_nmethod()) {
if (method_code != nullptr && method_code->is_nmethod()
&& (compile_reason != CompileTask::Reason_DirectivesChanged)) {
if (compilation_is_complete(method, osr_bci, comp_level)) {
return (nmethod*) method_code;
}

@ -62,6 +62,7 @@ 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
};
@ -74,7 +75,8 @@ class CompileTask : public CHeapObj<mtCompiler> {
"replay",
"whitebox",
"must_be_compiled",
"bootstrap"
"bootstrap",
"directives_changed"
};
return reason_names[compile_reason];
}

@ -747,6 +747,25 @@ 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");

@ -115,6 +115,7 @@ private:
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);

@ -816,6 +816,14 @@ public:
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); }

@ -58,6 +58,7 @@ 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,

@ -269,7 +269,6 @@ 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
@ -327,6 +326,7 @@ 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(CompiledMethod_lock , PaddedMutex , CodeCache_lock);
MUTEX_DEFL(Threads_lock , PaddedMonitor, CompileThread_lock, true);

@ -144,6 +144,7 @@ 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));
@ -923,21 +924,81 @@ void CompilerDirectivesPrintDCmd::execute(DCmdSource source, TRAPS) {
CompilerDirectivesAddDCmd::CompilerDirectivesAddDCmd(outputStream* output, bool heap) :
DCmdWithParser(output, heap),
_filename("filename","Name of the directives file", "STRING",true) {
_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 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) {
DirectivesStack::pop(1);
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);
}
void CompilerDirectivesClearDCmd::execute(DCmdSource source, TRAPS) {
DirectivesStack::clear();
if (_refresh.value()) {
CodeCache::mark_directives_matches();
DirectivesStack::clear();
CodeCache::recompile_marked_directives_matches();
} else {
DirectivesStack::clear();
}
}
#if INCLUDE_SERVICES
ClassHierarchyDCmd::ClassHierarchyDCmd(outputStream* output, bool heap) :
DCmdWithParser(output, heap),

@ -688,9 +688,12 @@ public:
virtual void execute(DCmdSource source, TRAPS);
};
class CompilerDirectivesRemoveDCmd : public DCmd {
class CompilerDirectivesRemoveDCmd : public DCmdWithParser {
protected:
DCmdArgument<bool> _refresh; // true if update should be forced after directives changes.
public:
CompilerDirectivesRemoveDCmd(outputStream* output, bool heap) : DCmd(output, heap) {}
static int num_arguments() { return 1; }
CompilerDirectivesRemoveDCmd(outputStream* output, bool heap);
static const char* name() {
return "Compiler.directives_remove";
}
@ -711,8 +714,9 @@ public:
class CompilerDirectivesAddDCmd : public DCmdWithParser {
protected:
DCmdArgument<char*> _filename;
DCmdArgument<bool> _refresh; // true if update should be forced after directives changes.
public:
static int num_arguments() { return 1; }
static int num_arguments() { return 2; }
CompilerDirectivesAddDCmd(outputStream* output, bool heap);
static const char* name() {
return "Compiler.directives_add";
@ -731,9 +735,36 @@ public:
virtual void execute(DCmdSource source, TRAPS);
};
class CompilerDirectivesClearDCmd : public DCmd {
class CompilerDirectivesReplaceDCmd : public DCmdWithParser {
protected:
DCmdArgument<char*> _filename;
DCmdArgument<bool> _refresh; // true if update should be forced after directives changes.
public:
CompilerDirectivesClearDCmd(outputStream* output, bool heap) : DCmd(output, heap) {}
static int num_arguments() { return 2; }
CompilerDirectivesReplaceDCmd(outputStream* output, bool heap);
static const char* name() {
return "Compiler.directives_replace";
}
static const char* description() {
return "Clear directives stack, and load new compiler directives from file.";
}
static const char* impact() {
return "Low";
}
static const JavaPermission permission() {
JavaPermission p = {"java.lang.management.ManagementPermission",
"monitor", NULL};
return p;
}
virtual void execute(DCmdSource source, TRAPS);
};
class CompilerDirectivesClearDCmd : public DCmdWithParser {
protected:
DCmdArgument<bool> _refresh; // true if update should be forced after directives changes.
public:
static int num_arguments() { return 1; }
CompilerDirectivesClearDCmd(outputStream* output, bool heap);
static const char* name() {
return "Compiler.directives_clear";
}

@ -1,5 +1,5 @@
/*
* Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2015, 2023, 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
@ -96,6 +96,14 @@ public class CompilerDirectivesDCMDTest {
Assert.fail("Expected three directives - found " + count);
}
// Test replacement with some directives from file
output = executor.execute("Compiler.directives_replace " + filename);
output = executor.execute("Compiler.directives_print");
count = find(output, "Directive:");
if (count != 3) {
Assert.fail("Expected three directives - found " + count);
}
// Test remove one directive
output = executor.execute("Compiler.directives_remove");
output = executor.execute("Compiler.directives_print");

@ -0,0 +1,138 @@
/*
* Copyright (c) 2023, BELLSOFT. All rights reserved.
* Copyright (c) 2023, 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
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
/**
* @test DirectivesRefreshTest
* @summary Test of forced recompile after compiler directives changes by diagnostic command
* @requires vm.compiler1.enabled & vm.compiler2.enabled
* @library /test/lib /
* @modules java.base/jdk.internal.misc
*
* @build jdk.test.whitebox.WhiteBox
* @run driver jdk.test.lib.helpers.ClassFileInstaller jdk.test.whitebox.WhiteBox
*
* @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI
* -XX:-BackgroundCompilation
* serviceability.dcmd.compiler.DirectivesRefreshTest
*/
package serviceability.dcmd.compiler;
import jdk.test.whitebox.WhiteBox;
import jdk.test.lib.process.OutputAnalyzer;
import jdk.test.lib.dcmd.CommandExecutor;
import jdk.test.lib.dcmd.JMXExecutor;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.lang.reflect.Method;
import java.util.Random;
import static jdk.test.lib.Asserts.assertEQ;
import static compiler.whitebox.CompilerWhiteBoxTest.COMP_LEVEL_NONE;
import static compiler.whitebox.CompilerWhiteBoxTest.COMP_LEVEL_SIMPLE;
import static compiler.whitebox.CompilerWhiteBoxTest.COMP_LEVEL_FULL_OPTIMIZATION;
public class DirectivesRefreshTest {
static Path cmdPath = Paths.get(System.getProperty("test.src", "."), "refresh_control.txt");
static WhiteBox wb = WhiteBox.getWhiteBox();
static Random random = new Random();
static Method method;
static CommandExecutor executor;
static int callable() {
int result = 0;
for (int i = 0; i < 100; i++) {
result += random.nextInt(100);
}
return result;
}
static void checkCompilationLevel(Method method, int level) {
assertEQ(wb.getMethodCompilationLevel(method), level, "Compilation level");
}
static void setup() throws Exception {
method = DirectivesRefreshTest.class.getDeclaredMethod("callable");
executor = new JMXExecutor();
System.out.println("Compilation with C2");
// Happens with fairly hot methods.
wb.enqueueMethodForCompilation(method, COMP_LEVEL_FULL_OPTIMIZATION);
checkCompilationLevel(method, COMP_LEVEL_FULL_OPTIMIZATION);
}
static void testDirectivesAddRefresh() {
System.out.println("Force forbid C2 via directive, method deoptimized");
var output = executor.execute("Compiler.directives_add -r " + cmdPath.toString());
output.stderrShouldBeEmpty().shouldContain("1 compiler directives added");
// Current handling of 'Exclude' for '-r' clears flags.
checkCompilationLevel(method, COMP_LEVEL_NONE);
System.out.println("C2 is excluded, re-compilation with C1");
// Sanity check for the directive.
wb.enqueueMethodForCompilation(method, COMP_LEVEL_FULL_OPTIMIZATION);
checkCompilationLevel(method, COMP_LEVEL_NONE);
// Happens with fairly hot methods.
wb.enqueueMethodForCompilation(method, COMP_LEVEL_SIMPLE);
checkCompilationLevel(method, COMP_LEVEL_SIMPLE);
}
static void testDirectivesClearRefresh() {
System.out.println("Re-compilation with C2 due to removed restriction");
var output = executor.execute("Compiler.directives_clear -r");
output.stderrShouldBeEmpty().stdoutShouldBeEmpty();
// No need to enqueue the method, "immediate" effect of '-r' without deoptimization.
checkCompilationLevel(method, COMP_LEVEL_FULL_OPTIMIZATION);
}
static void testDirectivesAddRegular() {
System.out.println("No changes if the restriction is not forced");
// According to original JEP 165, the directive will be handled
// "when a method is submitted for a compilation".
var output = executor.execute("Compiler.directives_add " + cmdPath.toString());
output.stderrShouldBeEmpty().shouldContain("1 compiler directives added");
// In this program the method is not called, and here it is not enqueued.
checkCompilationLevel(method, COMP_LEVEL_FULL_OPTIMIZATION);
}
public static void main(String[] args) throws Exception {
setup();
testDirectivesAddRefresh();
testDirectivesClearRefresh();
testDirectivesAddRegular();
}
}

@ -0,0 +1,8 @@
[
{
match: "serviceability.dcmd.compiler.DirectivesRefreshTest::callable",
c2: {
Exclude: true
}
}
]