Combine JDWP location events
The runtime now sends location events BREAKPOINT, SINGLE_STEP, METHOD_ENTRY,
METHOD_EXIT and METHOD_EXIT_WITH_RETURN_VALUE in the same JDWP event packet
when they relate to the same location.
We update the Dbg::UpdateDebugger method to take initial event flags and
returned value. It allows to call this method from DebugInstrumentationListener
so we can treat method entry/exit events with breakpoint and single-step.
In the interpreter, we ensure we do not call Instrumentation::DexPcMovedEvent
when Instrumentation::MethodEnterEvent has just been called or when we're about
to call Instrumentation::MethodExitEvent. This prevents from sending duplicated
events.
I measured the average performance impact on some benchmarks with a Nexus 4
without a debugger attached:
* 1%-2% for the computed-goto-based interpreter (default interpreter)
* 5%-10% for the switch-based interpreter.
This is mostly due to the test of the boolean flag for the method entry event.
Bug: https://code.google.com/p/android/issues/detail?id=68427
Bug: 11874828
Change-Id: Ic4ff61375ff6b4ed5825adeac09f61f97b4be619
diff --git a/runtime/debugger.cc b/runtime/debugger.cc
index 22a0e22..b7564e1 100644
--- a/runtime/debugger.cc
+++ b/runtime/debugger.cc
@@ -139,7 +139,7 @@
// TODO: post location events is a suspension point and native method entry stubs aren't.
return;
}
- Dbg::PostLocationEvent(method, 0, this_object, Dbg::kMethodEntry, nullptr);
+ Dbg::UpdateDebugger(thread, this_object, method, 0, Dbg::kMethodEntry, nullptr);
}
void MethodExited(Thread* thread, mirror::Object* this_object, mirror::ArtMethod* method,
@@ -149,7 +149,7 @@
// TODO: post location events is a suspension point and native method entry stubs aren't.
return;
}
- Dbg::PostLocationEvent(method, dex_pc, this_object, Dbg::kMethodExit, &return_value);
+ Dbg::UpdateDebugger(thread, this_object, method, dex_pc, Dbg::kMethodExit, &return_value);
}
void MethodUnwind(Thread* thread, mirror::Object* this_object, mirror::ArtMethod* method,
@@ -163,7 +163,7 @@
void DexPcMoved(Thread* thread, mirror::Object* this_object, mirror::ArtMethod* method,
uint32_t new_dex_pc)
OVERRIDE SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
- Dbg::UpdateDebugger(thread, this_object, method, new_dex_pc);
+ Dbg::UpdateDebugger(thread, this_object, method, new_dex_pc, 0, nullptr);
}
void FieldRead(Thread* thread, mirror::Object* this_object, mirror::ArtMethod* method,
@@ -2586,13 +2586,12 @@
}
void Dbg::UpdateDebugger(Thread* thread, mirror::Object* this_object,
- mirror::ArtMethod* m, uint32_t dex_pc) {
+ mirror::ArtMethod* m, uint32_t dex_pc,
+ int event_flags, const JValue* return_value) {
if (!IsDebuggerActive() || dex_pc == static_cast<uint32_t>(-2) /* fake method exit */) {
return;
}
- int event_flags = 0;
-
if (IsBreakpoint(m, dex_pc)) {
event_flags |= kBreakpoint;
}
@@ -2660,7 +2659,7 @@
// If there's something interesting going on, see if it matches one
// of the debugger filters.
if (event_flags != 0) {
- Dbg::PostLocationEvent(m, dex_pc, this_object, event_flags, nullptr);
+ Dbg::PostLocationEvent(m, dex_pc, this_object, event_flags, return_value);
}
}
diff --git a/runtime/debugger.h b/runtime/debugger.h
index bef708c..1ad1a00 100644
--- a/runtime/debugger.h
+++ b/runtime/debugger.h
@@ -417,10 +417,6 @@
kMethodEntry = 0x04,
kMethodExit = 0x08,
};
- static void PostLocationEvent(mirror::ArtMethod* method, int pcOffset,
- mirror::Object* thisPtr, int eventFlags,
- const JValue* return_value)
- SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
static void PostFieldAccessEvent(mirror::ArtMethod* m, int dex_pc, mirror::Object* this_object,
mirror::ArtField* f)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
@@ -439,7 +435,8 @@
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
static void UpdateDebugger(Thread* thread, mirror::Object* this_object,
- mirror::ArtMethod* method, uint32_t new_dex_pc)
+ mirror::ArtMethod* method, uint32_t new_dex_pc,
+ int event_flags, const JValue* return_value)
LOCKS_EXCLUDED(Locks::breakpoint_lock_)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
@@ -561,6 +558,11 @@
static void PostThreadStartOrStop(Thread*, uint32_t)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
+ static void PostLocationEvent(mirror::ArtMethod* method, int pcOffset,
+ mirror::Object* thisPtr, int eventFlags,
+ const JValue* return_value)
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
+
static JDWP::ObjectId GetThisObjectIdForEvent(mirror::Object* this_object)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
diff --git a/runtime/interpreter/interpreter_goto_table_impl.cc b/runtime/interpreter/interpreter_goto_table_impl.cc
index e425e91..74f386c 100644
--- a/runtime/interpreter/interpreter_goto_table_impl.cc
+++ b/runtime/interpreter/interpreter_goto_table_impl.cc
@@ -145,12 +145,14 @@
const Instruction* inst = Instruction::At(code_item->insns_ + dex_pc);
uint16_t inst_data;
const void* const* currentHandlersTable;
+ bool notified_method_entry_event = false;
UPDATE_HANDLER_TABLE();
if (LIKELY(dex_pc == 0)) { // We are entering the method as opposed to deoptimizing..
instrumentation::Instrumentation* instrumentation = Runtime::Current()->GetInstrumentation();
if (UNLIKELY(instrumentation->HasMethodEntryListeners())) {
instrumentation->MethodEnterEvent(self, shadow_frame.GetThisObject(code_item->ins_size_),
shadow_frame.GetMethod(), 0);
+ notified_method_entry_event = true;
}
}
@@ -2384,16 +2386,29 @@
}
}
- // Create alternative instruction handlers dedicated to instrumentation.
-#define INSTRUMENTATION_INSTRUCTION_HANDLER(o, code, n, f, r, i, a, v) \
- alt_op_##code: { \
- instrumentation::Instrumentation* instrumentation = Runtime::Current()->GetInstrumentation(); \
- if (UNLIKELY(instrumentation->HasDexPcListeners())) { \
- instrumentation->DexPcMovedEvent(self, shadow_frame.GetThisObject(code_item->ins_size_), \
- shadow_frame.GetMethod(), dex_pc); \
- } \
- UPDATE_HANDLER_TABLE(); \
- goto *handlersTable[instrumentation::kMainHandlerTable][Instruction::code]; \
+// Create alternative instruction handlers dedicated to instrumentation.
+// Return instructions must not call Instrumentation::DexPcMovedEvent since they already call
+// Instrumentation::MethodExited. This is to avoid posting debugger events twice for this location.
+#define INSTRUMENTATION_INSTRUCTION_HANDLER(o, code, n, f, r, i, a, v) \
+ alt_op_##code: { \
+ if (Instruction::code != Instruction::RETURN_VOID && \
+ Instruction::code != Instruction::RETURN_VOID_BARRIER && \
+ Instruction::code != Instruction::RETURN && \
+ Instruction::code != Instruction::RETURN_WIDE && \
+ Instruction::code != Instruction::RETURN_OBJECT) { \
+ if (LIKELY(!notified_method_entry_event)) { \
+ Runtime* runtime = Runtime::Current(); \
+ const instrumentation::Instrumentation* instrumentation = runtime->GetInstrumentation(); \
+ if (UNLIKELY(instrumentation->HasDexPcListeners())) { \
+ Object* this_object = shadow_frame.GetThisObject(code_item->ins_size_); \
+ instrumentation->DexPcMovedEvent(self, this_object, shadow_frame.GetMethod(), dex_pc); \
+ } \
+ } else { \
+ notified_method_entry_event = false; \
+ } \
+ } \
+ UPDATE_HANDLER_TABLE(); \
+ goto *handlersTable[instrumentation::kMainHandlerTable][Instruction::code]; \
}
#include "dex_instruction_list.h"
DEX_INSTRUCTION_LIST(INSTRUMENTATION_INSTRUCTION_HANDLER)
diff --git a/runtime/interpreter/interpreter_switch_impl.cc b/runtime/interpreter/interpreter_switch_impl.cc
index 9c13973..97c216d 100644
--- a/runtime/interpreter/interpreter_switch_impl.cc
+++ b/runtime/interpreter/interpreter_switch_impl.cc
@@ -48,11 +48,20 @@
} while (false)
// Code to run before each dex instruction.
-#define PREAMBLE()
+#define PREAMBLE() \
+ do { \
+ DCHECK(!inst->IsReturn()); \
+ if (UNLIKELY(notified_method_entry_event)) { \
+ notified_method_entry_event = false; \
+ } else if (UNLIKELY(instrumentation->HasDexPcListeners())) { \
+ instrumentation->DexPcMovedEvent(self, shadow_frame.GetThisObject(code_item->ins_size_), \
+ shadow_frame.GetMethod(), dex_pc); \
+ } \
+ } while (false)
template<bool do_access_check, bool transaction_active>
JValue ExecuteSwitchImpl(Thread* self, MethodHelper& mh, const DexFile::CodeItem* code_item,
- ShadowFrame& shadow_frame, JValue result_register) {
+ ShadowFrame& shadow_frame, JValue result_register) {
bool do_assignability_check = do_access_check;
if (UNLIKELY(!shadow_frame.HasReferenceArray())) {
LOG(FATAL) << "Invalid shadow frame for interpreter use";
@@ -61,11 +70,13 @@
self->VerifyStack();
uint32_t dex_pc = shadow_frame.GetDexPC();
+ bool notified_method_entry_event = false;
const instrumentation::Instrumentation* const instrumentation = Runtime::Current()->GetInstrumentation();
if (LIKELY(dex_pc == 0)) { // We are entering the method as opposed to deoptimizing..
if (UNLIKELY(instrumentation->HasMethodEntryListeners())) {
instrumentation->MethodEnterEvent(self, shadow_frame.GetThisObject(code_item->ins_size_),
shadow_frame.GetMethod(), 0);
+ notified_method_entry_event = true;
}
}
const uint16_t* const insns = code_item->insns_;
@@ -74,10 +85,6 @@
while (true) {
dex_pc = inst->GetDexPc(insns);
shadow_frame.SetDexPC(dex_pc);
- if (UNLIKELY(instrumentation->HasDexPcListeners())) {
- instrumentation->DexPcMovedEvent(self, shadow_frame.GetThisObject(code_item->ins_size_),
- shadow_frame.GetMethod(), dex_pc);
- }
TraceExecution(shadow_frame, inst, dex_pc, mh);
inst_data = inst->Fetch16(0);
switch (inst->Opcode(inst_data)) {
@@ -163,7 +170,6 @@
break;
}
case Instruction::RETURN_VOID: {
- PREAMBLE();
JValue result;
if (do_access_check) {
// If access checks are required then the dex-to-dex compiler and analysis of
@@ -182,7 +188,6 @@
return result;
}
case Instruction::RETURN_VOID_BARRIER: {
- PREAMBLE();
QuasiAtomic::MembarStoreLoad();
JValue result;
if (UNLIKELY(self->TestAllFlags())) {
@@ -196,7 +201,6 @@
return result;
}
case Instruction::RETURN: {
- PREAMBLE();
JValue result;
result.SetJ(0);
result.SetI(shadow_frame.GetVReg(inst->VRegA_11x(inst_data)));
@@ -211,7 +215,6 @@
return result;
}
case Instruction::RETURN_WIDE: {
- PREAMBLE();
JValue result;
result.SetJ(shadow_frame.GetVRegLong(inst->VRegA_11x(inst_data)));
if (UNLIKELY(self->TestAllFlags())) {
@@ -225,7 +228,6 @@
return result;
}
case Instruction::RETURN_OBJECT: {
- PREAMBLE();
JValue result;
if (UNLIKELY(self->TestAllFlags())) {
CheckSuspend(self);