Deoptimize zygote compiled methods in DeoptimizeBootImage.
Those methods don't get compiled with the "debuggable" flag,
so we need to deoptimize them.
Also fix a bug revealed by the new test where a concurrent
JIT collection happens when trying to disable it.
Also make DeoptimizeBootImage truly mutator lock exclusive.
Test: 689-zygote-jit-deopt
Change-Id: I00607dbe100350c5328293c35c87946fa97924b8
diff --git a/openjdkjvmti/OpenjdkJvmTi.cc b/openjdkjvmti/OpenjdkJvmTi.cc
index bd5b598..7a2b638 100644
--- a/openjdkjvmti/OpenjdkJvmTi.cc
+++ b/openjdkjvmti/OpenjdkJvmTi.cc
@@ -1552,7 +1552,7 @@
{
// Make sure we can deopt anything we need to.
- art::ScopedObjectAccess soa(art::Thread::Current());
+ art::ScopedSuspendAll ssa(__FUNCTION__);
gDeoptManager->FinishSetup();
}
diff --git a/openjdkjvmti/deopt_manager.h b/openjdkjvmti/deopt_manager.h
index d9f34a5..065baa7 100644
--- a/openjdkjvmti/deopt_manager.h
+++ b/openjdkjvmti/deopt_manager.h
@@ -107,7 +107,7 @@
void FinishSetup()
REQUIRES(!deoptimization_status_lock_, !art::Roles::uninterruptible_)
- REQUIRES_SHARED(art::Locks::mutator_lock_);
+ REQUIRES(art::Locks::mutator_lock_);
static DeoptManager* Get();
diff --git a/openjdkjvmti/ti_redefine.cc b/openjdkjvmti/ti_redefine.cc
index 6ca4e38..fd352b0 100644
--- a/openjdkjvmti/ti_redefine.cc
+++ b/openjdkjvmti/ti_redefine.cc
@@ -187,7 +187,9 @@
if (obsoleted_methods_.find(old_method) != obsoleted_methods_.end()) {
// We cannot ensure that the right dex file is used in inlined frames so we don't support
// redefining them.
- DCHECK(!IsInInlinedFrame()) << "Inlined frames are not supported when using redefinition";
+ DCHECK(!IsInInlinedFrame()) << "Inlined frames are not supported when using redefinition: "
+ << old_method->PrettyMethod() << " is inlined into "
+ << GetOuterMethod()->PrettyMethod();
art::ArtMethod* new_obsolete_method = obsolete_maps_->FindObsoleteVersion(old_method);
if (new_obsolete_method == nullptr) {
// Create a new Obsolete Method and put it in the list.
diff --git a/runtime/art_method.h b/runtime/art_method.h
index cc214f7..1e5ffaa 100644
--- a/runtime/art_method.h
+++ b/runtime/art_method.h
@@ -474,7 +474,7 @@
}
ProfilingInfo* GetProfilingInfo(PointerSize pointer_size) REQUIRES_SHARED(Locks::mutator_lock_) {
- if (UNLIKELY(IsNative()) || UNLIKELY(IsProxyMethod())) {
+ if (UNLIKELY(IsNative() || IsProxyMethod() || !IsInvokable())) {
return nullptr;
}
return reinterpret_cast<ProfilingInfo*>(GetDataPtrSize(pointer_size));
diff --git a/runtime/debugger.cc b/runtime/debugger.cc
index 80140b3..836e9b0 100644
--- a/runtime/debugger.cc
+++ b/runtime/debugger.cc
@@ -681,13 +681,13 @@
//
// The performance cost of this is non-negligible during native-debugging due to the
// forced JIT, so we keep the AOT code in that case in exchange for limited native debugging.
+ ScopedSuspendAll ssa(__FUNCTION__);
if (!runtime->IsJavaDebuggable() &&
!runtime->GetInstrumentation()->IsForcedInterpretOnly() &&
!runtime->IsNativeDebuggable()) {
runtime->DeoptimizeBootImage();
}
- ScopedSuspendAll ssa(__FUNCTION__);
if (RequiresDeoptimization()) {
runtime->GetInstrumentation()->EnableDeoptimization();
}
diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
index 27c47bc..36f7b3d 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -848,7 +848,7 @@
new_quick_code = GetQuickInstrumentationEntryPoint();
if (!method->IsNative() && Runtime::Current()->GetJit() != nullptr) {
// Native methods use trampoline entrypoints during interpreter tracing.
- DCHECK(!Runtime::Current()->GetJit()->GetCodeCache()->GetGarbageCollectCode());
+ DCHECK(!Runtime::Current()->GetJit()->GetCodeCache()->GetGarbageCollectCodeUnsafe());
ProfilingInfo* profiling_info = method->GetProfilingInfo(kRuntimePointerSize);
// Tracing will look at the saved entry point in the profiling info to know the actual
// entrypoint, so we store it here.
@@ -1050,14 +1050,6 @@
level = InstrumentationLevel::kInstrumentWithInterpreter;
} else {
level = InstrumentationLevel::kInstrumentWithInstrumentationStubs;
- if (Runtime::Current()->GetJit() != nullptr) {
- // TODO b/110263880 It would be better if we didn't need to do this.
- // Since we need to hold the method entrypoint across a suspend to ensure instrumentation
- // hooks are called correctly we have to disable jit-gc to ensure that the entrypoint doesn't
- // go away. Furthermore we need to leave this off permanently since one could get the same
- // effect by causing this to be toggled on and off.
- Runtime::Current()->GetJit()->GetCodeCache()->SetGarbageCollectCode(false);
- }
}
ConfigureStubs(key, level);
}
diff --git a/runtime/jit/jit_code_cache.cc b/runtime/jit/jit_code_cache.cc
index d976fec..c1b9a1a 100644
--- a/runtime/jit/jit_code_cache.cc
+++ b/runtime/jit/jit_code_cache.cc
@@ -390,7 +390,7 @@
// Zygote should never collect code to share the memory with the children.
if (is_zygote) {
- jit_code_cache->SetGarbageCollectCode(false);
+ jit_code_cache->garbage_collect_code_ = false;
}
if (!jit_code_cache->InitializeMappings(rwx_memory_allowed, is_zygote, error_msg)) {
@@ -1247,6 +1247,24 @@
}
}
+void JitCodeCache::ClearEntryPointsInZygoteExecSpace() {
+ MutexLock mu(Thread::Current(), lock_);
+ // Iterate over profiling infos to know which methods may have been JITted. Note that
+ // to be JITted, a method must have a profiling info.
+ for (ProfilingInfo* info : profiling_infos_) {
+ ArtMethod* method = info->GetMethod();
+ if (IsInZygoteExecSpace(method->GetEntryPointFromQuickCompiledCode())) {
+ method->SetEntryPointFromQuickCompiledCode(GetQuickToInterpreterBridge());
+ }
+ // If zygote does method tracing, or in some configuration where
+ // the JIT zygote does GC, we also need to clear the saved entry point
+ // in the profiling info.
+ if (IsInZygoteExecSpace(info->GetSavedEntryPoint())) {
+ info->SetSavedEntryPoint(nullptr);
+ }
+ }
+}
+
size_t JitCodeCache::CodeCacheSizeLocked() {
return used_memory_for_code_;
}
@@ -1437,17 +1455,14 @@
void JitCodeCache::GarbageCollectCache(Thread* self) {
ScopedTrace trace(__FUNCTION__);
- if (!garbage_collect_code_) {
- MutexLock mu(self, lock_);
- IncreaseCodeCacheCapacity();
- return;
- }
-
// Wait for an existing collection, or let everyone know we are starting one.
{
ScopedThreadSuspension sts(self, kSuspended);
MutexLock mu(self, lock_);
- if (WaitForPotentialCollectionToComplete(self)) {
+ if (!garbage_collect_code_) {
+ IncreaseCodeCacheCapacity();
+ return;
+ } else if (WaitForPotentialCollectionToComplete(self)) {
return;
} else {
number_of_collections_++;
@@ -1573,6 +1588,29 @@
FreeAllMethodHeaders(method_headers);
}
+bool JitCodeCache::GetGarbageCollectCode() {
+ MutexLock mu(Thread::Current(), lock_);
+ return garbage_collect_code_;
+}
+
+void JitCodeCache::SetGarbageCollectCode(bool value) {
+ Thread* self = Thread::Current();
+ MutexLock mu(self, lock_);
+ if (garbage_collect_code_ != value) {
+ if (garbage_collect_code_) {
+ // When dynamically disabling the garbage collection, we neee
+ // to make sure that a potential current collection is finished, and also
+ // clear the saved entry point in profiling infos to avoid dangling pointers.
+ WaitForPotentialCollectionToComplete(self);
+ for (ProfilingInfo* info : profiling_infos_) {
+ info->SetSavedEntryPoint(nullptr);
+ }
+ }
+ // Update the flag while holding the lock to ensure no thread will try to GC.
+ garbage_collect_code_ = value;
+ }
+}
+
void JitCodeCache::DoCollection(Thread* self, bool collect_profiling_info) {
ScopedTrace trace(__FUNCTION__);
{
diff --git a/runtime/jit/jit_code_cache.h b/runtime/jit/jit_code_cache.h
index e2f3357..8a6cebe 100644
--- a/runtime/jit/jit_code_cache.h
+++ b/runtime/jit/jit_code_cache.h
@@ -246,13 +246,13 @@
void MoveObsoleteMethod(ArtMethod* old_method, ArtMethod* new_method)
REQUIRES(!lock_) REQUIRES(Locks::mutator_lock_);
- // Dynamically change whether we want to garbage collect code. Should only be used during JIT
- // initialization or by tests.
- void SetGarbageCollectCode(bool value) {
- garbage_collect_code_ = value;
- }
+ // Dynamically change whether we want to garbage collect code.
+ void SetGarbageCollectCode(bool value) REQUIRES(!lock_);
- bool GetGarbageCollectCode() const {
+ bool GetGarbageCollectCode() REQUIRES(!lock_);
+
+ // Unsafe variant for debug checks.
+ bool GetGarbageCollectCodeUnsafe() const NO_THREAD_SAFETY_ANALYSIS {
return garbage_collect_code_;
}
@@ -264,6 +264,11 @@
void PostForkChildAction(bool is_system_server, bool is_zygote);
+ // Clear the entrypoints of JIT compiled methods that belong in the zygote space.
+ // This is used for removing non-debuggable JIT code at the point we realize the runtime
+ // is debuggable.
+ void ClearEntryPointsInZygoteExecSpace() REQUIRES(!lock_) REQUIRES(Locks::mutator_lock_);
+
private:
JitCodeCache();
@@ -451,7 +456,7 @@
bool last_collection_increased_code_cache_ GUARDED_BY(lock_);
// Whether we can do garbage collection. Not 'const' as tests may override this.
- bool garbage_collect_code_;
+ bool garbage_collect_code_ GUARDED_BY(lock_);
// The size in bytes of used memory for the data portion of the code cache.
size_t used_memory_for_data_ GUARDED_BY(lock_);
diff --git a/runtime/native/dalvik_system_ZygoteHooks.cc b/runtime/native/dalvik_system_ZygoteHooks.cc
index 9ce4749..26fc5e9 100644
--- a/runtime/native/dalvik_system_ZygoteHooks.cc
+++ b/runtime/native/dalvik_system_ZygoteHooks.cc
@@ -203,8 +203,11 @@
runtime->AddCompilerOption("--debuggable");
runtime_flags |= DEBUG_GENERATE_MINI_DEBUG_INFO;
runtime->SetJavaDebuggable(true);
- // Deoptimize the boot image as it may be non-debuggable.
- runtime->DeoptimizeBootImage();
+ {
+ // Deoptimize the boot image as it may be non-debuggable.
+ ScopedSuspendAll ssa(__FUNCTION__);
+ runtime->DeoptimizeBootImage();
+ }
runtime_flags &= ~DEBUG_JAVA_DEBUGGABLE;
needs_non_debuggable_classes = true;
}
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index 7eac3d9..10585e5 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -1452,6 +1452,8 @@
if (IsJavaDebuggable()) {
// Now that we have loaded the boot image, deoptimize its methods if we are running
// debuggable, as the code may have been compiled non-debuggable.
+ ScopedThreadSuspension sts(self, ThreadState::kNative);
+ ScopedSuspendAll ssa(__FUNCTION__);
DeoptimizeBootImage();
}
} else {
@@ -1551,10 +1553,14 @@
// Runtime initialization is largely done now.
// We load plugins first since that can modify the runtime state slightly.
// Load all plugins
- for (auto& plugin : plugins_) {
- std::string err;
- if (!plugin.Load(&err)) {
- LOG(FATAL) << plugin << " failed to load: " << err;
+ {
+ // The init method of plugins expect the state of the thread to be non runnable.
+ ScopedThreadSuspension sts(self, ThreadState::kNative);
+ for (auto& plugin : plugins_) {
+ std::string err;
+ if (!plugin.Load(&err)) {
+ LOG(FATAL) << plugin << " failed to load: " << err;
+ }
}
}
@@ -2627,6 +2633,7 @@
: instrumentation_(instrumentation) {}
bool operator()(ObjPtr<mirror::Class> klass) override REQUIRES(Locks::mutator_lock_) {
+ DCHECK(Locks::mutator_lock_->IsExclusiveHeld(Thread::Current()));
auto pointer_size = Runtime::Current()->GetClassLinker()->GetImagePointerSize();
for (auto& m : klass->GetMethods(pointer_size)) {
const void* code = m.GetEntryPointFromQuickCompiledCode();
@@ -2653,9 +2660,13 @@
// we patch entry points of methods in boot image to interpreter bridge, as
// boot image code may be AOT compiled as not debuggable.
if (!GetInstrumentation()->IsForcedInterpretOnly()) {
- ScopedObjectAccess soa(Thread::Current());
UpdateEntryPointsClassVisitor visitor(GetInstrumentation());
GetClassLinker()->VisitClasses(&visitor);
+ jit::Jit* jit = GetJit();
+ if (jit != nullptr) {
+ // Code JITted by the zygote is not compiled debuggable.
+ jit->GetCodeCache()->ClearEntryPointsInZygoteExecSpace();
+ }
}
}
} // namespace art
diff --git a/runtime/runtime.h b/runtime/runtime.h
index 76cfcd1..6877693 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -634,7 +634,7 @@
void SetJavaDebuggable(bool value);
// Deoptimize the boot image, called for Java debuggable apps.
- void DeoptimizeBootImage();
+ void DeoptimizeBootImage() REQUIRES(Locks::mutator_lock_);
bool IsNativeDebuggable() const {
return is_native_debuggable_;
diff --git a/runtime/stack.cc b/runtime/stack.cc
index 25939d2..9a677de 100644
--- a/runtime/stack.cc
+++ b/runtime/stack.cc
@@ -568,7 +568,9 @@
cur_shadow_frame_->SetMethod(method);
} else {
DCHECK(cur_quick_frame_ != nullptr);
- CHECK(!IsInInlinedFrame()) << "We do not support setting inlined method's ArtMethod!";
+ CHECK(!IsInInlinedFrame()) << "We do not support setting inlined method's ArtMethod: "
+ << GetMethod()->PrettyMethod() << " is inlined into "
+ << GetOuterMethod()->PrettyMethod();
*cur_quick_frame_ = method;
}
}
diff --git a/runtime/trace.cc b/runtime/trace.cc
index ce955d8..074e846 100644
--- a/runtime/trace.cc
+++ b/runtime/trace.cc
@@ -38,6 +38,8 @@
#include "entrypoints/quick/quick_entrypoints.h"
#include "gc/scoped_gc_critical_section.h"
#include "instrumentation.h"
+#include "jit/jit.h"
+#include "jit/jit_code_cache.h"
#include "mirror/class-inl.h"
#include "mirror/dex_cache-inl.h"
#include "mirror/object-inl.h"
@@ -390,6 +392,15 @@
// Enable count of allocs if specified in the flags.
bool enable_stats = false;
+ if (runtime->GetJit() != nullptr) {
+ // TODO b/110263880 It would be better if we didn't need to do this.
+ // Since we need to hold the method entrypoint across a suspend to ensure instrumentation
+ // hooks are called correctly we have to disable jit-gc to ensure that the entrypoint doesn't
+ // go away. Furthermore we need to leave this off permanently since one could get the same
+ // effect by causing this to be toggled on and off.
+ runtime->GetJit()->GetCodeCache()->SetGarbageCollectCode(false);
+ }
+
// Create Trace object.
{
// Required since EnableMethodTracing calls ConfigureStubs which visits class linker classes.
diff --git a/test/689-zygote-jit-deopt/expected.txt b/test/689-zygote-jit-deopt/expected.txt
new file mode 100644
index 0000000..6a5618e
--- /dev/null
+++ b/test/689-zygote-jit-deopt/expected.txt
@@ -0,0 +1 @@
+JNI_OnLoad called
diff --git a/test/689-zygote-jit-deopt/info.txt b/test/689-zygote-jit-deopt/info.txt
new file mode 100644
index 0000000..100032e
--- /dev/null
+++ b/test/689-zygote-jit-deopt/info.txt
@@ -0,0 +1,2 @@
+Regression test for debuggable apps that need to deoptimize
+methods JIT-compiled by the zygote.
diff --git a/test/689-zygote-jit-deopt/src/Main.java b/test/689-zygote-jit-deopt/src/Main.java
new file mode 100644
index 0000000..330663e
--- /dev/null
+++ b/test/689-zygote-jit-deopt/src/Main.java
@@ -0,0 +1,36 @@
+/*
+ * Copyright (C) 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+public class Main {
+ public static void main(String[] args) {
+ System.loadLibrary(args[0]);
+ if (!hasJit()) {
+ return;
+ }
+ ensureJitCompiled(Object.class, "toString");
+ transitionJitFromZygote();
+ deoptimizeBootImage();
+ if (hasJitCompiledEntrypoint(Object.class, "toString")) {
+ throw new Error("Expected Object.toString to be deoptimized");
+ }
+ }
+
+ private static native boolean hasJit();
+ private static native void ensureJitCompiled(Class<?> cls, String name);
+ private static native boolean hasJitCompiledEntrypoint(Class<?> cls, String name);
+ private static native void deoptimizeBootImage();
+ private static native void transitionJitFromZygote();
+}
diff --git a/test/common/runtime_state.cc b/test/common/runtime_state.cc
index 55631a9..bbc3039 100644
--- a/test/common/runtime_state.cc
+++ b/test/common/runtime_state.cc
@@ -161,6 +161,19 @@
return !interpreter;
}
+static ArtMethod* GetMethod(ScopedObjectAccess& soa, jclass cls, const ScopedUtfChars& chars)
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ CHECK(chars.c_str() != nullptr);
+ ArtMethod* method = soa.Decode<mirror::Class>(cls)->FindDeclaredDirectMethodByName(
+ chars.c_str(), kRuntimePointerSize);
+ if (method == nullptr) {
+ method = soa.Decode<mirror::Class>(cls)->FindDeclaredVirtualMethodByName(
+ chars.c_str(), kRuntimePointerSize);
+ }
+ DCHECK(method != nullptr) << "Unable to find method called " << chars.c_str();
+ return method;
+}
+
extern "C" JNIEXPORT jboolean JNICALL Java_Main_hasJitCompiledEntrypoint(JNIEnv* env,
jclass,
jclass cls,
@@ -172,9 +185,7 @@
Thread* self = Thread::Current();
ScopedObjectAccess soa(self);
ScopedUtfChars chars(env, method_name);
- CHECK(chars.c_str() != nullptr);
- ArtMethod* method = soa.Decode<mirror::Class>(cls)->FindDeclaredDirectMethodByName(
- chars.c_str(), kRuntimePointerSize);
+ ArtMethod* method = GetMethod(soa, cls, chars);
ScopedAssertNoThreadSuspension sants(__FUNCTION__);
return jit->GetCodeCache()->ContainsPc(
Runtime::Current()->GetInstrumentation()->GetCodeForInvoke(method));
@@ -191,9 +202,7 @@
Thread* self = Thread::Current();
ScopedObjectAccess soa(self);
ScopedUtfChars chars(env, method_name);
- CHECK(chars.c_str() != nullptr);
- ArtMethod* method = soa.Decode<mirror::Class>(cls)->FindDeclaredDirectMethodByName(
- chars.c_str(), kRuntimePointerSize);
+ ArtMethod* method = GetMethod(soa, cls, chars);
return jit->GetCodeCache()->ContainsMethod(method);
}
@@ -262,14 +271,7 @@
ScopedObjectAccess soa(self);
ScopedUtfChars chars(env, method_name);
- CHECK(chars.c_str() != nullptr);
- method = soa.Decode<mirror::Class>(cls)->FindDeclaredDirectMethodByName(
- chars.c_str(), kRuntimePointerSize);
- if (method == nullptr) {
- method = soa.Decode<mirror::Class>(cls)->FindDeclaredVirtualMethodByName(
- chars.c_str(), kRuntimePointerSize);
- }
- DCHECK(method != nullptr) << "Unable to find method called " << chars.c_str();
+ method = GetMethod(soa, cls, chars);
}
ForceJitCompiled(self, method);
}
@@ -353,4 +355,20 @@
return (jit != nullptr) ? jit->HotMethodThreshold() : 0;
}
+extern "C" JNIEXPORT void JNICALL Java_Main_transitionJitFromZygote(JNIEnv*, jclass) {
+ jit::Jit* jit = Runtime::Current()->GetJit();
+ if (jit == nullptr) {
+ return;
+ }
+ // Mimic the transition behavior a zygote fork would have.
+ jit->PreZygoteFork();
+ jit->GetCodeCache()->PostForkChildAction(/*is_system_server=*/ false, /*is_zygote=*/ false);
+ jit->PostForkChildAction(/*is_zygote=*/ false);
+}
+
+extern "C" JNIEXPORT void JNICALL Java_Main_deoptimizeBootImage(JNIEnv*, jclass) {
+ ScopedSuspendAll ssa(__FUNCTION__);
+ Runtime::Current()->DeoptimizeBootImage();
+}
+
} // namespace art
