Report DexPcMoved events only when required
We used to report DexPCMove events unconditionally when running in
interpreter. This Cl makes it so we only report if
1. The method is deoptimized for breakpoints
2. The thread is deoptimized for single stepping
3. Interpreter stubs installed. When interpreter stubs are installed no
additional data is available on whether we are single stepping / if
method has breakpoints etc., so we cannot optimize this further.
Bug: 206029744
Change-Id: Iaf059bc2ed74ca5174d445d46f3dc0d4bb0106b9
diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
index 5f8b6da..b7f60f1 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -280,6 +280,10 @@
}
}
+bool Instrumentation::NeedsDexPcEvents(ArtMethod* method, Thread* thread) {
+ return (InterpretOnly(method) || thread->IsForceInterpreter()) && HasDexPcListeners();
+}
+
bool Instrumentation::InterpretOnly(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_) {
if (method->IsNative()) {
return false;
@@ -476,6 +480,14 @@
return true; // Ignore upcalls and runtime methods.
}
if (GetCurrentQuickFrame() == nullptr) {
+ // Since we are updating the instrumentation related information we have to recalculate
+ // NeedsDexPcEvents. For example, when a new method or thread is deoptimized / interpreter
+ // stubs are installed the NeedsDexPcEvents could change for the shadow frames on the stack.
+ // If we don't update it here we would miss reporting dex pc events which is incorrect.
+ ShadowFrame* shadow_frame = GetCurrentShadowFrame();
+ DCHECK(shadow_frame != nullptr);
+ shadow_frame->SetNotifyDexPcMoveEvents(
+ Runtime::Current()->GetInstrumentation()->NeedsDexPcEvents(GetMethod(), GetThread()));
if (kVerboseInstrumentation) {
LOG(INFO) << "Pushing shadow frame method " << m->PrettyMethod();
}
@@ -589,6 +601,34 @@
thread->VerifyStack();
}
+void UpdateNeedsDexPcEventsOnStack(Thread* thread) REQUIRES(Locks::mutator_lock_) {
+ Locks::mutator_lock_->AssertExclusiveHeld(Thread::Current());
+
+ struct InstallStackVisitor final : public StackVisitor {
+ InstallStackVisitor(Thread* thread_in, Context* context)
+ : StackVisitor(thread_in, context, kInstrumentationStackWalk) {}
+
+ bool VisitFrame() override REQUIRES_SHARED(Locks::mutator_lock_) {
+ ShadowFrame* shadow_frame = GetCurrentShadowFrame();
+ if (shadow_frame != nullptr) {
+ shadow_frame->SetNotifyDexPcMoveEvents(
+ Runtime::Current()->GetInstrumentation()->NeedsDexPcEvents(GetMethod(), GetThread()));
+ }
+ return true;
+ }
+ };
+
+ if (kVerboseInstrumentation) {
+ std::string thread_name;
+ thread->GetThreadName(thread_name);
+ LOG(INFO) << "Updating DexPcMoveEvents on shadow frames on stack " << thread_name;
+ }
+
+ std::unique_ptr<Context> context(Context::Create());
+ InstallStackVisitor visitor(thread, context.get());
+ visitor.WalkStack(true);
+}
+
void Instrumentation::InstrumentThreadStack(Thread* thread, bool force_deopt) {
instrumentation_stubs_installed_ = true;
InstrumentationInstallStack(thread, this, force_deopt);
@@ -782,6 +822,12 @@
exception_handled_listeners_,
listener,
&have_exception_handled_listeners_);
+ if (HasEvent(kDexPcMoved, events)) {
+ MutexLock mu(Thread::Current(), *Locks::thread_list_lock_);
+ for (Thread* thread : Runtime::Current()->GetThreadList()->GetList()) {
+ UpdateNeedsDexPcEventsOnStack(thread);
+ }
+ }
}
static void PotentiallyRemoveListenerFrom(Instrumentation::InstrumentationEvent event,
@@ -863,6 +909,12 @@
exception_handled_listeners_,
listener,
&have_exception_handled_listeners_);
+ if (HasEvent(kDexPcMoved, events)) {
+ MutexLock mu(Thread::Current(), *Locks::thread_list_lock_);
+ for (Thread* thread : Runtime::Current()->GetThreadList()->GetList()) {
+ UpdateNeedsDexPcEventsOnStack(thread);
+ }
+ }
}
Instrumentation::InstrumentationLevel Instrumentation::GetCurrentInstrumentationLevel() const {
diff --git a/runtime/instrumentation.h b/runtime/instrumentation.h
index 9d0a33b..ab8bee7 100644
--- a/runtime/instrumentation.h
+++ b/runtime/instrumentation.h
@@ -375,6 +375,20 @@
return have_exception_handled_listeners_;
}
+ // Returns if dex pc events need to be reported for the specified method.
+ // These events are reported when DexPCListeners are installed and at least one of the
+ // following conditions hold:
+ // 1. The method is deoptimized. This is done when there is a breakpoint on method.
+ // 2. When the thread is deoptimized. This is used when single stepping a single thread.
+ // 3. When interpreter stubs are installed. In this case no additional information is maintained
+ // about which methods need dex pc move events. This is usually used for features which need
+ // them for several methods across threads or need expensive processing. So it is OK to not
+ // further optimize this case.
+ // DexPCListeners are installed when there is a breakpoint on any method / single stepping
+ // on any of thread. These are removed when the last breakpoint was removed. See AddListener and
+ // RemoveListener for more details.
+ bool NeedsDexPcEvents(ArtMethod* method, Thread* thread) REQUIRES_SHARED(Locks::mutator_lock_);
+
bool NeedsSlowInterpreterForListeners() const REQUIRES_SHARED(Locks::mutator_lock_) {
return have_field_read_listeners_ ||
have_field_write_listeners_ ||
diff --git a/runtime/interpreter/interpreter.cc b/runtime/interpreter/interpreter.cc
index 8ceade8..7a4ff5f 100644
--- a/runtime/interpreter/interpreter.cc
+++ b/runtime/interpreter/interpreter.cc
@@ -266,6 +266,13 @@
DCHECK(!shadow_frame.GetMethod()->IsAbstract());
DCHECK(!shadow_frame.GetMethod()->IsNative());
+ // We cache the result of NeedsDexPcEvents in the shadow frame so we don't need to call
+ // NeedsDexPcEvents on every instruction for better performance. NeedsDexPcEvents only gets
+ // updated asynchronoulsy in a SuspendAll scope and any existing shadow frames are updated with
+ // new value. So it is safe to cache it here.
+ shadow_frame.SetNotifyDexPcMoveEvents(
+ Runtime::Current()->GetInstrumentation()->NeedsDexPcEvents(shadow_frame.GetMethod(), self));
+
if (LIKELY(!from_deoptimize)) { // Entering the method, but not via deoptimization.
if (kIsDebugBuild) {
CHECK_EQ(shadow_frame.GetDexPC(), 0u);
diff --git a/runtime/interpreter/interpreter_switch_impl-inl.h b/runtime/interpreter/interpreter_switch_impl-inl.h
index 5e744dc..62bf7d8 100644
--- a/runtime/interpreter/interpreter_switch_impl-inl.h
+++ b/runtime/interpreter/interpreter_switch_impl-inl.h
@@ -144,7 +144,7 @@
if (!CheckForceReturn()) {
return false;
}
- if (UNLIKELY(Instrumentation()->HasDexPcListeners())) {
+ if (UNLIKELY(shadow_frame_.GetNotifyDexPcMoveEvents())) {
uint8_t opcode = inst_->Opcode(inst_data_);
bool is_move_result_object = (opcode == Instruction::MOVE_RESULT_OBJECT);
JValue* save_ref = is_move_result_object ? &ctx_->result_register : nullptr;
diff --git a/runtime/interpreter/shadow_frame.h b/runtime/interpreter/shadow_frame.h
index 67f6759..be93dfa 100644
--- a/runtime/interpreter/shadow_frame.h
+++ b/runtime/interpreter/shadow_frame.h
@@ -54,7 +54,7 @@
// We have been requested to notify when this frame gets popped.
kNotifyFramePop = 1 << 0,
// We have been asked to pop this frame off the stack as soon as possible.
- kForcePopFrame = 1 << 1,
+ kForcePopFrame = 1 << 1,
// We have been asked to re-execute the last instruction.
kForceRetryInst = 1 << 2,
// Mark that we expect the next frame to retry the last instruction (used by instrumentation and
@@ -62,6 +62,9 @@
kSkipMethodExitEvents = 1 << 3,
// Used to suppress exception events caused by other instrumentation events.
kSkipNextExceptionEvent = 1 << 4,
+ // Used to specify if DexPCMoveEvents have to be reported. These events will
+ // only be reported if the method has a breakpoint set.
+ kNotifyDexPcMoveEvents = 1 << 5,
};
public:
@@ -373,6 +376,14 @@
UpdateFrameFlag(enable, FrameFlags::kSkipNextExceptionEvent);
}
+ bool GetNotifyDexPcMoveEvents() const {
+ return GetFrameFlag(FrameFlags::kNotifyDexPcMoveEvents);
+ }
+
+ void SetNotifyDexPcMoveEvents(bool enable) {
+ UpdateFrameFlag(enable, FrameFlags::kNotifyDexPcMoveEvents);
+ }
+
void CheckConsistentVRegs() const {
if (kIsDebugBuild) {
// A shadow frame visible to GC requires the following rule: for a given vreg,