From a76a6d4cb3dedec909bd927f20bc0ab23198a337 Mon Sep 17 00:00:00 2001 From: Sebastien Hertz Date: Thu, 20 Mar 2014 16:40:17 +0100 Subject: [PATCH] Support inlining with breakpoint When installing/uninstalling a breakpoint in a method, we fully or selectively deoptimize/undeoptimize depending on whether the method can be inlined. When the method can be inlined, it requires full deoptimization. Otherwise, it only requires selective deoptimization. We add sanity check to control we are in a consistent state each time we add or remove a breakpoint. We also add some comments to better describe the process of deoptimization for breakpoint. Bug: 12187616 Change-Id: Id15adc6e5e2fe783c83c925cbcd19ae02431b7e0 --- runtime/debugger.cc | 157 +++++++++++++++++++++++++++++++++------------- runtime/instrumentation.h | 9 ++- 2 files changed, 120 insertions(+), 46 deletions(-) diff --git a/runtime/debugger.cc b/runtime/debugger.cc index 20890f51a..9af9c7a00 100644 --- a/runtime/debugger.cc +++ b/runtime/debugger.cc @@ -38,6 +38,7 @@ #include "mirror/object_array-inl.h" #include "mirror/throwable.h" #include "object_utils.h" +#include "quick/inline_method_analyser.h" #include "reflection.h" #include "safe_map.h" #include "scoped_thread_state_change.h" @@ -48,6 +49,7 @@ #include "thread_list.h" #include "throw_location.h" #include "utf.h" +#include "verifier/method_verifier-inl.h" #include "well_known_classes.h" #ifdef HAVE_ANDROID_OS @@ -101,9 +103,15 @@ struct AllocRecord { }; struct Breakpoint { + // The location of this breakpoint. mirror::ArtMethod* method; uint32_t dex_pc; - Breakpoint(mirror::ArtMethod* method, uint32_t dex_pc) : method(method), dex_pc(dex_pc) {} + + // Indicates whether breakpoint needs full deoptimization or selective deoptimization. + bool need_full_deoptimization; + + Breakpoint(mirror::ArtMethod* method, uint32_t dex_pc, bool need_full_deoptimization) + : method(method), dex_pc(dex_pc), need_full_deoptimization(need_full_deoptimization) {} void VisitRoots(RootCallback* callback, void* arg) { if (method != nullptr) { @@ -2658,62 +2666,123 @@ void Dbg::ManageDeoptimization() { self->TransitionFromSuspendedToRunnable(); } -void Dbg::WatchLocation(const JDWP::JdwpLocation* location, DeoptimizationRequest* req) { - // TODO we don't need to deoptimize a method if it's not compiled since it already runs with the - // interpreter. - bool need_deoptimization = true; - mirror::ArtMethod* m = FromMethodId(location->method_id); - { - MutexLock mu(Thread::Current(), *Locks::breakpoint_lock_); +static bool IsMethodPossiblyInlined(Thread* self, mirror::ArtMethod* m) + SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { + MethodHelper mh(m); + const DexFile::CodeItem* code_item = mh.GetCodeItem(); + if (code_item == nullptr) { + // TODO We should not be asked to watch location in a native or abstract method so the code item + // should never be null. We could just check we never encounter this case. + return false; + } + SirtRef dex_cache(self, mh.GetDexCache()); + SirtRef class_loader(self, mh.GetClassLoader()); + verifier::MethodVerifier verifier(&mh.GetDexFile(), &dex_cache, &class_loader, + &mh.GetClassDef(), code_item, m->GetDexMethodIndex(), m, + m->GetAccessFlags(), false, true); + // Note: we don't need to verify the method. + return InlineMethodAnalyser::AnalyseMethodCode(&verifier, nullptr); +} - // If there is no breakpoint on this method yet, we need to deoptimize it. - for (const Breakpoint& breakpoint : gBreakpoints) { - if (breakpoint.method == m) { - // We already set a breakpoint on this method, hence we deoptimized it. - DCHECK(Runtime::Current()->GetInstrumentation()->IsDeoptimized(m)); - need_deoptimization = false; - break; - } +static const Breakpoint* FindFirstBreakpointForMethod(mirror::ArtMethod* m) + EXCLUSIVE_LOCKS_REQUIRED(Locks::breakpoint_lock_) { + for (const Breakpoint& breakpoint : gBreakpoints) { + if (breakpoint.method == m) { + return &breakpoint; } + } + return nullptr; +} - gBreakpoints.push_back(Breakpoint(m, location->dex_pc)); - VLOG(jdwp) << "Set breakpoint #" << (gBreakpoints.size() - 1) << ": " - << gBreakpoints[gBreakpoints.size() - 1]; +// Sanity checks all existing breakpoints on the same method. +static void SanityCheckExistingBreakpoints(mirror::ArtMethod* m, bool need_full_deoptimization) + EXCLUSIVE_LOCKS_REQUIRED(Locks::breakpoint_lock_) { + if (kIsDebugBuild) { + for (const Breakpoint& breakpoint : gBreakpoints) { + CHECK_EQ(need_full_deoptimization, breakpoint.need_full_deoptimization); + } + if (need_full_deoptimization) { + // We should have deoptimized everything but not "selectively" deoptimized this method. + CHECK(Runtime::Current()->GetInstrumentation()->AreAllMethodsDeoptimized()); + CHECK(!Runtime::Current()->GetInstrumentation()->IsDeoptimized(m)); + } else { + // 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)); + } } +} - if (need_deoptimization) { - req->kind = DeoptimizationRequest::kSelectiveDeoptimization; - req->method = 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; + + MutexLock mu(self, *Locks::breakpoint_lock_); + const Breakpoint* const 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. + need_full_deoptimization = IsMethodPossiblyInlined(self, m); + if (need_full_deoptimization) { + req->kind = DeoptimizationRequest::kFullDeoptimization; + req->method = nullptr; + } else { + req->kind = DeoptimizationRequest::kSelectiveDeoptimization; + req->method = m; + } + } else { + // There is at least one breakpoint for this method: we don't need to deoptimize. + req->kind = DeoptimizationRequest::kNothing; + req->method = nullptr; + + need_full_deoptimization = existing_breakpoint->need_full_deoptimization; + SanityCheckExistingBreakpoints(m, need_full_deoptimization); } + + gBreakpoints.push_back(Breakpoint(m, location->dex_pc, need_full_deoptimization)); + VLOG(jdwp) << "Set breakpoint #" << (gBreakpoints.size() - 1) << ": " + << gBreakpoints[gBreakpoints.size() - 1]; } +// Uninstalls a breakpoint at the specified location. Also indicates through the deoptimization +// request if we need to undeoptimize. void Dbg::UnwatchLocation(const JDWP::JdwpLocation* location, DeoptimizationRequest* req) { - bool can_undeoptimize = true; mirror::ArtMethod* m = FromMethodId(location->method_id); - DCHECK(Runtime::Current()->GetInstrumentation()->IsDeoptimized(m)); - { - MutexLock mu(Thread::Current(), *Locks::breakpoint_lock_); - for (size_t i = 0, e = gBreakpoints.size(); i < e; ++i) { - if (gBreakpoints[i].method == m && gBreakpoints[i].dex_pc == location->dex_pc) { - VLOG(jdwp) << "Removed breakpoint #" << i << ": " << gBreakpoints[i]; - gBreakpoints.erase(gBreakpoints.begin() + i); - break; - } - } + DCHECK(m != nullptr) << "No method for method id " << location->method_id; - // If there is no breakpoint on this method, we can undeoptimize it. - for (const Breakpoint& breakpoint : gBreakpoints) { - if (breakpoint.method == m) { - can_undeoptimize = false; - break; - } + MutexLock mu(Thread::Current(), *Locks::breakpoint_lock_); + bool need_full_deoptimization = false; + for (size_t i = 0, e = gBreakpoints.size(); i < e; ++i) { + if (gBreakpoints[i].method == m && gBreakpoints[i].dex_pc == location->dex_pc) { + VLOG(jdwp) << "Removed breakpoint #" << i << ": " << gBreakpoints[i]; + need_full_deoptimization = gBreakpoints[i].need_full_deoptimization; + DCHECK_NE(need_full_deoptimization, Runtime::Current()->GetInstrumentation()->IsDeoptimized(m)); + gBreakpoints.erase(gBreakpoints.begin() + i); + break; } } - - if (can_undeoptimize) { - // Request its undeoptimization. This will be done after updating the JDWP event list. - req->kind = DeoptimizationRequest::kSelectiveUndeoptimization; - req->method = m; + 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) { + // This method required full deoptimization: we need to undeoptimize everything. + req->kind = DeoptimizationRequest::kFullUndeoptimization; + req->method = nullptr; + } else { + // This method required selective deoptimization: we need to undeoptimize only that method. + req->kind = DeoptimizationRequest::kSelectiveUndeoptimization; + req->method = m; + } + } else { + // There is at least one breakpoint for this method: we don't need to undeoptimize. + req->kind = DeoptimizationRequest::kNothing; + req->method = nullptr; + SanityCheckExistingBreakpoints(m, need_full_deoptimization); } } diff --git a/runtime/instrumentation.h b/runtime/instrumentation.h index cf7271b3f..2a9c35f5a 100644 --- a/runtime/instrumentation.h +++ b/runtime/instrumentation.h @@ -115,10 +115,15 @@ class Instrumentation { LOCKS_EXCLUDED(Locks::thread_list_lock_, Locks::classlinker_classes_lock_); // Deoptimization. - void EnableDeoptimization() EXCLUSIVE_LOCKS_REQUIRED(Locks::mutator_lock_) + void EnableDeoptimization() + EXCLUSIVE_LOCKS_REQUIRED(Locks::mutator_lock_) LOCKS_EXCLUDED(deoptimized_methods_lock_); - void DisableDeoptimization() EXCLUSIVE_LOCKS_REQUIRED(Locks::mutator_lock_) + void DisableDeoptimization() + EXCLUSIVE_LOCKS_REQUIRED(Locks::mutator_lock_) LOCKS_EXCLUDED(deoptimized_methods_lock_); + bool AreAllMethodsDeoptimized() const { + return interpreter_stubs_installed_; + } bool ShouldNotifyMethodEnterExitEvents() const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); // Executes everything with interpreter. -- 2.11.0