OSDN Git Service

JDWP: only deoptimize when it is required
authorSebastien Hertz <shertz@google.com>
Mon, 17 Nov 2014 18:00:37 +0000 (19:00 +0100)
committerSebastien Hertz <shertz@google.com>
Wed, 19 Nov 2014 13:35:28 +0000 (14:35 +0100)
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
runtime/debugger.h
runtime/jdwp/jdwp_event.cc

index e2f6085..ef5db2d 100644 (file)
@@ -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);
     }
   }
 }
index 488ba7f..9203163 100644 (file)
@@ -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_)
index 44f713c..1e0a2d2 100644 (file)
@@ -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: