Avoid JVMTI global deoptimization when possible
This changes the openjdkjvmti plugin to be more controlled about the
situations that it will deoptimize everything. Most notably this makes
the plugin deoptimize only individual methods for breakpoints instead
of doing a full deoptimization. It also doesn't deoptimize for the
JVMTI_EVENT_EXCEPTION method, since our throwing code will always send
the appropriate event.
Impact:
Exoplayer benchmark with breakpointlogger setting a breakpoint on
a method that is never called.
The agent is the tools/breakpoint-logger agent.
'art' options are for all runs were:
--64
-Xusejit:true
-Xcompiler-option --debuggable
'art' options for 'Pre change' and 'Post change' runs included:
-Xplugin:libopenjdkjvmti.so
'-agentpath:libbreakpointlogger.so=Lbenchmarks/common/java/BenchmarkBase;->run()V@0'
Clean run (no agent loaded):
Running FMP4 x 1 : 53
Running TS x 1 : 144
Running FMP4 x 2500 : 3309
Running TS x 100 : 3584
ExoPlayerBench(RunTime): 6977000.0 us.
Pre change:
Running FMP4 x 1 : 159
Running TS x 1 : 9395
Running FMP4 x 2500 : 298591
Running TS x 100 : 944447
ExoPlayerBench(RunTime): 1.243226E9 us.
Post change:
Running FMP4 x 1 : 87
Running TS x 1 : 495
Running FMP4 x 2500 : 2939
Running TS x 100 : 3947
ExoPlayerBench(RunTime): 6979000.0 us.
Post change vs clean run is well within margin of error for this
benchmark.
Test: ./test.py --host -j50
Test: ./art/tools/run-prebuild-libjdwp-tests.sh
Bug: 62821960
Bug: 67958496
Change-Id: I63ef04f71c36c34d8534651d0c075921a836ec08
diff --git a/runtime/base/mutex.h b/runtime/base/mutex.h
index 189c0d0..4b56d3b 100644
--- a/runtime/base/mutex.h
+++ b/runtime/base/mutex.h
@@ -664,7 +664,7 @@
// When declaring any Mutex add DEFAULT_MUTEX_ACQUIRED_AFTER to use annotalysis to check the code
// doesn't try to hold a higher level Mutex.
- #define DEFAULT_MUTEX_ACQUIRED_AFTER ACQUIRED_AFTER(Locks::classlinker_classes_lock_)
+ #define DEFAULT_MUTEX_ACQUIRED_AFTER ACQUIRED_AFTER(art::Locks::classlinker_classes_lock_)
static Mutex* allocated_monitor_ids_lock_ ACQUIRED_AFTER(classlinker_classes_lock_);
diff --git a/runtime/debugger.cc b/runtime/debugger.cc
index 85df14a..8898afe 100644
--- a/runtime/debugger.cc
+++ b/runtime/debugger.cc
@@ -346,6 +346,10 @@
return Dbg::IsDebuggerActive();
}
+bool DebuggerActiveMethodInspectionCallback::IsMethodSafeToJit(ArtMethod* m) {
+ return !Dbg::MethodHasAnyBreakpoints(m);
+}
+
// Breakpoints.
static std::vector<Breakpoint> gBreakpoints GUARDED_BY(Locks::breakpoint_lock_);
diff --git a/runtime/debugger.h b/runtime/debugger.h
index 18126b1..ec37833 100644
--- a/runtime/debugger.h
+++ b/runtime/debugger.h
@@ -55,6 +55,7 @@
struct DebuggerActiveMethodInspectionCallback : public MethodInspectionCallback {
bool IsMethodBeingInspected(ArtMethod* m ATTRIBUTE_UNUSED)
OVERRIDE REQUIRES_SHARED(Locks::mutator_lock_);
+ bool IsMethodSafeToJit(ArtMethod* m) OVERRIDE REQUIRES_SHARED(Locks::mutator_lock_);
};
diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
index 2c82cb1..49f2021 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -137,11 +137,12 @@
method->SetEntryPointFromQuickCompiledCode(quick_code);
}
-bool Instrumentation::NeedDebugVersionFor(ArtMethod* method) const REQUIRES_SHARED(Locks::mutator_lock_) {
- return Dbg::IsDebuggerActive() &&
- Runtime::Current()->IsJavaDebuggable() &&
+bool Instrumentation::NeedDebugVersionFor(ArtMethod* method) const
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ return Runtime::Current()->IsJavaDebuggable() &&
!method->IsNative() &&
- !method->IsProxyMethod();
+ !method->IsProxyMethod() &&
+ Runtime::Current()->GetRuntimeCallbacks()->IsMethodBeingInspected(method);
}
void Instrumentation::InstallStubsForMethod(ArtMethod* method) {
diff --git a/runtime/jit/jit.cc b/runtime/jit/jit.cc
index 97a3b71..72b5a94 100644
--- a/runtime/jit/jit.cc
+++ b/runtime/jit/jit.cc
@@ -272,9 +272,12 @@
DCHECK(Runtime::Current()->UseJitCompilation());
DCHECK(!method->IsRuntimeMethod());
+ RuntimeCallbacks* cb = Runtime::Current()->GetRuntimeCallbacks();
// Don't compile the method if it has breakpoints.
- if (Dbg::IsDebuggerActive() && Dbg::MethodHasAnyBreakpoints(method)) {
- VLOG(jit) << "JIT not compiling " << method->PrettyMethod() << " due to breakpoint";
+ if (cb->IsMethodBeingInspected(method) && !cb->IsMethodSafeToJit(method)) {
+ VLOG(jit) << "JIT not compiling " << method->PrettyMethod()
+ << " due to not being safe to jit according to runtime-callbacks. For example, there"
+ << " could be breakpoints in this method.";
return false;
}
diff --git a/runtime/runtime_callbacks.cc b/runtime/runtime_callbacks.cc
index f164f7c..339fe82 100644
--- a/runtime/runtime_callbacks.cc
+++ b/runtime/runtime_callbacks.cc
@@ -43,6 +43,17 @@
Remove(cb, &method_inspection_callbacks_);
}
+bool RuntimeCallbacks::IsMethodSafeToJit(ArtMethod* m) {
+ for (MethodInspectionCallback* cb : method_inspection_callbacks_) {
+ if (!cb->IsMethodSafeToJit(m)) {
+ DCHECK(cb->IsMethodBeingInspected(m))
+ << "Contract requires that !IsMethodSafeToJit(m) -> IsMethodBeingInspected(m)";
+ return false;
+ }
+ }
+ return true;
+}
+
bool RuntimeCallbacks::IsMethodBeingInspected(ArtMethod* m) {
for (MethodInspectionCallback* cb : method_inspection_callbacks_) {
if (cb->IsMethodBeingInspected(m)) {
diff --git a/runtime/runtime_callbacks.h b/runtime/runtime_callbacks.h
index c936049..c1ba964 100644
--- a/runtime/runtime_callbacks.h
+++ b/runtime/runtime_callbacks.h
@@ -104,6 +104,11 @@
// Returns true if the method is being inspected currently and the runtime should not modify it in
// potentially dangerous ways (i.e. replace with compiled version, JIT it, etc).
virtual bool IsMethodBeingInspected(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_) = 0;
+
+ // Returns true if the method is safe to Jit, false otherwise.
+ // Note that '!IsMethodSafeToJit(m) implies IsMethodBeingInspected(m)'. That is that if this
+ // method returns false IsMethodBeingInspected must return true.
+ virtual bool IsMethodSafeToJit(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_) = 0;
};
class RuntimeCallbacks {
@@ -167,6 +172,11 @@
// on by some code.
bool IsMethodBeingInspected(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_);
+ // Returns false if some MethodInspectionCallback indicates the method cannot be safetly jitted
+ // (which implies that it is being Inspected). Returns true otherwise. If it returns false the
+ // entrypoint should not be changed to JITed code.
+ bool IsMethodSafeToJit(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_);
+
void AddMethodInspectionCallback(MethodInspectionCallback* cb)
REQUIRES_SHARED(Locks::mutator_lock_);
void RemoveMethodInspectionCallback(MethodInspectionCallback* cb)