From f3928794a10516e2ac0ffe2686a10891788d4b9c Mon Sep 17 00:00:00 2001 From: Sebastien Hertz Date: Mon, 17 Nov 2014 19:00:37 +0100 Subject: [PATCH] JDWP: only deoptimize when it is required We don't need to deoptimize anything when we forced the use of the interpreter (-Xint). In this case, no compiled code is executed (except native methods which are not concerned by deoptimization). Therefore we even don't need to enable/disable deoptimization support in instrumentation. We also don't need to deoptimize a method that hasn't been compiled. Since it will run with interpreter, there is no point deoptimizing it. However this method may be inlined in a compiled caller method so we still need to deoptimize everything in this case. This CL updates breakpoint support by storing the required kind of deoptimization for a particular method. There are 3 cases: - kNothing: the method does not require deoptimization. - kSelectiveDeoptimization: the method needs to be deoptimized. - kFullDeoptimization: we must deoptimize everythinig. When uninstalling a breakpoint, we need to do the reverse operation. Also fixes the SanityCheckExistingBreakpoints function to control breakpoints related to the given method only and adds extra verbose logs when choosing the appropriate deoptimization kind. Bug: 18407046 Change-Id: I5212c1fd2f72e06c79e7871db15696824d37dc0b --- runtime/debugger.cc | 145 +++++++++++++++++++++++++++++++-------------- runtime/debugger.h | 3 + runtime/jdwp/jdwp_event.cc | 4 ++ 3 files changed, 109 insertions(+), 43 deletions(-) diff --git a/runtime/debugger.cc b/runtime/debugger.cc index e2f6085ae..ef5db2d5e 100644 --- a/runtime/debugger.cc +++ b/runtime/debugger.cc @@ -183,16 +183,20 @@ class AllocRecord { class Breakpoint { public: - Breakpoint(mirror::ArtMethod* method, uint32_t dex_pc, bool need_full_deoptimization) + Breakpoint(mirror::ArtMethod* method, uint32_t dex_pc, + DeoptimizationRequest::Kind deoptimization_kind) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) - : method_(nullptr), dex_pc_(dex_pc), need_full_deoptimization_(need_full_deoptimization) { + : method_(nullptr), dex_pc_(dex_pc), deoptimization_kind_(deoptimization_kind) { + CHECK(deoptimization_kind_ == DeoptimizationRequest::kNothing || + deoptimization_kind_ == DeoptimizationRequest::kSelectiveDeoptimization || + deoptimization_kind_ == DeoptimizationRequest::kFullDeoptimization); ScopedObjectAccessUnchecked soa(Thread::Current()); method_ = soa.EncodeMethod(method); } Breakpoint(const Breakpoint& other) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) : method_(nullptr), dex_pc_(other.dex_pc_), - need_full_deoptimization_(other.need_full_deoptimization_) { + deoptimization_kind_(other.deoptimization_kind_) { ScopedObjectAccessUnchecked soa(Thread::Current()); method_ = soa.EncodeMethod(other.Method()); } @@ -206,8 +210,8 @@ class Breakpoint { return dex_pc_; } - bool NeedFullDeoptimization() const { - return need_full_deoptimization_; + DeoptimizationRequest::Kind GetDeoptimizationKind() const { + return deoptimization_kind_; } private: @@ -216,7 +220,7 @@ class Breakpoint { uint32_t dex_pc_; // Indicates whether breakpoint needs full deoptimization or selective deoptimization. - bool need_full_deoptimization_; + DeoptimizationRequest::Kind deoptimization_kind_; }; static std::ostream& operator<<(std::ostream& os, const Breakpoint& rhs) @@ -736,6 +740,12 @@ bool Dbg::IsDisposed() { return gDisposed; } +bool Dbg::RequiresDeoptimization() { + // We don't need deoptimization if everything runs with interpreter after + // enabling -Xint mode. + return !Runtime::Current()->GetInstrumentation()->IsForcedInterpretOnly(); +} + void Dbg::GoActive() { // Enable all debugging features, including scans for breakpoints. // This is a no-op if we're already active. @@ -768,7 +778,9 @@ void Dbg::GoActive() { Thread* self = Thread::Current(); ThreadState old_state = self->SetStateUnsafe(kRunnable); CHECK_NE(old_state, kRunnable); - runtime->GetInstrumentation()->EnableDeoptimization(); + if (RequiresDeoptimization()) { + runtime->GetInstrumentation()->EnableDeoptimization(); + } instrumentation_events_ = 0; gDebuggerActive = true; CHECK_EQ(self->SetStateUnsafe(old_state), kRunnable); @@ -806,7 +818,9 @@ void Dbg::Disconnected() { instrumentation_events_); instrumentation_events_ = 0; } - runtime->GetInstrumentation()->DisableDeoptimization(); + if (RequiresDeoptimization()) { + runtime->GetInstrumentation()->DisableDeoptimization(); + } gDebuggerActive = false; } gRegistry->Clear(); @@ -3035,9 +3049,11 @@ void Dbg::ProcessDeoptimizationRequest(const DeoptimizationRequest& request) { } void Dbg::DelayFullUndeoptimization() { - MutexLock mu(Thread::Current(), *Locks::deoptimization_lock_); - ++delayed_full_undeoptimization_count_; - DCHECK_LE(delayed_full_undeoptimization_count_, full_deoptimization_event_count_); + if (RequiresDeoptimization()) { + MutexLock mu(Thread::Current(), *Locks::deoptimization_lock_); + ++delayed_full_undeoptimization_count_; + DCHECK_LE(delayed_full_undeoptimization_count_, full_deoptimization_event_count_); + } } void Dbg::ProcessDelayedFullUndeoptimizations() { @@ -3196,64 +3212,101 @@ static const Breakpoint* FindFirstBreakpointForMethod(mirror::ArtMethod* m) } // Sanity checks all existing breakpoints on the same method. -static void SanityCheckExistingBreakpoints(mirror::ArtMethod* m, bool need_full_deoptimization) +static void SanityCheckExistingBreakpoints(mirror::ArtMethod* m, + DeoptimizationRequest::Kind deoptimization_kind) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_, Locks::breakpoint_lock_) { for (const Breakpoint& breakpoint : gBreakpoints) { - CHECK_EQ(need_full_deoptimization, breakpoint.NeedFullDeoptimization()); + if (breakpoint.Method() == m) { + CHECK_EQ(deoptimization_kind, breakpoint.GetDeoptimizationKind()); + } } - if (need_full_deoptimization) { + instrumentation::Instrumentation* instrumentation = Runtime::Current()->GetInstrumentation(); + if (deoptimization_kind == DeoptimizationRequest::kFullDeoptimization) { // We should have deoptimized everything but not "selectively" deoptimized this method. - CHECK(Runtime::Current()->GetInstrumentation()->AreAllMethodsDeoptimized()); - CHECK(!Runtime::Current()->GetInstrumentation()->IsDeoptimized(m)); - } else { + CHECK(instrumentation->AreAllMethodsDeoptimized()); + CHECK(!instrumentation->IsDeoptimized(m)); + } else if (deoptimization_kind == DeoptimizationRequest::kSelectiveDeoptimization) { // We should have "selectively" deoptimized this method. // Note: while we have not deoptimized everything for this method, we may have done it for // another event. - CHECK(Runtime::Current()->GetInstrumentation()->IsDeoptimized(m)); + CHECK(instrumentation->IsDeoptimized(m)); + } else { + // This method does not require deoptimization. + CHECK_EQ(deoptimization_kind, DeoptimizationRequest::kNothing); + CHECK(!instrumentation->IsDeoptimized(m)); } } -// Installs a breakpoint at the specified location. Also indicates through the deoptimization -// request if we need to deoptimize. -void Dbg::WatchLocation(const JDWP::JdwpLocation* location, DeoptimizationRequest* req) { - Thread* const self = Thread::Current(); - mirror::ArtMethod* m = FromMethodId(location->method_id); - DCHECK(m != nullptr) << "No method for method id " << location->method_id; - +static DeoptimizationRequest::Kind GetRequiredDeoptimizationKind(Thread* self, + mirror::ArtMethod* m) + SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { + if (!Dbg::RequiresDeoptimization()) { + // We already run in interpreter-only mode so we don't need to deoptimize anything. + VLOG(jdwp) << "No need for deoptimization when fully running with interpreter for method " + << PrettyMethod(m); + return DeoptimizationRequest::kNothing; + } const Breakpoint* existing_breakpoint; { ReaderMutexLock mu(self, *Locks::breakpoint_lock_); existing_breakpoint = FindFirstBreakpointForMethod(m); } - bool need_full_deoptimization; if (existing_breakpoint == nullptr) { // There is no breakpoint on this method yet: we need to deoptimize. If this method may be // inlined, we deoptimize everything; otherwise we deoptimize only this method. // Note: IsMethodPossiblyInlined goes into the method verifier and may cause thread suspension. // Therefore we must not hold any lock when we call it. - need_full_deoptimization = IsMethodPossiblyInlined(self, m); + bool need_full_deoptimization = IsMethodPossiblyInlined(self, m); if (need_full_deoptimization) { - req->SetKind(DeoptimizationRequest::kFullDeoptimization); - req->SetMethod(nullptr); + VLOG(jdwp) << "Need full deoptimization because of possible inlining of method " + << PrettyMethod(m); + return DeoptimizationRequest::kFullDeoptimization; } else { - req->SetKind(DeoptimizationRequest::kSelectiveDeoptimization); - req->SetMethod(m); + // We don't need to deoptimize if the method has not been compiled. + ClassLinker* const class_linker = Runtime::Current()->GetClassLinker(); + const bool is_compiled = class_linker->GetOatMethodQuickCodeFor(m) != nullptr; + if (is_compiled) { + VLOG(jdwp) << "Need selective deoptimization for compiled method " << PrettyMethod(m); + return DeoptimizationRequest::kSelectiveDeoptimization; + } else { + // Method is not compiled: we don't need to deoptimize. + VLOG(jdwp) << "No need for deoptimization for non-compiled method " << PrettyMethod(m); + return DeoptimizationRequest::kNothing; + } } } else { // There is at least one breakpoint for this method: we don't need to deoptimize. - req->SetKind(DeoptimizationRequest::kNothing); - req->SetMethod(nullptr); - - need_full_deoptimization = existing_breakpoint->NeedFullDeoptimization(); + // Let's check that all breakpoints are configured the same way for deoptimization. + VLOG(jdwp) << "Breakpoint already set: no deoptimization is required"; + DeoptimizationRequest::Kind deoptimization_kind = existing_breakpoint->GetDeoptimizationKind(); if (kIsDebugBuild) { ReaderMutexLock mu(self, *Locks::breakpoint_lock_); - SanityCheckExistingBreakpoints(m, need_full_deoptimization); + SanityCheckExistingBreakpoints(m, deoptimization_kind); } + return DeoptimizationRequest::kNothing; + } +} + +// Installs a breakpoint at the specified location. Also indicates through the deoptimization +// request if we need to deoptimize. +void Dbg::WatchLocation(const JDWP::JdwpLocation* location, DeoptimizationRequest* req) { + Thread* const self = Thread::Current(); + mirror::ArtMethod* m = FromMethodId(location->method_id); + DCHECK(m != nullptr) << "No method for method id " << location->method_id; + + const DeoptimizationRequest::Kind deoptimization_kind = GetRequiredDeoptimizationKind(self, m); + req->SetKind(deoptimization_kind); + if (deoptimization_kind == DeoptimizationRequest::kSelectiveDeoptimization) { + req->SetMethod(m); + } else { + CHECK(deoptimization_kind == DeoptimizationRequest::kNothing || + deoptimization_kind == DeoptimizationRequest::kFullDeoptimization); + req->SetMethod(nullptr); } { WriterMutexLock mu(self, *Locks::breakpoint_lock_); - gBreakpoints.push_back(Breakpoint(m, location->dex_pc, need_full_deoptimization)); + gBreakpoints.push_back(Breakpoint(m, location->dex_pc, deoptimization_kind)); VLOG(jdwp) << "Set breakpoint #" << (gBreakpoints.size() - 1) << ": " << gBreakpoints[gBreakpoints.size() - 1]; } @@ -3265,12 +3318,13 @@ void Dbg::UnwatchLocation(const JDWP::JdwpLocation* location, DeoptimizationRequ WriterMutexLock mu(Thread::Current(), *Locks::breakpoint_lock_); mirror::ArtMethod* m = FromMethodId(location->method_id); DCHECK(m != nullptr) << "No method for method id " << location->method_id; - bool need_full_deoptimization = false; + DeoptimizationRequest::Kind deoptimization_kind = DeoptimizationRequest::kNothing; for (size_t i = 0, e = gBreakpoints.size(); i < e; ++i) { if (gBreakpoints[i].DexPc() == location->dex_pc && gBreakpoints[i].Method() == m) { VLOG(jdwp) << "Removed breakpoint #" << i << ": " << gBreakpoints[i]; - need_full_deoptimization = gBreakpoints[i].NeedFullDeoptimization(); - DCHECK_NE(need_full_deoptimization, Runtime::Current()->GetInstrumentation()->IsDeoptimized(m)); + deoptimization_kind = gBreakpoints[i].GetDeoptimizationKind(); + DCHECK_EQ(deoptimization_kind == DeoptimizationRequest::kSelectiveDeoptimization, + Runtime::Current()->GetInstrumentation()->IsDeoptimized(m)); gBreakpoints.erase(gBreakpoints.begin() + i); break; } @@ -3278,21 +3332,26 @@ void Dbg::UnwatchLocation(const JDWP::JdwpLocation* location, DeoptimizationRequ const Breakpoint* const existing_breakpoint = FindFirstBreakpointForMethod(m); if (existing_breakpoint == nullptr) { // There is no more breakpoint on this method: we need to undeoptimize. - if (need_full_deoptimization) { + if (deoptimization_kind == DeoptimizationRequest::kFullDeoptimization) { // This method required full deoptimization: we need to undeoptimize everything. req->SetKind(DeoptimizationRequest::kFullUndeoptimization); req->SetMethod(nullptr); - } else { + } else if (deoptimization_kind == DeoptimizationRequest::kSelectiveDeoptimization) { // This method required selective deoptimization: we need to undeoptimize only that method. req->SetKind(DeoptimizationRequest::kSelectiveUndeoptimization); req->SetMethod(m); + } else { + // This method had no need for deoptimization: do nothing. + CHECK_EQ(deoptimization_kind, DeoptimizationRequest::kNothing); + req->SetKind(DeoptimizationRequest::kNothing); + req->SetMethod(nullptr); } } else { // There is at least one breakpoint for this method: we don't need to undeoptimize. req->SetKind(DeoptimizationRequest::kNothing); req->SetMethod(nullptr); if (kIsDebugBuild) { - SanityCheckExistingBreakpoints(m, need_full_deoptimization); + SanityCheckExistingBreakpoints(m, deoptimization_kind); } } } diff --git a/runtime/debugger.h b/runtime/debugger.h index 488ba7f72..92031634c 100644 --- a/runtime/debugger.h +++ b/runtime/debugger.h @@ -523,6 +523,9 @@ class Dbg { LOCKS_EXCLUDED(Locks::breakpoint_lock_) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); + // Indicates whether we need deoptimization for debugging. + static bool RequiresDeoptimization(); + // Records deoptimization request in the queue. static void RequestDeoptimization(const DeoptimizationRequest& req) LOCKS_EXCLUDED(Locks::deoptimization_lock_) diff --git a/runtime/jdwp/jdwp_event.cc b/runtime/jdwp/jdwp_event.cc index 44f713ca9..1e0a2d2c2 100644 --- a/runtime/jdwp/jdwp_event.cc +++ b/runtime/jdwp/jdwp_event.cc @@ -125,6 +125,10 @@ struct ModBasket { }; static bool NeedsFullDeoptimization(JdwpEventKind eventKind) { + if (!Dbg::RequiresDeoptimization()) { + // We don't need deoptimization for debugging. + return false; + } switch (eventKind) { case EK_METHOD_ENTRY: case EK_METHOD_EXIT: -- 2.11.0