Cleanup around method inspection callbacks
IsMethodBeingInspected callback was used to prevent moving to compiled
code (JIT / OSR / Nterp) while a method has breakpoints or other
debugger related events enabled. We also use Instrumentation::IsDeoptimized
for similar purpose in some places (for ex: when determining if a method
needs to be deoptimized). Deoptimized methods list is a super set of
methods that have breakpoints on them. So we can replace most method
inspection callbacks to use IsDeoptimized check instead.
The one exception is when we decide if we need to OSR a method. Here we
need to check if the frame has any locals changed which isn't covered by
IsDeoptimized check and requires a runtime callback to see if any locals
have changed.
This CL:
1. Adds a new runtime callback called HaveLocalsChanged to check if any
locals have changed and uses it to prevent OSRing such frames.
2. Removes IsMethodBeingInspected and replaces it with IsDeoptimized /
HaveLocalsChanged as required.
Bug: 206029744
Test: art/test.py
Change-Id: Ie649cffaeba3d31746527e0ee326abe81284978d
diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
index 91d0fae..3b17bb5 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -285,9 +285,7 @@
if (method->IsNative()) {
return false;
}
- return InterpretOnly() ||
- IsDeoptimized(method) ||
- Runtime::Current()->GetRuntimeCallbacks()->IsMethodBeingInspected(method);
+ return InterpretOnly() || IsDeoptimized(method);
}
static bool CanUseAotCode(const void* quick_code)
diff --git a/runtime/jit/jit.cc b/runtime/jit/jit.cc
index 5d0430e..91869c6 100644
--- a/runtime/jit/jit.cc
+++ b/runtime/jit/jit.cc
@@ -282,9 +282,8 @@
compilation_kind = CompilationKind::kOptimized;
}
- RuntimeCallbacks* cb = Runtime::Current()->GetRuntimeCallbacks();
// Don't compile the method if it has breakpoints.
- if (cb->IsMethodBeingInspected(method)) {
+ if (Runtime::Current()->GetInstrumentation()->IsDeoptimized(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.";
@@ -571,8 +570,11 @@
// Before allowing the jump, make sure no code is actively inspecting the method to avoid
// jumping from interpreter to OSR while e.g. single stepping. Note that we could selectively
// disable OSR when single stepping, but that's currently hard to know at this point.
+ // Currently, HaveLocalsChanged is not frame specific. It is possible to make it frame specific
+ // to allow OSR of frames that don't have any locals changed but it isn't worth the additional
+ // complexity.
if (Runtime::Current()->GetInstrumentation()->NeedsSlowInterpreterForMethod(thread, method) ||
- Runtime::Current()->GetRuntimeCallbacks()->IsMethodBeingInspected(method)) {
+ Runtime::Current()->GetRuntimeCallbacks()->HaveLocalsChanged()) {
return false;
}
diff --git a/runtime/runtime_callbacks.cc b/runtime/runtime_callbacks.cc
index 753ac28..28c81a2 100644
--- a/runtime/runtime_callbacks.cc
+++ b/runtime/runtime_callbacks.cc
@@ -105,9 +105,9 @@
Remove(cb, &method_inspection_callbacks_);
}
-bool RuntimeCallbacks::IsMethodBeingInspected(ArtMethod* m) {
+bool RuntimeCallbacks::HaveLocalsChanged() {
for (MethodInspectionCallback* cb : COPY(method_inspection_callbacks_)) {
- if (cb->IsMethodBeingInspected(m)) {
+ if (cb->HaveLocalsChanged()) {
return true;
}
}
diff --git a/runtime/runtime_callbacks.h b/runtime/runtime_callbacks.h
index b1a7e55..98584a8 100644
--- a/runtime/runtime_callbacks.h
+++ b/runtime/runtime_callbacks.h
@@ -143,9 +143,8 @@
public:
virtual ~MethodInspectionCallback() {}
- // 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 any locals have changed. If any locals have changed we shouldn't OSR.
+ virtual bool HaveLocalsChanged() REQUIRES_SHARED(Locks::mutator_lock_) = 0;
};
// Callback to let something request to be notified when reflective objects are being visited and
@@ -225,9 +224,9 @@
void AddParkCallback(ParkCallback* cb) REQUIRES_SHARED(Locks::mutator_lock_);
void RemoveParkCallback(ParkCallback* cb) REQUIRES_SHARED(Locks::mutator_lock_);
- // Returns true if some MethodInspectionCallback indicates the method is being inspected/depended
- // on by some code.
- bool IsMethodBeingInspected(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_);
+ // Returns true if any locals have changed. This is used to prevent OSRing frames that have
+ // some locals changed.
+ bool HaveLocalsChanged() REQUIRES_SHARED(Locks::mutator_lock_);
void AddMethodInspectionCallback(MethodInspectionCallback* cb)
REQUIRES_SHARED(Locks::mutator_lock_);