From 62fde687088ce72ef33b94e73babf4bfe1395c17 Mon Sep 17 00:00:00 2001 From: Cesar Soares Lucas Date: Thu, 5 Jun 2025 16:43:29 +0000 Subject: [PATCH] 8357396: Refactor nmethod::make_not_entrant to use Enum instead of "const char*" Reviewed-by: mhaessig, shade --- src/hotspot/share/c1/c1_Runtime1.cpp | 8 +- src/hotspot/share/ci/ciReplay.cpp | 2 +- src/hotspot/share/code/codeCache.cpp | 2 +- src/hotspot/share/code/nmethod.cpp | 14 ++- src/hotspot/share/code/nmethod.hpp | 85 ++++++++++++++++++- .../share/compiler/compilationPolicy.cpp | 4 +- src/hotspot/share/jvmci/jvmciCompilerToVM.cpp | 9 +- src/hotspot/share/jvmci/jvmciEnv.cpp | 6 +- src/hotspot/share/jvmci/jvmciEnv.hpp | 2 +- src/hotspot/share/jvmci/jvmciRuntime.cpp | 2 +- src/hotspot/share/oops/instanceKlass.cpp | 2 +- src/hotspot/share/oops/method.cpp | 2 +- src/hotspot/share/prims/whitebox.cpp | 2 +- src/hotspot/share/runtime/deoptimization.cpp | 4 +- src/hotspot/share/runtime/javaThread.cpp | 2 +- 15 files changed, 111 insertions(+), 35 deletions(-) diff --git a/src/hotspot/share/c1/c1_Runtime1.cpp b/src/hotspot/share/c1/c1_Runtime1.cpp index 7c41c09d378..e2760689daa 100644 --- a/src/hotspot/share/c1/c1_Runtime1.cpp +++ b/src/hotspot/share/c1/c1_Runtime1.cpp @@ -818,7 +818,7 @@ JRT_ENTRY(void, Runtime1::deoptimize(JavaThread* current, jint trap_request)) Deoptimization::DeoptReason reason = Deoptimization::trap_request_reason(trap_request); if (action == Deoptimization::Action_make_not_entrant) { - if (nm->make_not_entrant("C1 deoptimize")) { + if (nm->make_not_entrant(nmethod::ChangeReason::C1_deoptimize)) { if (reason == Deoptimization::Reason_tenured) { MethodData* trap_mdo = Deoptimization::get_method_data(current, method, true /*create_if_missing*/); if (trap_mdo != nullptr) { @@ -1110,7 +1110,7 @@ JRT_ENTRY(void, Runtime1::patch_code(JavaThread* current, C1StubId stub_id )) // safepoint, but if it's still alive then make it not_entrant. nmethod* nm = CodeCache::find_nmethod(caller_frame.pc()); if (nm != nullptr) { - nm->make_not_entrant("C1 code patch"); + nm->make_not_entrant(nmethod::ChangeReason::C1_codepatch); } Deoptimization::deoptimize_frame(current, caller_frame.id()); @@ -1358,7 +1358,7 @@ void Runtime1::patch_code(JavaThread* current, C1StubId stub_id) { // Make sure the nmethod is invalidated, i.e. made not entrant. nmethod* nm = CodeCache::find_nmethod(caller_frame.pc()); if (nm != nullptr) { - nm->make_not_entrant("C1 deoptimize for patching"); + nm->make_not_entrant(nmethod::ChangeReason::C1_deoptimize_for_patching); } } @@ -1486,7 +1486,7 @@ JRT_ENTRY(void, Runtime1::predicate_failed_trap(JavaThread* current)) nmethod* nm = CodeCache::find_nmethod(caller_frame.pc()); assert (nm != nullptr, "no more nmethod?"); - nm->make_not_entrant("C1 predicate failed trap"); + nm->make_not_entrant(nmethod::ChangeReason::C1_predicate_failed_trap); methodHandle m(current, nm->method()); MethodData* mdo = m->method_data(); diff --git a/src/hotspot/share/ci/ciReplay.cpp b/src/hotspot/share/ci/ciReplay.cpp index 5ea4336f35e..f9829e88c4a 100644 --- a/src/hotspot/share/ci/ciReplay.cpp +++ b/src/hotspot/share/ci/ciReplay.cpp @@ -802,7 +802,7 @@ class CompileReplay : public StackObj { // Make sure the existence of a prior compile doesn't stop this one nmethod* nm = (entry_bci != InvocationEntryBci) ? method->lookup_osr_nmethod_for(entry_bci, comp_level, true) : method->code(); if (nm != nullptr) { - nm->make_not_entrant("CI replay"); + nm->make_not_entrant(nmethod::ChangeReason::CI_replay); } replay_state = this; CompileBroker::compile_method(methodHandle(THREAD, method), entry_bci, comp_level, diff --git a/src/hotspot/share/code/codeCache.cpp b/src/hotspot/share/code/codeCache.cpp index 902d4345622..3a5da1d7cd2 100644 --- a/src/hotspot/share/code/codeCache.cpp +++ b/src/hotspot/share/code/codeCache.cpp @@ -1361,7 +1361,7 @@ void CodeCache::make_marked_nmethods_deoptimized() { while(iter.next()) { nmethod* nm = iter.method(); if (nm->is_marked_for_deoptimization() && !nm->has_been_deoptimized() && nm->can_be_deoptimized()) { - nm->make_not_entrant("marked for deoptimization"); + nm->make_not_entrant(nmethod::ChangeReason::marked_for_deoptimization); nm->make_deoptimized(); } } diff --git a/src/hotspot/share/code/nmethod.cpp b/src/hotspot/share/code/nmethod.cpp index 510160a6667..acebaae6ba4 100644 --- a/src/hotspot/share/code/nmethod.cpp +++ b/src/hotspot/share/code/nmethod.cpp @@ -1975,14 +1975,12 @@ void nmethod::invalidate_osr_method() { } } -void nmethod::log_state_change(const char* reason) const { - assert(reason != nullptr, "Must provide a reason"); - +void nmethod::log_state_change(ChangeReason change_reason) const { if (LogCompilation) { if (xtty != nullptr) { ttyLocker ttyl; // keep the following output all in one block xtty->begin_elem("make_not_entrant thread='%zu' reason='%s'", - os::current_thread_id(), reason); + os::current_thread_id(), change_reason_to_string(change_reason)); log_identity(xtty); xtty->stamp(); xtty->end_elem(); @@ -1991,7 +1989,7 @@ void nmethod::log_state_change(const char* reason) const { ResourceMark rm; stringStream ss(NEW_RESOURCE_ARRAY(char, 256), 256); - ss.print("made not entrant: %s", reason); + ss.print("made not entrant: %s", change_reason_to_string(change_reason)); CompileTask::print_ul(this, ss.freeze()); if (PrintCompilation) { @@ -2006,9 +2004,7 @@ void nmethod::unlink_from_method() { } // Invalidate code -bool nmethod::make_not_entrant(const char* reason) { - assert(reason != nullptr, "Must provide a reason"); - +bool nmethod::make_not_entrant(ChangeReason change_reason) { // This can be called while the system is already at a safepoint which is ok NoSafepointVerifier nsv; @@ -2077,7 +2073,7 @@ bool nmethod::make_not_entrant(const char* reason) { assert(success, "Transition can't fail"); // Log the transition once - log_state_change(reason); + log_state_change(change_reason); // Remove nmethod from method. unlink_from_method(); diff --git a/src/hotspot/share/code/nmethod.hpp b/src/hotspot/share/code/nmethod.hpp index 2ce6e5cd361..7453bdfa0ef 100644 --- a/src/hotspot/share/code/nmethod.hpp +++ b/src/hotspot/share/code/nmethod.hpp @@ -471,6 +471,85 @@ class nmethod : public CodeBlob { void oops_do_set_strong_done(nmethod* old_head); public: + enum class ChangeReason : u1 { + C1_codepatch, + C1_deoptimize, + C1_deoptimize_for_patching, + C1_predicate_failed_trap, + CI_replay, + JVMCI_invalidate_nmethod, + JVMCI_invalidate_nmethod_mirror, + JVMCI_materialize_virtual_object, + JVMCI_new_installation, + JVMCI_register_method, + JVMCI_replacing_with_new_code, + JVMCI_reprofile, + marked_for_deoptimization, + missing_exception_handler, + not_used, + OSR_invalidation_back_branch, + OSR_invalidation_for_compiling_with_C1, + OSR_invalidation_of_lower_level, + set_native_function, + uncommon_trap, + whitebox_deoptimization, + zombie, + }; + + + static const char* change_reason_to_string(ChangeReason change_reason) { + switch (change_reason) { + case ChangeReason::C1_codepatch: + return "C1 code patch"; + case ChangeReason::C1_deoptimize: + return "C1 deoptimized"; + case ChangeReason::C1_deoptimize_for_patching: + return "C1 deoptimize for patching"; + case ChangeReason::C1_predicate_failed_trap: + return "C1 predicate failed trap"; + case ChangeReason::CI_replay: + return "CI replay"; + case ChangeReason::JVMCI_invalidate_nmethod: + return "JVMCI invalidate nmethod"; + case ChangeReason::JVMCI_invalidate_nmethod_mirror: + return "JVMCI invalidate nmethod mirror"; + case ChangeReason::JVMCI_materialize_virtual_object: + return "JVMCI materialize virtual object"; + case ChangeReason::JVMCI_new_installation: + return "JVMCI new installation"; + case ChangeReason::JVMCI_register_method: + return "JVMCI register method"; + case ChangeReason::JVMCI_replacing_with_new_code: + return "JVMCI replacing with new code"; + case ChangeReason::JVMCI_reprofile: + return "JVMCI reprofile"; + case ChangeReason::marked_for_deoptimization: + return "marked for deoptimization"; + case ChangeReason::missing_exception_handler: + return "missing exception handler"; + case ChangeReason::not_used: + return "not used"; + case ChangeReason::OSR_invalidation_back_branch: + return "OSR invalidation back branch"; + case ChangeReason::OSR_invalidation_for_compiling_with_C1: + return "OSR invalidation for compiling with C1"; + case ChangeReason::OSR_invalidation_of_lower_level: + return "OSR invalidation of lower level"; + case ChangeReason::set_native_function: + return "set native function"; + case ChangeReason::uncommon_trap: + return "uncommon trap"; + case ChangeReason::whitebox_deoptimization: + return "whitebox deoptimization"; + case ChangeReason::zombie: + return "zombie"; + default: { + assert(false, "Unhandled reason"); + return "Unknown"; + } + } + } + // create nmethod with entry_bci static nmethod* new_nmethod(const methodHandle& method, int compile_id, @@ -633,8 +712,8 @@ public: // alive. It is used when an uncommon trap happens. Returns true // if this thread changed the state of the nmethod or false if // another thread performed the transition. - bool make_not_entrant(const char* reason); - bool make_not_used() { return make_not_entrant("not used"); } + bool make_not_entrant(ChangeReason change_reason); + bool make_not_used() { return make_not_entrant(ChangeReason::not_used); } bool is_marked_for_deoptimization() const { return deoptimization_status() != not_marked; } bool has_been_deoptimized() const { return deoptimization_status() == deoptimize_done; } @@ -947,7 +1026,7 @@ public: // Logging void log_identity(xmlStream* log) const; void log_new_nmethod() const; - void log_state_change(const char* reason) const; + void log_state_change(ChangeReason change_reason) const; // Prints block-level comments, including nmethod specific block labels: void print_nmethod_labels(outputStream* stream, address block_begin, bool print_section_labels=true) const; diff --git a/src/hotspot/share/compiler/compilationPolicy.cpp b/src/hotspot/share/compiler/compilationPolicy.cpp index 39c90281c79..bab437eaade 100644 --- a/src/hotspot/share/compiler/compilationPolicy.cpp +++ b/src/hotspot/share/compiler/compilationPolicy.cpp @@ -924,7 +924,7 @@ void CompilationPolicy::compile(const methodHandle& mh, int bci, CompLevel level nmethod* osr_nm = mh->lookup_osr_nmethod_for(bci, CompLevel_simple, false); if (osr_nm != nullptr && osr_nm->comp_level() > CompLevel_simple) { // Invalidate the existing OSR nmethod so that a compile at CompLevel_simple is permitted. - osr_nm->make_not_entrant("OSR invalidation for compiling with C1"); + osr_nm->make_not_entrant(nmethod::ChangeReason::OSR_invalidation_for_compiling_with_C1); } compile(mh, bci, CompLevel_simple, THREAD); } @@ -1516,7 +1516,7 @@ void CompilationPolicy::method_back_branch_event(const methodHandle& mh, const m int osr_bci = nm->is_osr_method() ? nm->osr_entry_bci() : InvocationEntryBci; print_event(MAKE_NOT_ENTRANT, mh(), mh(), osr_bci, level); } - nm->make_not_entrant("OSR invalidation, back branch"); + nm->make_not_entrant(nmethod::ChangeReason::OSR_invalidation_back_branch); } } // Fix up next_level if necessary to avoid deopts diff --git a/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp b/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp index d45036ced94..001a40f74bc 100644 --- a/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp +++ b/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp @@ -27,6 +27,7 @@ #include "classfile/symbolTable.hpp" #include "classfile/systemDictionary.hpp" #include "classfile/vmClasses.hpp" +#include "code/nmethod.hpp" #include "code/scopeDesc.hpp" #include "compiler/compileBroker.hpp" #include "compiler/compilerEvent.hpp" @@ -1206,7 +1207,7 @@ C2V_VMENTRY_0(jint, installCode0, (JNIEnv *env, jobject, assert(JVMCIENV->isa_HotSpotNmethod(installed_code_handle), "wrong type"); // Clear the link to an old nmethod first JVMCIObject nmethod_mirror = installed_code_handle; - JVMCIENV->invalidate_nmethod_mirror(nmethod_mirror, true, JVMCI_CHECK_0); + JVMCIENV->invalidate_nmethod_mirror(nmethod_mirror, true, nmethod::ChangeReason::JVMCI_replacing_with_new_code, JVMCI_CHECK_0); } else { assert(JVMCIENV->isa_InstalledCode(installed_code_handle), "wrong type"); } @@ -1382,7 +1383,7 @@ C2V_VMENTRY(void, reprofile, (JNIEnv* env, jobject, ARGUMENT_PAIR(method))) nmethod* code = method->code(); if (code != nullptr) { - code->make_not_entrant("JVMCI reprofile"); + code->make_not_entrant(nmethod::ChangeReason::JVMCI_reprofile); } MethodData* method_data = method->method_data(); @@ -1397,7 +1398,7 @@ C2V_END C2V_VMENTRY(void, invalidateHotSpotNmethod, (JNIEnv* env, jobject, jobject hs_nmethod, jboolean deoptimize)) JVMCIObject nmethod_mirror = JVMCIENV->wrap(hs_nmethod); - JVMCIENV->invalidate_nmethod_mirror(nmethod_mirror, deoptimize, JVMCI_CHECK); + JVMCIENV->invalidate_nmethod_mirror(nmethod_mirror, deoptimize, nmethod::ChangeReason::JVMCI_invalidate_nmethod, JVMCI_CHECK); C2V_END C2V_VMENTRY_NULL(jlongArray, collectCounters, (JNIEnv* env, jobject)) @@ -1822,7 +1823,7 @@ C2V_VMENTRY(void, materializeVirtualObjects, (JNIEnv* env, jobject, jobject _hs_ if (!fst.current()->is_compiled_frame()) { JVMCI_THROW_MSG(IllegalStateException, "compiled stack frame expected"); } - fst.current()->cb()->as_nmethod()->make_not_entrant("JVMCI materialize virtual objects"); + fst.current()->cb()->as_nmethod()->make_not_entrant(nmethod::ChangeReason::JVMCI_materialize_virtual_object); } Deoptimization::deoptimize(thread, *fst.current(), Deoptimization::Reason_none); // look for the frame again as it has been updated by deopt (pc, deopt state...) diff --git a/src/hotspot/share/jvmci/jvmciEnv.cpp b/src/hotspot/share/jvmci/jvmciEnv.cpp index b8474552256..8c9facf8489 100644 --- a/src/hotspot/share/jvmci/jvmciEnv.cpp +++ b/src/hotspot/share/jvmci/jvmciEnv.cpp @@ -1750,7 +1750,7 @@ void JVMCIEnv::initialize_installed_code(JVMCIObject installed_code, CodeBlob* c } -void JVMCIEnv::invalidate_nmethod_mirror(JVMCIObject mirror, bool deoptimize, JVMCI_TRAPS) { +void JVMCIEnv::invalidate_nmethod_mirror(JVMCIObject mirror, bool deoptimize, nmethod::ChangeReason change_reason, JVMCI_TRAPS) { if (mirror.is_null()) { JVMCI_THROW(NullPointerException); } @@ -1773,7 +1773,7 @@ void JVMCIEnv::invalidate_nmethod_mirror(JVMCIObject mirror, bool deoptimize, JV if (!deoptimize) { // Prevent future executions of the nmethod but let current executions complete. - nm->make_not_entrant("JVMCI invalidate nmethod mirror"); + nm->make_not_entrant(change_reason); // Do not clear the address field here as the Java code may still // want to later call this method with deoptimize == true. That requires @@ -1782,7 +1782,7 @@ void JVMCIEnv::invalidate_nmethod_mirror(JVMCIObject mirror, bool deoptimize, JV // Deoptimize the nmethod immediately. DeoptimizationScope deopt_scope; deopt_scope.mark(nm); - nm->make_not_entrant("JVMCI invalidate nmethod mirror"); + nm->make_not_entrant(change_reason); nm->make_deoptimized(); deopt_scope.deoptimize_marked(); diff --git a/src/hotspot/share/jvmci/jvmciEnv.hpp b/src/hotspot/share/jvmci/jvmciEnv.hpp index 69f6647b0d6..b7b7c8f6771 100644 --- a/src/hotspot/share/jvmci/jvmciEnv.hpp +++ b/src/hotspot/share/jvmci/jvmciEnv.hpp @@ -462,7 +462,7 @@ public: // field of `mirror` to prevent it from being called. // If `deoptimize` is true, the nmethod is immediately deoptimized. // The HotSpotNmethod.address field is zero upon returning. - void invalidate_nmethod_mirror(JVMCIObject mirror, bool deoptimze, JVMCI_TRAPS); + void invalidate_nmethod_mirror(JVMCIObject mirror, bool deoptimze, nmethod::ChangeReason change_reason, JVMCI_TRAPS); void initialize_installed_code(JVMCIObject installed_code, CodeBlob* cb, JVMCI_TRAPS); diff --git a/src/hotspot/share/jvmci/jvmciRuntime.cpp b/src/hotspot/share/jvmci/jvmciRuntime.cpp index bee5f4ea445..1f10e132eff 100644 --- a/src/hotspot/share/jvmci/jvmciRuntime.cpp +++ b/src/hotspot/share/jvmci/jvmciRuntime.cpp @@ -2184,7 +2184,7 @@ JVMCI::CodeInstallResult JVMCIRuntime::register_method(JVMCIEnv* JVMCIENV, tty->print_cr("Replacing method %s", method_name); } if (old != nullptr) { - old->make_not_entrant("JVMCI register method"); + old->make_not_entrant(nmethod::ChangeReason::JVMCI_register_method); } LogTarget(Info, nmethod, install) lt; diff --git a/src/hotspot/share/oops/instanceKlass.cpp b/src/hotspot/share/oops/instanceKlass.cpp index b3d896b6863..32d7943e1e8 100644 --- a/src/hotspot/share/oops/instanceKlass.cpp +++ b/src/hotspot/share/oops/instanceKlass.cpp @@ -3492,7 +3492,7 @@ void InstanceKlass::add_osr_nmethod(nmethod* n) { for (int l = CompLevel_limited_profile; l < n->comp_level(); l++) { nmethod *inv = lookup_osr_nmethod(n->method(), n->osr_entry_bci(), l, true); if (inv != nullptr && inv->is_in_use()) { - inv->make_not_entrant("OSR invalidation of lower levels"); + inv->make_not_entrant(nmethod::ChangeReason::OSR_invalidation_of_lower_level); } } } diff --git a/src/hotspot/share/oops/method.cpp b/src/hotspot/share/oops/method.cpp index d13458dfa93..1a36fce23aa 100644 --- a/src/hotspot/share/oops/method.cpp +++ b/src/hotspot/share/oops/method.cpp @@ -1028,7 +1028,7 @@ void Method::set_native_function(address function, bool post_event_flag) { // If so, we have to make it not_entrant. nmethod* nm = code(); // Put it into local variable to guard against concurrent updates if (nm != nullptr) { - nm->make_not_entrant("set native function"); + nm->make_not_entrant(nmethod::ChangeReason::set_native_function); } } diff --git a/src/hotspot/share/prims/whitebox.cpp b/src/hotspot/share/prims/whitebox.cpp index f14c5efc65d..cf3b48c11b5 100644 --- a/src/hotspot/share/prims/whitebox.cpp +++ b/src/hotspot/share/prims/whitebox.cpp @@ -794,7 +794,7 @@ class VM_WhiteBoxDeoptimizeFrames : public VM_WhiteBoxOperation { if (_make_not_entrant) { nmethod* nm = CodeCache::find_nmethod(f->pc()); assert(nm != nullptr, "did not find nmethod"); - nm->make_not_entrant("Whitebox deoptimization"); + nm->make_not_entrant(nmethod::ChangeReason::whitebox_deoptimization); } ++_result; } diff --git a/src/hotspot/share/runtime/deoptimization.cpp b/src/hotspot/share/runtime/deoptimization.cpp index 4042bb01c58..a0d9dd00339 100644 --- a/src/hotspot/share/runtime/deoptimization.cpp +++ b/src/hotspot/share/runtime/deoptimization.cpp @@ -1826,7 +1826,7 @@ void Deoptimization::deoptimize(JavaThread* thread, frame fr, DeoptReason reason #if INCLUDE_JVMCI address Deoptimization::deoptimize_for_missing_exception_handler(nmethod* nm) { // there is no exception handler for this pc => deoptimize - nm->make_not_entrant("missing exception handler"); + nm->make_not_entrant(nmethod::ChangeReason::missing_exception_handler); // Use Deoptimization::deoptimize for all of its side-effects: // gathering traps statistics, logging... @@ -2455,7 +2455,7 @@ JRT_ENTRY(void, Deoptimization::uncommon_trap_inner(JavaThread* current, jint tr // Recompile if (make_not_entrant) { - if (!nm->make_not_entrant("uncommon trap")) { + if (!nm->make_not_entrant(nmethod::ChangeReason::uncommon_trap)) { return; // the call did not change nmethod's state } diff --git a/src/hotspot/share/runtime/javaThread.cpp b/src/hotspot/share/runtime/javaThread.cpp index 57f93f87d47..f3ad0e0a325 100644 --- a/src/hotspot/share/runtime/javaThread.cpp +++ b/src/hotspot/share/runtime/javaThread.cpp @@ -1337,7 +1337,7 @@ void JavaThread::make_zombies() { // it is a Java nmethod nmethod* nm = CodeCache::find_nmethod(fst.current()->pc()); assert(nm != nullptr, "did not find nmethod"); - nm->make_not_entrant("zombie"); + nm->make_not_entrant(nmethod::ChangeReason::zombie); } } }