Revert "Support calling entry / exit hooks from JIT code for non-debuggable"
This reverts commit bd95682d8a3ddb6f5e2ce1563d25e52ba2d823d7.
Reason for revert: Breaks git_master-art-host/art-forcecopy: https://android-build.googleplex.com/builds/submitted/7923798/art-forcecopy/latest/view/logs/build_error.log
Change-Id: I8d9b9bef6a9e4116b076c73dc5df3e6d19bcc79b
diff --git a/compiler/optimizing/optimizing_compiler.cc b/compiler/optimizing/optimizing_compiler.cc
index c3ccf95..16abf9d 100644
--- a/compiler/optimizing/optimizing_compiler.cc
+++ b/compiler/optimizing/optimizing_compiler.cc
@@ -800,8 +800,6 @@
dead_reference_safe = false;
}
- bool is_instrumentation_enabled =
- Runtime::Current()->GetInstrumentation()->NeedInstrumentationSupportForJIT();
HGraph* graph = new (allocator) HGraph(
allocator,
arena_stack,
@@ -811,7 +809,7 @@
compiler_options.GetInstructionSet(),
kInvalidInvokeType,
dead_reference_safe,
- compiler_options.GetDebuggable() || is_instrumentation_enabled,
+ compiler_options.GetDebuggable(),
compilation_kind);
if (method != nullptr) {
@@ -1298,8 +1296,7 @@
/* is_full_debug_info= */ compiler_options.GetGenerateDebugInfo(),
compilation_kind,
/* has_should_deoptimize_flag= */ false,
- cha_single_implementation_list,
- /* has_instrumentation_support= */ false)) {
+ cha_single_implementation_list)) {
code_cache->Free(self, region, reserved_code.data(), reserved_data.data());
return false;
}
@@ -1407,8 +1404,7 @@
/* is_full_debug_info= */ compiler_options.GetGenerateDebugInfo(),
compilation_kind,
codegen->GetGraph()->HasShouldDeoptimizeFlag(),
- codegen->GetGraph()->GetCHASingleImplementationList(),
- codegen->GetGraph()->IsDebuggable())) {
+ codegen->GetGraph()->GetCHASingleImplementationList())) {
code_cache->Free(self, region, reserved_code.data(), reserved_data.data());
return false;
}
diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
index c985c6b..8ef5ef4 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -234,6 +234,13 @@
return true;
}
+ // When jiting code for debuggable apps we generate the code to call method
+ // entry / exit hooks when required. Hence it is not required to update
+ // to instrumentation entry point for JITed code in debuggable mode.
+ if (!Runtime::Current()->IsJavaDebuggable()) {
+ return true;
+ }
+
// Native functions can have JITed entry points but we don't include support
// for calling entry / exit hooks directly from the JITed code for native
// functions. So we still have to install entry exit stubs for such cases.
@@ -783,28 +790,6 @@
}
void Instrumentation::UpdateInstrumentationLevel(InstrumentationLevel requested_level) {
- if (requested_level == instrumentation_level_) {
- return;
- }
-
- if (!Runtime::Current()->IsJavaDebuggable() &&
- (requested_level == InstrumentationLevel::kInstrumentWithInstrumentationStubs ||
- instrumentation_level_ == InstrumentationLevel::kInstrumentWithInstrumentationStubs)) {
- // In non-debuggable apps JITed code doesn't include code to call method entry / exit hooks
- // by default. So discard all the JITed code here so we can re-jit with the support when
- // the methods become hot again. For debuggable apps JITed code calls method entry / exit
- // hooks when required so nothing needs to be done there. Also discard JITed code when
- // moving away from instrumentation with stub, since the generated code disables some
- // optimizations when instrumentation is enabled.
- bool needs_instrumentation_support =
- (requested_level == InstrumentationLevel::kInstrumentWithInstrumentationStubs);
- jit::Jit* jit = Runtime::Current()->GetJit();
- if (jit != nullptr) {
- jit->GetCodeCache()->InvalidateAllCompiledCodeForInstrumentation(
- needs_instrumentation_support);
- }
- }
-
instrumentation_level_ = requested_level;
}
diff --git a/runtime/instrumentation.h b/runtime/instrumentation.h
index 38441e6..a505ce3 100644
--- a/runtime/instrumentation.h
+++ b/runtime/instrumentation.h
@@ -340,10 +340,6 @@
instrumentation_level_ == InstrumentationLevel::kInstrumentWithInterpreter;
}
- bool NeedInstrumentationSupportForJIT() const {
- return instrumentation_level_ == InstrumentationLevel::kInstrumentWithInstrumentationStubs;
- }
-
bool InterpreterStubsInstalled() const {
return instrumentation_level_ == InstrumentationLevel::kInstrumentWithInterpreter;
}
@@ -590,10 +586,8 @@
// directly call entry / exit hooks and don't need the stub.
bool CodeNeedsEntryExitStub(const void* code, ArtMethod* method);
- // Update the current instrumentation_level_. This takes care to discard JITed
- // code when kInstrumentWithInstrumentationStubs is requested and JITed code
- // isn't compiled with support for calling method entry / exit hooks.
- void UpdateInstrumentationLevel(InstrumentationLevel level) REQUIRES(Locks::mutator_lock_);
+ // Update the current instrumentation_level_.
+ void UpdateInstrumentationLevel(InstrumentationLevel level);
// Does the job of installing or removing instrumentation code within methods.
// In order to support multiple clients using instrumentation at the same time,
diff --git a/runtime/jit/jit_code_cache.cc b/runtime/jit/jit_code_cache.cc
index 4b9736f..e05ca11 100644
--- a/runtime/jit/jit_code_cache.cc
+++ b/runtime/jit/jit_code_cache.cc
@@ -263,7 +263,6 @@
JitCodeCache::JitCodeCache()
: is_weak_access_enabled_(true),
inline_cache_cond_("Jit inline cache condition variable", *Locks::jit_lock_),
- needs_instrumentation_support_(Runtime::Current()->IsJavaDebuggable()),
zygote_map_(&shared_region_),
lock_cond_("Jit code cache condition variable", *Locks::jit_lock_),
collection_in_progress_(false),
@@ -327,13 +326,7 @@
if (method->IsPreCompiled()) {
const void* code_ptr = nullptr;
if (method->GetDeclaringClass()->GetClassLoader() == nullptr) {
- MutexLock mu(Thread::Current(), *Locks::jit_lock_);
- // Don't use code from zygote_map_ if we need instrumentation support in
- // the JITed code. When instrumentation support is needed we re-jit the
- // code by generating code to call method entry / exit hooks.
- if (!needs_instrumentation_support_) {
- code_ptr = zygote_map_.GetCodeFor(method);
- }
+ code_ptr = zygote_map_.GetCodeFor(method);
} else {
MutexLock mu(Thread::Current(), *Locks::jit_lock_);
auto it = saved_compiled_methods_map_.find(method);
@@ -652,8 +645,7 @@
bool is_full_debug_info,
CompilationKind compilation_kind,
bool has_should_deoptimize_flag,
- const ArenaSet<ArtMethod*>& cha_single_implementation_list,
- bool has_instrumentation_support) {
+ const ArenaSet<ArtMethod*>& cha_single_implementation_list) {
DCHECK(!method->IsNative() || (compilation_kind != CompilationKind::kOsr));
if (!method->IsNative()) {
@@ -667,16 +659,6 @@
const uint8_t* stack_map_data = roots_data + root_table_size;
MutexLock mu(self, *Locks::jit_lock_);
-
- // Check if a non-native was compiled with the expected instrumentation support.
- // TODO(mythria): Currently native methods still use entry / exit stubs and we
- // don't compile them with instrumentation support. So this check can be
- // omitted for native methods for now. Update native methods to stop using
- // entry / exit stubs and update this check.
- if (!method->IsNative() && needs_instrumentation_support_ != has_instrumentation_support) {
- return false;
- }
-
// We need to make sure that there will be no jit-gcs going on and wait for any ongoing one to
// finish.
WaitForPotentialCollectionToCompleteRunnable(self);
@@ -1730,45 +1712,24 @@
}
}
-namespace {
-
-void RemoveCompiledCodeFromMethod(ArtMethod* method, ClassLinker* linker)
- REQUIRES_SHARED(Locks::mutator_lock_) {
- // We were compiled, so we must be warm.
- ClearMethodCounter(method, /*was_warm=*/true);
- if (method->IsObsolete()) {
- linker->SetEntryPointsForObsoleteMethod(method);
- } else {
- linker->SetEntryPointsToInterpreter(method);
- }
-}
-
-} // namespace
-
-void JitCodeCache::InvalidateAllCompiledCodeWithLock() {
+void JitCodeCache::InvalidateAllCompiledCode() {
+ art::MutexLock mu(Thread::Current(), *Locks::jit_lock_);
VLOG(jit) << "Invalidating all compiled code";
ClassLinker* linker = Runtime::Current()->GetClassLinker();
for (auto it : method_code_map_) {
- RemoveCompiledCodeFromMethod(it.second, linker);
- }
- for (auto it : zygote_map_) {
- RemoveCompiledCodeFromMethod(it.method, linker);
+ ArtMethod* meth = it.second;
+ // We were compiled, so we must be warm.
+ ClearMethodCounter(meth, /*was_warm=*/true);
+ if (meth->IsObsolete()) {
+ linker->SetEntryPointsForObsoleteMethod(meth);
+ } else {
+ linker->SetEntryPointsToInterpreter(meth);
+ }
}
saved_compiled_methods_map_.clear();
osr_code_map_.clear();
}
-void JitCodeCache::InvalidateAllCompiledCodeForInstrumentation(bool needs_instrumentation_support) {
- art::MutexLock mu(Thread::Current(), *Locks::jit_lock_);
- InvalidateAllCompiledCodeWithLock();
- needs_instrumentation_support_ = needs_instrumentation_support;
-}
-
-void JitCodeCache::InvalidateAllCompiledCode() {
- art::MutexLock mu(Thread::Current(), *Locks::jit_lock_);
- InvalidateAllCompiledCodeWithLock();
-}
-
void JitCodeCache::InvalidateCompiledCodeFor(ArtMethod* method,
const OatQuickMethodHeader* header) {
DCHECK(!method->IsNative());
diff --git a/runtime/jit/jit_code_cache.h b/runtime/jit/jit_code_cache.h
index 712c496..76b7f77 100644
--- a/runtime/jit/jit_code_cache.h
+++ b/runtime/jit/jit_code_cache.h
@@ -266,8 +266,7 @@
bool is_full_debug_info,
CompilationKind compilation_kind,
bool has_should_deoptimize_flag,
- const ArenaSet<ArtMethod*>& cha_single_implementation_list,
- bool has_instrumentation_support)
+ const ArenaSet<ArtMethod*>& cha_single_implementation_list)
REQUIRES_SHARED(Locks::mutator_lock_)
REQUIRES(!Locks::jit_lock_);
@@ -331,15 +330,9 @@
REQUIRES(!Locks::jit_lock_)
REQUIRES_SHARED(Locks::mutator_lock_);
- void InvalidateAllCompiledCodeWithLock()
- REQUIRES(Locks::jit_lock_)
- REQUIRES_SHARED(Locks::mutator_lock_);
void InvalidateAllCompiledCode()
REQUIRES(!Locks::jit_lock_)
REQUIRES_SHARED(Locks::mutator_lock_);
- void InvalidateAllCompiledCodeForInstrumentation(bool needs_instrumentation_support)
- REQUIRES(!Locks::jit_lock_)
- REQUIRES_SHARED(Locks::mutator_lock_);
void InvalidateCompiledCodeFor(ArtMethod* method, const OatQuickMethodHeader* code)
REQUIRES(!Locks::jit_lock_)
@@ -521,9 +514,6 @@
// Condition to wait on for accessing inline caches.
ConditionVariable inline_cache_cond_ GUARDED_BY(Locks::jit_lock_);
- // Specifies if instrumentation support is required by JITed code.
- bool needs_instrumentation_support_ GUARDED_BY(Locks::jit_lock_);
-
// -------------- JIT memory regions ------------------------------------- //
// Shared region, inherited from the zygote.
diff --git a/runtime/trace.cc b/runtime/trace.cc
index e9c8f49..4b5412f 100644
--- a/runtime/trace.cc
+++ b/runtime/trace.cc
@@ -426,12 +426,13 @@
instrumentation::Instrumentation::kMethodEntered |
instrumentation::Instrumentation::kMethodExited |
instrumentation::Instrumentation::kMethodUnwind);
- // For non-debuggable cases method tracing is best effort and we may not capture method
- // entry / exits for the methods that are inlined. We also don't report method exits for
- // JITed code currently on the stack. For java-debuggable it is precise since inlining is
- // disabled and the jit code is always compiled with instrumentation support.
- runtime->GetInstrumentation()->EnableMethodTracing(kTracerInstrumentationKey,
- /* needs_interpreter= */ false);
+ // TODO: In full-PIC mode, we don't need to fully deopt.
+ // TODO: We can only use trampoline entrypoints if we are java-debuggable since in that case
+ // we know that inlining and other problematic optimizations are disabled. We might just
+ // want to use the trampolines anyway since it is faster. It makes the story with disabling
+ // jit-gc more complex though.
+ runtime->GetInstrumentation()->EnableMethodTracing(
+ kTracerInstrumentationKey, /*needs_interpreter=*/!runtime->IsJavaDebuggable());
}
}
}