Reland "Remove Instrumentation::can_use_instrumentation_trampolines_."
This reverts commit 2b3cea96265b033c8810f6193350c39613fc230b.
Reason for revert: https://android-review.googlesource.com/c/platform/art/+/1916477 is in.
Change-Id: I9d217333e3c02d4fa99df009477773949eed4f99
diff --git a/openjdkjvmti/deopt_manager.cc b/openjdkjvmti/deopt_manager.cc
index f30a21d..6377050 100644
--- a/openjdkjvmti/deopt_manager.cc
+++ b/openjdkjvmti/deopt_manager.cc
@@ -450,6 +450,8 @@
return OK;
}
+static constexpr const char* kInstrumentationKey = "JVMTI_DeoptRequester";
+
void DeoptManager::RemoveDeoptimizationRequester() {
art::Thread* self = art::Thread::Current();
art::ScopedThreadStateChange sts(self, art::ThreadState::kSuspended);
@@ -458,8 +460,7 @@
deopter_count_--;
if (deopter_count_ == 0) {
ScopedDeoptimizationContext sdc(self, this);
- // TODO Give this a real key.
- art::Runtime::Current()->GetInstrumentation()->DisableDeoptimization("");
+ art::Runtime::Current()->GetInstrumentation()->DisableDeoptimization(kInstrumentationKey);
return;
} else {
deoptimization_status_lock_.ExclusiveUnlock(self);
@@ -478,7 +479,7 @@
// Enable deoptimization
instrumentation->EnableDeoptimization();
// Tell instrumentation we will be deopting single threads.
- instrumentation->EnableSingleThreadDeopt();
+ instrumentation->EnableSingleThreadDeopt(kInstrumentationKey);
} else {
deoptimization_status_lock_.ExclusiveUnlock(self);
}
diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
index 7449408..99f8226 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -185,8 +185,7 @@
deoptimization_enabled_(false),
interpreter_handler_table_(kMainHandlerTable),
quick_alloc_entry_points_instrumentation_counter_(0),
- alloc_entrypoints_instrumented_(false),
- can_use_instrumentation_trampolines_(true) {
+ alloc_entrypoints_instrumented_(false) {
}
void Instrumentation::InstallStubsForClass(ObjPtr<mirror::Class> klass) {
@@ -760,19 +759,6 @@
return GetCurrentInstrumentationLevel() != new_level;
}
-void Instrumentation::UpdateInstrumentationLevels(InstrumentationLevel level) {
- if (level == InstrumentationLevel::kInstrumentWithInterpreter) {
- can_use_instrumentation_trampolines_ = false;
- }
- if (UNLIKELY(!can_use_instrumentation_trampolines_)) {
- for (auto& p : requested_instrumentation_levels_) {
- if (p.second == InstrumentationLevel::kInstrumentWithInstrumentationStubs) {
- p.second = InstrumentationLevel::kInstrumentWithInterpreter;
- }
- }
- }
-}
-
void Instrumentation::ConfigureStubs(const char* key, InstrumentationLevel desired_level) {
// Store the instrumentation level for this key or remove it.
if (desired_level == InstrumentationLevel::kInstrumentNothing) {
@@ -783,15 +769,12 @@
requested_instrumentation_levels_.Overwrite(key, desired_level);
}
- UpdateInstrumentationLevels(desired_level);
UpdateStubs();
}
-void Instrumentation::EnableSingleThreadDeopt() {
+void Instrumentation::EnableSingleThreadDeopt(const char* key) {
// Single-thread deopt only uses interpreter.
- can_use_instrumentation_trampolines_ = false;
- UpdateInstrumentationLevels(InstrumentationLevel::kInstrumentWithInterpreter);
- UpdateStubs();
+ ConfigureStubs(key, InstrumentationLevel::kInstrumentWithInterpreter);
}
void Instrumentation::UpdateInstrumentationLevel(InstrumentationLevel requested_level) {
@@ -805,11 +788,6 @@
requested_level = std::max(requested_level, v.second);
}
- DCHECK(can_use_instrumentation_trampolines_ ||
- requested_level != InstrumentationLevel::kInstrumentWithInstrumentationStubs)
- << "Use trampolines: " << can_use_instrumentation_trampolines_ << " level "
- << requested_level;
-
if (!RequiresInstrumentationInstallation(requested_level)) {
// We're already set.
return;
diff --git a/runtime/instrumentation.h b/runtime/instrumentation.h
index a505ce3..1f7ee7f 100644
--- a/runtime/instrumentation.h
+++ b/runtime/instrumentation.h
@@ -545,7 +545,7 @@
REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(!GetDeoptimizedMethodsLock());
// Sets up instrumentation to allow single thread deoptimization using ForceInterpreterCount.
- void EnableSingleThreadDeopt()
+ void EnableSingleThreadDeopt(const char* key)
REQUIRES(Locks::mutator_lock_, Roles::uninterruptible_)
REQUIRES(!Locks::thread_list_lock_,
!Locks::classlinker_classes_lock_,
@@ -603,11 +603,6 @@
REQUIRES(!GetDeoptimizedMethodsLock(),
!Locks::thread_list_lock_,
!Locks::classlinker_classes_lock_);
- void UpdateInstrumentationLevels(InstrumentationLevel level)
- REQUIRES(Locks::mutator_lock_, Roles::uninterruptible_)
- REQUIRES(!GetDeoptimizedMethodsLock(),
- !Locks::thread_list_lock_,
- !Locks::classlinker_classes_lock_);
void UpdateInterpreterHandlerTable() REQUIRES(Locks::mutator_lock_) {
/*
@@ -771,11 +766,6 @@
// alloc_entrypoints_instrumented_ change during suspend points.
bool alloc_entrypoints_instrumented_;
- // If we can use instrumentation trampolines. After the first time we instrument something with
- // the interpreter we can no longer use trampolines because it can lead to stack corruption.
- // TODO Figure out a way to remove the need for this.
- bool can_use_instrumentation_trampolines_;
-
friend class InstrumentationTest; // For GetCurrentInstrumentationLevel and ConfigureStubs.
friend class InstrumentationStackPopper; // For popping instrumentation frames.
friend void InstrumentationInstallStack(Thread*, void*, bool);
diff --git a/runtime/instrumentation_test.cc b/runtime/instrumentation_test.cc
index cfca51e..d3fa5c8 100644
--- a/runtime/instrumentation_test.cc
+++ b/runtime/instrumentation_test.cc
@@ -837,8 +837,7 @@
// Configure stubs with instrumentation stubs.
CheckConfigureStubs(kClientOneKey,
Instrumentation::InstrumentationLevel::kInstrumentWithInstrumentationStubs);
- // Make sure we are still interpreter since going from interpreter->instrumentation is dangerous.
- CHECK_INSTRUMENTATION(Instrumentation::InstrumentationLevel::kInstrumentWithInterpreter,
+ CHECK_INSTRUMENTATION(Instrumentation::InstrumentationLevel::kInstrumentWithInstrumentationStubs,
1U);
// Check we can disable instrumentation.
@@ -864,7 +863,7 @@
// Configure stubs with instrumentation stubs again.
CheckConfigureStubs(kClientOneKey,
Instrumentation::InstrumentationLevel::kInstrumentWithInstrumentationStubs);
- CHECK_INSTRUMENTATION(Instrumentation::InstrumentationLevel::kInstrumentWithInterpreter,
+ CHECK_INSTRUMENTATION(Instrumentation::InstrumentationLevel::kInstrumentWithInstrumentationStubs,
1U);
// Check we can disable instrumentation.
@@ -968,9 +967,9 @@
CHECK_INSTRUMENTATION(Instrumentation::InstrumentationLevel::kInstrumentWithInterpreter, 2U);
// 1st client requests instrumentation deactivation but 2nd client still needs
- // instrumentation stubs. Since we already got interpreter stubs we need to stay there.
+ // instrumentation stubs.
CheckConfigureStubs(kClientOneKey, Instrumentation::InstrumentationLevel::kInstrumentNothing);
- CHECK_INSTRUMENTATION(Instrumentation::InstrumentationLevel::kInstrumentWithInterpreter,
+ CHECK_INSTRUMENTATION(Instrumentation::InstrumentationLevel::kInstrumentWithInstrumentationStubs,
1U);
// 2nd client requests instrumentation deactivation