diff options
Diffstat (limited to 'openjdkjvmti')
| -rw-r--r-- | openjdkjvmti/deopt_manager.cc | 79 | ||||
| -rw-r--r-- | openjdkjvmti/deopt_manager.h | 23 | ||||
| -rw-r--r-- | openjdkjvmti/ti_method.cc | 3 |
3 files changed, 73 insertions, 32 deletions
diff --git a/openjdkjvmti/deopt_manager.cc b/openjdkjvmti/deopt_manager.cc index 6d84ffa53f..a6f1207f34 100644 --- a/openjdkjvmti/deopt_manager.cc +++ b/openjdkjvmti/deopt_manager.cc @@ -53,20 +53,29 @@ 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) { return !manager_->MethodHasBreakpoints(method); } +bool JvmtiMethodInspectionCallback::MethodNeedsDebugVersion( + art::ArtMethod* method ATTRIBUTE_UNUSED) { + return true; +} + DeoptManager::DeoptManager() : deoptimization_status_lock_("JVMTI_DeoptimizationStatusLock", static_cast<art::LockLevel>( @@ -75,7 +84,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 +133,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 +167,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 +210,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 { diff --git a/openjdkjvmti/deopt_manager.h b/openjdkjvmti/deopt_manager.h index a495b6835c..6e991dee3d 100644 --- a/openjdkjvmti/deopt_manager.h +++ b/openjdkjvmti/deopt_manager.h @@ -32,6 +32,7 @@ #ifndef ART_OPENJDKJVMTI_DEOPT_MANAGER_H_ #define ART_OPENJDKJVMTI_DEOPT_MANAGER_H_ +#include <atomic> #include <unordered_map> #include "jni.h" @@ -62,6 +63,9 @@ struct JvmtiMethodInspectionCallback : public art::MethodInspectionCallback { bool IsMethodSafeToJit(art::ArtMethod* method) OVERRIDE REQUIRES_SHARED(art::Locks::mutator_lock_); + bool MethodNeedsDebugVersion(art::ArtMethod* method) + OVERRIDE REQUIRES_SHARED(art::Locks::mutator_lock_); + private: DeoptManager* manager_; }; @@ -107,9 +111,17 @@ class DeoptManager { static DeoptManager* Get(); + bool HaveLocalsChanged() const { + return set_local_variable_called_.load(); + } + + void SetLocalsUpdated() { + set_local_variable_called_.store(true); + } + private: bool MethodHasBreakpointsLocked(art::ArtMethod* method) - REQUIRES(deoptimization_status_lock_); + REQUIRES(breakpoint_status_lock_); // Wait until nothing is currently in the middle of deoptimizing/undeoptimizing something. This is // needed to ensure that everything is synchronized since threads need to drop the @@ -156,13 +168,20 @@ class DeoptManager { // Number of users of deoptimization there currently are. uint32_t deopter_count_ GUARDED_BY(deoptimization_status_lock_); + // A mutex that just protects the breakpoint-status map. This mutex should always be at the + // bottom of the lock hierarchy. Nothing more should be locked if we hold this. + art::Mutex breakpoint_status_lock_ ACQUIRED_BEFORE(art::Locks::abort_lock_); // A map from methods to the number of breakpoints in them from all envs. std::unordered_map<art::ArtMethod*, uint32_t> breakpoint_status_ - GUARDED_BY(deoptimization_status_lock_); + GUARDED_BY(breakpoint_status_lock_); // The MethodInspectionCallback we use to tell the runtime if we care about particular methods. JvmtiMethodInspectionCallback inspection_callback_; + // Set to true if anything calls SetLocalVariables on any thread since we need to be careful about + // OSR after this. + std::atomic<bool> set_local_variable_called_; + // Helper for setting up/tearing-down for deoptimization. friend class ScopedDeoptimizationContext; }; diff --git a/openjdkjvmti/ti_method.cc b/openjdkjvmti/ti_method.cc index bf2e6cd104..b83310dc85 100644 --- a/openjdkjvmti/ti_method.cc +++ b/openjdkjvmti/ti_method.cc @@ -915,6 +915,9 @@ jvmtiError MethodUtil::SetLocalVariableGeneric(jvmtiEnv* env ATTRIBUTE_UNUSED, if (depth < 0) { return ERR(ILLEGAL_ARGUMENT); } + // Make sure that we know not to do any OSR anymore. + // TODO We should really keep track of this at the Frame granularity. + DeoptManager::Get()->SetLocalsUpdated(); art::Thread* self = art::Thread::Current(); // Suspend JIT since it can get confused if we deoptimize methods getting jitted. art::jit::ScopedJitSuspend suspend_jit; |