1 files changed, 44 insertions, 30 deletions

diff --git a/openjdkjvmti/deopt_manager.cc b/openjdkjvmti/deopt_manager.cc
index 6d84ffa53f..380d95c545 100644
--- a/openjdkjvmti/deopt_manager.cc
+++ b/openjdkjvmti/deopt_manager.cc
@@ -53,14 +53,18 @@
 namespace openjdkjvmti {
 
 // TODO We should make this much more selective in the future so we only return true when we
-// actually care about the method (i.e. had locals changed, have breakpoints, etc.). For now though
-// we can just assume that we care we are loaded at all.
-//
-// Even if we don't keep track of this at the method level we might want to keep track of it at the
-// level of enabled capabilities.
-bool JvmtiMethodInspectionCallback::IsMethodBeingInspected(
-    art::ArtMethod* method ATTRIBUTE_UNUSED) {
-  return true;
+// actually care about the method at this time (ie active frames had locals changed). For now we
+// just assume that if anything has changed any frame's locals we care about all methods. If nothing
+// has we only care about methods with active breakpoints on them. In the future we should probably
+// rewrite all of this to instead do this at the ShadowFrame or thread granularity.
+bool JvmtiMethodInspectionCallback::IsMethodBeingInspected(art::ArtMethod* method) {
+  // Non-java-debuggable runtimes we need to assume that any method might not be debuggable and
+  // therefore potentially being inspected (due to inlines). If we are debuggable we rely hard on
+  // inlining not being done since we don't keep track of which methods get inlined where and simply
+  // look to see if the method is breakpointed.
+  return !art::Runtime::Current()->IsJavaDebuggable() ||
+      manager_->HaveLocalsChanged() ||
+      manager_->MethodHasBreakpoints(method);
 }
 
 bool JvmtiMethodInspectionCallback::IsMethodSafeToJit(art::ArtMethod* method) {
@@ -75,7 +79,10 @@ DeoptManager::DeoptManager()
     performing_deoptimization_(false),
     global_deopt_count_(0),
     deopter_count_(0),
-    inspection_callback_(this) { }
+    breakpoint_status_lock_("JVMTI_BreakpointStatusLock",
+                            static_cast<art::LockLevel>(art::LockLevel::kAbortLock + 1)),
+    inspection_callback_(this),
+    set_local_variable_called_(false) { }
 
 void DeoptManager::Setup() {
   art::ScopedThreadStateChange stsc(art::Thread::Current(),
@@ -121,14 +128,11 @@ void DeoptManager::FinishSetup() {
 }
 
 bool DeoptManager::MethodHasBreakpoints(art::ArtMethod* method) {
-  art::MutexLock lk(art::Thread::Current(), deoptimization_status_lock_);
+  art::MutexLock lk(art::Thread::Current(), breakpoint_status_lock_);
   return MethodHasBreakpointsLocked(method);
 }
 
 bool DeoptManager::MethodHasBreakpointsLocked(art::ArtMethod* method) {
-  if (deopter_count_ == 0) {
-    return false;
-  }
   auto elem = breakpoint_status_.find(method);
   return elem != breakpoint_status_.end() && elem->second != 0;
 }
@@ -158,18 +162,23 @@ void DeoptManager::AddMethodBreakpoint(art::ArtMethod* method) {
 
   art::ScopedThreadSuspension sts(self, art::kSuspended);
   deoptimization_status_lock_.ExclusiveLock(self);
-
-  DCHECK_GT(deopter_count_, 0u) << "unexpected deotpimization request";
-
-  if (MethodHasBreakpointsLocked(method)) {
-    // Don't need to do anything extra.
-    breakpoint_status_[method]++;
-    // Another thread might be deoptimizing the very method we just added new breakpoints for. Wait
-    // for any deopts to finish before moving on.
-    WaitForDeoptimizationToFinish(self);
-    return;
+  {
+    breakpoint_status_lock_.ExclusiveLock(self);
+
+    DCHECK_GT(deopter_count_, 0u) << "unexpected deotpimization request";
+
+    if (MethodHasBreakpointsLocked(method)) {
+      // Don't need to do anything extra.
+      breakpoint_status_[method]++;
+      // Another thread might be deoptimizing the very method we just added new breakpoints for.
+      // Wait for any deopts to finish before moving on.
+      breakpoint_status_lock_.ExclusiveUnlock(self);
+      WaitForDeoptimizationToFinish(self);
+      return;
+    }
+    breakpoint_status_[method] = 1;
+    breakpoint_status_lock_.ExclusiveUnlock(self);
   }
-  breakpoint_status_[method] = 1;
   auto instrumentation = art::Runtime::Current()->GetInstrumentation();
   if (instrumentation->IsForcedInterpretOnly()) {
     // We are already interpreting everything so no need to do anything.
@@ -196,17 +205,22 @@ void DeoptManager::RemoveMethodBreakpoint(art::ArtMethod* method) {
   // need but since that is very heavy we will instead just use a condition variable to make sure we
   // don't race with ourselves.
   deoptimization_status_lock_.ExclusiveLock(self);
-
-  DCHECK_GT(deopter_count_, 0u) << "unexpected deotpimization request";
-  DCHECK(MethodHasBreakpointsLocked(method)) << "Breakpoint on a method was removed without "
-                                             << "breakpoints present!";
+  bool is_last_breakpoint;
+  {
+    art::MutexLock mu(self, breakpoint_status_lock_);
+
+    DCHECK_GT(deopter_count_, 0u) << "unexpected deotpimization request";
+    DCHECK(MethodHasBreakpointsLocked(method)) << "Breakpoint on a method was removed without "
+                                              << "breakpoints present!";
+    breakpoint_status_[method] -= 1;
+    is_last_breakpoint = (breakpoint_status_[method] == 0);
+  }
   auto instrumentation = art::Runtime::Current()->GetInstrumentation();
-  breakpoint_status_[method] -= 1;
   if (UNLIKELY(instrumentation->IsForcedInterpretOnly())) {
     // We don't need to do anything since we are interpreting everything anyway.
     deoptimization_status_lock_.ExclusiveUnlock(self);
     return;
-  } else if (breakpoint_status_[method] == 0) {
+  } else if (is_last_breakpoint) {
     if (UNLIKELY(is_default)) {
       RemoveDeoptimizeAllMethodsLocked(self);
     } else {