Reland "Use InitializeMethodsCode when we need to reinitialize a method entrypoint."
This reverts commit b0f63c93e44bf53a34dda90c74b8725a932922da.
Reason for revert: Fix For proxy init.
Test: test.py
Change-Id: I6b4796115d73a093f86309cec1f03cafd981c2e9
diff --git a/compiler/common_compiler_test.cc b/compiler/common_compiler_test.cc
index 1b69f2a..bbb2016 100644
--- a/compiler/common_compiler_test.cc
+++ b/compiler/common_compiler_test.cc
@@ -149,20 +149,18 @@
void CommonCompilerTestImpl::MakeExecutable(ArtMethod* method,
const CompiledMethod* compiled_method) {
CHECK(method != nullptr);
+ const void* method_code = nullptr;
// If the code size is 0 it means the method was skipped due to profile guided compilation.
if (compiled_method != nullptr && compiled_method->GetQuickCode().size() != 0u) {
const void* code_ptr = MakeExecutable(compiled_method->GetQuickCode(),
compiled_method->GetVmapTable(),
compiled_method->GetInstructionSet());
- const void* method_code =
+ method_code =
CompiledMethod::CodePointer(code_ptr, compiled_method->GetInstructionSet());
LOG(INFO) << "MakeExecutable " << method->PrettyMethod() << " code=" << method_code;
- method->SetEntryPointFromQuickCompiledCode(method_code);
- } else {
- // No code? You must mean to go into the interpreter.
- // Or the generic JNI...
- GetClassLinker()->SetEntryPointsToInterpreter(method);
}
+ Runtime::Current()->GetInstrumentation()->InitializeMethodsCode(
+ method, /*aot_code=*/ method_code);
}
void CommonCompilerTestImpl::SetUp() {
diff --git a/openjdkjvmti/events.cc b/openjdkjvmti/events.cc
index c69ee7b..a6425af 100644
--- a/openjdkjvmti/events.cc
+++ b/openjdkjvmti/events.cc
@@ -1260,7 +1260,7 @@
continue;
} else if (!runtime_->GetClassLinker()->IsQuickToInterpreterBridge(code) &&
!runtime_->IsAsyncDeoptimizeable(reinterpret_cast<uintptr_t>(code))) {
- runtime_->GetInstrumentation()->UpdateMethodsCodeToInterpreterEntryPoint(&m);
+ runtime_->GetInstrumentation()->InitializeMethodsCode(&m, /*aot_code=*/ nullptr);
}
}
return true;
diff --git a/openjdkjvmti/ti_redefine.cc b/openjdkjvmti/ti_redefine.cc
index 37a61d3..c234bd4 100644
--- a/openjdkjvmti/ti_redefine.cc
+++ b/openjdkjvmti/ti_redefine.cc
@@ -2116,7 +2116,7 @@
}
// Finish setting up methods.
linked_class->VisitMethods([&](art::ArtMethod* m) REQUIRES_SHARED(art::Locks::mutator_lock_) {
- linker->SetEntryPointsToInterpreter(m);
+ driver_->runtime_->GetInstrumentation()->InitializeMethodsCode(m, /* aot_code= */ nullptr);
m->SetNotIntrinsic();
DCHECK(m->IsCopied() || m->GetDeclaringClass() == linked_class.Get())
<< m->PrettyMethod()
@@ -2543,7 +2543,7 @@
CHECK(method_id != nullptr);
uint32_t dex_method_idx = dex_file_->GetIndexForMethodId(*method_id);
method.SetDexMethodIndex(dex_method_idx);
- linker->SetEntryPointsToInterpreter(&method);
+ driver_->runtime_->GetInstrumentation()->InitializeMethodsCode(&method, /*aot_code=*/ nullptr);
if (method.HasCodeItem()) {
method.SetCodeItem(
dex_file_->GetCodeItem(dex_file_->FindCodeItemOffset(class_def, dex_method_idx)),
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 5ae6513..5eab75d 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -201,22 +201,6 @@
self->AssertPendingException();
}
-static void ChangeInterpreterBridgeToNterp(ArtMethod* method, ClassLinker* class_linker)
- REQUIRES_SHARED(Locks::mutator_lock_) {
- Runtime* runtime = Runtime::Current();
- if (class_linker->IsQuickToInterpreterBridge(method->GetEntryPointFromQuickCompiledCode()) &&
- CanMethodUseNterp(method)) {
- if (method->GetDeclaringClass()->IsVisiblyInitialized() ||
- !NeedsClinitCheckBeforeCall(method)) {
- runtime->GetInstrumentation()->UpdateMethodsCode(method, interpreter::GetNterpEntryPoint());
- } else {
- // Put the resolution stub, which will initialize the class and then
- // call the method with nterp.
- runtime->GetInstrumentation()->UpdateMethodsCode(method, GetQuickResolutionStub());
- }
- }
-}
-
static void UpdateClassAfterVerification(Handle<mirror::Class> klass,
PointerSize pointer_size,
verifier::FailureKind failure_kind)
@@ -231,7 +215,9 @@
// to methods that currently use the switch interpreter.
if (interpreter::CanRuntimeUseNterp()) {
for (ArtMethod& m : klass->GetMethods(pointer_size)) {
- ChangeInterpreterBridgeToNterp(&m, class_linker);
+ if (class_linker->IsQuickToInterpreterBridge(m.GetEntryPointFromQuickCompiledCode())) {
+ runtime->GetInstrumentation()->InitializeMethodsCode(&m, /*aot_code=*/nullptr);
+ }
}
}
}
@@ -2008,10 +1994,11 @@
// reset it with the runtime value.
method.ResetCounter(hotness_threshold);
}
- // Set image methods' entry point that point to the interpreter bridge to the
- // nterp entry point.
if (method.GetEntryPointFromQuickCompiledCode() == nterp_trampoline_) {
if (can_use_nterp) {
+ // Set image methods' entry point that point to the nterp trampoline to the
+ // nterp entry point. This allows taking the fast path when doing a
+ // nterp->nterp call.
DCHECK(!NeedsClinitCheckBeforeCall(&method) ||
method.GetDeclaringClass()->IsVisiblyInitialized());
method.SetEntryPointFromQuickCompiledCode(interpreter::GetNterpEntryPoint());
@@ -3388,6 +3375,7 @@
return;
}
+ instrumentation::Instrumentation* instrumentation = runtime->GetInstrumentation();
// Link the code of methods skipped by LinkCode.
for (size_t method_index = 0; method_index < num_direct_methods; ++method_index) {
ArtMethod* method = klass->GetDirectMethod(method_index, pointer_size);
@@ -3395,7 +3383,6 @@
// Only update static methods.
continue;
}
- instrumentation::Instrumentation* instrumentation = runtime->GetInstrumentation();
instrumentation->UpdateMethodsCode(method, instrumentation->GetCodeForInvoke(method));
}
// Ignore virtual methods on the iterator.
@@ -9779,14 +9766,6 @@
return GetQuickGenericJniStub();
}
-void ClassLinker::SetEntryPointsToInterpreter(ArtMethod* method) const {
- if (!method->IsNative()) {
- method->SetEntryPointFromQuickCompiledCode(GetQuickToInterpreterBridge());
- } else {
- method->SetEntryPointFromQuickCompiledCode(GetQuickGenericJniStub());
- }
-}
-
void ClassLinker::SetEntryPointsForObsoleteMethod(ArtMethod* method) const {
DCHECK(method->IsObsolete());
// We cannot mess with the entrypoints of native methods because they are used to determine how
diff --git a/runtime/class_linker.h b/runtime/class_linker.h
index 22a8c7f..e2b8794 100644
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -616,10 +616,6 @@
return intern_table_;
}
- // Set the entrypoints up for method to the enter the interpreter.
- void SetEntryPointsToInterpreter(ArtMethod* method) const
- REQUIRES_SHARED(Locks::mutator_lock_);
-
// Set the entrypoints up for an obsolete method.
void SetEntryPointsForObsoleteMethod(ArtMethod* method) const
REQUIRES_SHARED(Locks::mutator_lock_);
diff --git a/runtime/common_runtime_test.cc b/runtime/common_runtime_test.cc
index c8cd151..d7c77e8 100644
--- a/runtime/common_runtime_test.cc
+++ b/runtime/common_runtime_test.cc
@@ -406,7 +406,7 @@
void CommonRuntimeTestImpl::MakeInterpreted(ObjPtr<mirror::Class> klass) {
PointerSize pointer_size = class_linker_->GetImagePointerSize();
for (ArtMethod& method : klass->GetMethods(pointer_size)) {
- class_linker_->SetEntryPointsToInterpreter(&method);
+ Runtime::Current()->GetInstrumentation()->InitializeMethodsCode(&method, /*aot_code=*/ nullptr);
}
}
diff --git a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
index 027bda5..926d534 100644
--- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
@@ -1052,7 +1052,7 @@
!jit->GetCodeCache()->ContainsPc(result))
<< method->PrettyMethod() << " code will jump to possibly cleaned up jit code!";
- bool interpreter_entry = (result == GetQuickToInterpreterBridge());
+ bool interpreter_entry = Runtime::Current()->GetClassLinker()->IsQuickToInterpreterBridge(result);
bool is_static = method->IsStatic();
uint32_t shorty_len;
const char* shorty =
diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
index 091c98d..b4abf00 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -209,6 +209,24 @@
(code == GetQuickInstrumentationEntryPoint());
}
+static bool IsProxyInit(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_) {
+ // Annoyingly this can be called before we have actually initialized WellKnownClasses so therefore
+ // we also need to check this based on the declaring-class descriptor. The check is valid because
+ // Proxy only has a single constructor.
+ ArtMethod* well_known_proxy_init = jni::DecodeArtMethod(
+ WellKnownClasses::java_lang_reflect_Proxy_init);
+ if (well_known_proxy_init == method) {
+ return true;
+ }
+
+ if (well_known_proxy_init != nullptr) {
+ return false;
+ }
+
+ return method->IsConstructor() &&
+ method->GetDeclaringClass()->DescriptorEquals("Ljava/lang/reflect/Proxy;");
+}
+
static void UpdateEntryPoints(ArtMethod* method, const void* quick_code)
REQUIRES_SHARED(Locks::mutator_lock_) {
if (kIsDebugBuild) {
@@ -223,6 +241,9 @@
CHECK_EQ(reinterpret_cast<uintptr_t>(quick_code) & 1, 1u);
}
}
+ if (IsProxyInit(method)) {
+ CHECK_NE(quick_code, GetQuickInstrumentationEntryPoint());
+ }
}
// If the method is from a boot image, don't dirty it if the entrypoint
// doesn't change.
@@ -231,7 +252,13 @@
}
}
-bool Instrumentation::CodeNeedsEntryExitStub(const void* code, ArtMethod* method) {
+bool Instrumentation::CodeNeedsEntryExitStub(const void* code, ArtMethod* method)
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ // Proxy.init should never have entry/exit stubs.
+ if (IsProxyInit(method)) {
+ return false;
+ }
+
// In some tests runtime isn't setup fully and hence the entry points could
// be nullptr.
if (code == nullptr) {
@@ -264,24 +291,6 @@
return true;
}
-static bool IsProxyInit(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_) {
- // Annoyingly this can be called before we have actually initialized WellKnownClasses so therefore
- // we also need to check this based on the declaring-class descriptor. The check is valid because
- // Proxy only has a single constructor.
- ArtMethod* well_known_proxy_init = jni::DecodeArtMethod(
- WellKnownClasses::java_lang_reflect_Proxy_init);
- if (well_known_proxy_init == method) {
- return true;
- }
-
- if (well_known_proxy_init != nullptr) {
- return false;
- }
-
- return method->IsConstructor() &&
- method->GetDeclaringClass()->DescriptorEquals("Ljava/lang/reflect/Proxy;");
-}
-
bool Instrumentation::InterpretOnly(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_) {
if (method->IsNative()) {
return false;
@@ -320,6 +329,12 @@
return true;
}
+static bool CanUseNterp(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_) {
+ return interpreter::CanRuntimeUseNterp() &&
+ CanMethodUseNterp(method) &&
+ method->GetDeclaringClass()->IsVerified();
+}
+
static const void* GetOptimizedCodeFor(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_) {
DCHECK(!Runtime::Current()->GetInstrumentation()->InterpretOnly(method));
CHECK(method->IsInvokable()) << method->PrettyMethod();
@@ -346,9 +361,7 @@
// We need to check if the class has been verified for setting up nterp, as
// the verifier could punt the method to the switch interpreter in case we
// need to do lock counting.
- if (interpreter::CanRuntimeUseNterp() &&
- CanMethodUseNterp(method) &&
- method->GetDeclaringClass()->IsVerified()) {
+ if (CanUseNterp(method)) {
return interpreter::GetNterpEntryPoint();
}
@@ -358,7 +371,7 @@
void Instrumentation::InitializeMethodsCode(ArtMethod* method, const void* aot_code)
REQUIRES_SHARED(Locks::mutator_lock_) {
// Use instrumentation entrypoints if instrumentation is installed.
- if (UNLIKELY(EntryExitStubsInstalled())) {
+ if (UNLIKELY(EntryExitStubsInstalled()) && !IsProxyInit(method)) {
if (!method->IsNative() && InterpretOnly()) {
UpdateEntryPoints(method, GetQuickToInterpreterBridge());
} else {
@@ -374,15 +387,15 @@
}
// Special case if we need an initialization check.
- if (NeedsClinitCheckBeforeCall(method)) {
+ if (NeedsClinitCheckBeforeCall(method) && !method->GetDeclaringClass()->IsVisiblyInitialized()) {
// If we have code but the method needs a class initialization check before calling
// that code, install the resolution stub that will perform the check.
// It will be replaced by the proper entry point by ClassLinker::FixupStaticTrampolines
// after initializing class (see ClassLinker::InitializeClass method).
// Note: this mimics the logic in image_writer.cc that installs the resolution
- // stub only if we have compiled code and the method needs a class initialization
- // check.
- if (aot_code != nullptr || method->IsNative()) {
+ // stub only if we have compiled code or we can execute nterp, and the method needs a class
+ // initialization check.
+ if (aot_code != nullptr || method->IsNative() || CanUseNterp(method)) {
UpdateEntryPoints(method, GetQuickResolutionStub());
} else {
UpdateEntryPoints(method, GetQuickToInterpreterBridge());
@@ -396,10 +409,15 @@
return;
}
+ // We check if the class is verified as we need the slow interpreter for lock verification.
+ // If the class is not verified, This will be updated in
+ // ClassLinker::UpdateClassAfterVerification.
+ if (CanUseNterp(method)) {
+ UpdateEntryPoints(method, interpreter::GetNterpEntryPoint());
+ return;
+ }
+
// Use default entrypoints.
- // Note we cannot use the nterp entrypoint because we do not know if the
- // method will need the slow interpreter for lock verification. This will
- // be updated in ClassLinker::UpdateClassAfterVerification.
UpdateEntryPoints(
method, method->IsNative() ? GetQuickGenericJniStub() : GetQuickToInterpreterBridge());
}
@@ -1097,19 +1115,6 @@
UpdateMethodsCodeImpl(method, new_code);
}
-void Instrumentation::UpdateMethodsCodeToInterpreterEntryPoint(ArtMethod* method) {
- UpdateMethodsCodeImpl(method, GetQuickToInterpreterBridge());
-}
-
-void Instrumentation::UpdateMethodsCodeForJavaDebuggable(ArtMethod* method,
- const void* new_code) {
- // When the runtime is set to Java debuggable, we may update the entry points of
- // all methods of a class to the interpreter bridge. A method's declaring class
- // might not be in resolved state yet in that case, so we bypass the DCHECK in
- // UpdateMethodsCode.
- UpdateMethodsCodeImpl(method, new_code);
-}
-
bool Instrumentation::AddDeoptimizedMethod(ArtMethod* method) {
if (IsDeoptimizedMethod(method)) {
// Already in the map. Return.
diff --git a/runtime/instrumentation.h b/runtime/instrumentation.h
index 1c82163..e53ee82 100644
--- a/runtime/instrumentation.h
+++ b/runtime/instrumentation.h
@@ -301,14 +301,6 @@
void UpdateNativeMethodsCodeToJitCode(ArtMethod* method, const void* new_code)
REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(!GetDeoptimizedMethodsLock());
- // Update the code of a method to the interpreter respecting any installed stubs from debugger.
- void UpdateMethodsCodeToInterpreterEntryPoint(ArtMethod* method)
- REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(!GetDeoptimizedMethodsLock());
-
- // Update the code of a method respecting any installed stubs from debugger.
- void UpdateMethodsCodeForJavaDebuggable(ArtMethod* method, const void* new_code)
- REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(!GetDeoptimizedMethodsLock());
-
// Return the code that we can execute for an invoke including from the JIT.
const void* GetCodeForInvoke(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_);
@@ -561,7 +553,8 @@
// Returns true if we need entry exit stub to call entry hooks. JITed code
// directly call entry / exit hooks and don't need the stub.
- bool CodeNeedsEntryExitStub(const void* code, ArtMethod* method);
+ static bool CodeNeedsEntryExitStub(const void* code, ArtMethod* method)
+ REQUIRES_SHARED(Locks::mutator_lock_);
// Update the current instrumentation_level_.
void UpdateInstrumentationLevel(InstrumentationLevel level);
diff --git a/runtime/jit/jit_code_cache.cc b/runtime/jit/jit_code_cache.cc
index 8b91f9e..34ddfc4 100644
--- a/runtime/jit/jit_code_cache.cc
+++ b/runtime/jit/jit_code_cache.cc
@@ -793,8 +793,7 @@
}
ClearMethodCounter(method, /* was_warm= */ false);
- Runtime::Current()->GetInstrumentation()->UpdateMethodsCode(
- method, GetQuickToInterpreterBridge());
+ Runtime::Current()->GetInstrumentation()->InitializeMethodsCode(method, /*aot_code=*/ nullptr);
VLOG(jit)
<< "JIT removed (osr=" << std::boolalpha << osr << std::noboolalpha << ") "
<< ArtMethod::PrettyMethod(method) << "@" << method
@@ -1318,7 +1317,8 @@
OatQuickMethodHeader::FromEntryPoint(entry_point);
if (CodeInfo::IsBaseline(method_header->GetOptimizedCodeInfoPtr())) {
info->GetMethod()->ResetCounter(warmup_threshold);
- info->GetMethod()->SetEntryPointFromQuickCompiledCode(GetQuickToInterpreterBridge());
+ Runtime::Current()->GetInstrumentation()->InitializeMethodsCode(
+ info->GetMethod(), /*aot_code=*/ nullptr);
}
}
}
@@ -1744,7 +1744,7 @@
if (meth->IsObsolete()) {
linker->SetEntryPointsForObsoleteMethod(meth);
} else {
- linker->SetEntryPointsToInterpreter(meth);
+ Runtime::Current()->GetInstrumentation()->InitializeMethodsCode(meth, /*aot_code=*/ nullptr);
}
}
saved_compiled_methods_map_.clear();
@@ -1761,8 +1761,7 @@
if (method_entrypoint == header->GetEntryPoint()) {
// The entrypoint is the one to invalidate, so we just update it to the interpreter entry point
// and clear the counter to get the method Jitted again.
- Runtime::Current()->GetInstrumentation()->UpdateMethodsCode(
- method, GetQuickToInterpreterBridge());
+ Runtime::Current()->GetInstrumentation()->InitializeMethodsCode(method, /*aot_code=*/ nullptr);
ClearMethodCounter(method, /*was_warm=*/ true);
} else {
MutexLock mu(Thread::Current(), *Locks::jit_lock_);
diff --git a/runtime/quick_exception_handler.cc b/runtime/quick_exception_handler.cc
index ac5065b..c372248 100644
--- a/runtime/quick_exception_handler.cc
+++ b/runtime/quick_exception_handler.cc
@@ -606,9 +606,8 @@
Runtime::Current()->GetJit()->GetCodeCache()->InvalidateCompiledCodeFor(
deopt_method, visitor.GetSingleFrameDeoptQuickMethodHeader());
} else {
- // Transfer the code to interpreter.
- Runtime::Current()->GetInstrumentation()->UpdateMethodsCode(
- deopt_method, GetQuickToInterpreterBridge());
+ Runtime::Current()->GetInstrumentation()->InitializeMethodsCode(
+ deopt_method, /*aot_code=*/ nullptr);
}
PrepareForLongJumpToInvokeStubOrInterpreterBridge();
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index 07b992d..cd8f936 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -3097,21 +3097,21 @@
if (Runtime::Current()->GetHeap()->IsInBootImageOatFile(code) &&
!m.IsNative() &&
!m.IsProxyMethod()) {
- instrumentation_->UpdateMethodsCodeForJavaDebuggable(&m, GetQuickToInterpreterBridge());
+ instrumentation_->InitializeMethodsCode(&m, /*aot_code=*/ nullptr);
}
if (Runtime::Current()->GetJit() != nullptr &&
Runtime::Current()->GetJit()->GetCodeCache()->IsInZygoteExecSpace(code) &&
!m.IsNative()) {
DCHECK(!m.IsProxyMethod());
- instrumentation_->UpdateMethodsCodeForJavaDebuggable(&m, GetQuickToInterpreterBridge());
+ instrumentation_->InitializeMethodsCode(&m, /*aot_code=*/ nullptr);
}
if (m.IsPreCompiled()) {
// Precompilation is incompatible with debuggable, so clear the flag
// and update the entrypoint in case it has been compiled.
m.ClearPreCompiled();
- instrumentation_->UpdateMethodsCodeForJavaDebuggable(&m, GetQuickToInterpreterBridge());
+ instrumentation_->InitializeMethodsCode(&m, /*aot_code=*/ nullptr);
}
}
return true;
diff --git a/runtime/runtime_callbacks_test.cc b/runtime/runtime_callbacks_test.cc
index 7619750..7f64721 100644
--- a/runtime/runtime_callbacks_test.cc
+++ b/runtime/runtime_callbacks_test.cc
@@ -80,7 +80,7 @@
PointerSize pointer_size = class_linker_->GetImagePointerSize();
for (auto& m : klass->GetMethods(pointer_size)) {
if (!m.IsAbstract()) {
- class_linker_->SetEntryPointsToInterpreter(&m);
+ Runtime::Current()->GetInstrumentation()->InitializeMethodsCode(&m, /*aot_code=*/ nullptr);
}
}
}