Make ScopedAssertNoThreadSuspension no overhead for non-debug
Previously it required Thread::Current() which may not be free.
The plan is to add a lot more ScopedAssertNoThreadSuspension in
the codebase.
Also cleaned up callers.
Bug: 31458474
Change-Id: I5a1621a5435476504d22266cc01a9bf26aab7568
diff --git a/runtime/art_method.cc b/runtime/art_method.cc
index 1392399..82f8edf 100644
--- a/runtime/art_method.cc
+++ b/runtime/art_method.cc
@@ -107,7 +107,7 @@
}
bool ArtMethod::HasSameNameAndSignature(ArtMethod* other) {
- ScopedAssertNoThreadSuspension ants(Thread::Current(), "HasSameNameAndSignature");
+ ScopedAssertNoThreadSuspension ants("HasSameNameAndSignature");
const DexFile* dex_file = GetDexFile();
const DexFile::MethodId& mid = dex_file->GetMethodId(GetDexMethodIndex());
if (GetDexCache() == other->GetDexCache()) {
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 8edb1b4..8306485 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -1533,7 +1533,7 @@
bool ClassLinker::OpenImageDexFiles(gc::space::ImageSpace* space,
std::vector<std::unique_ptr<const DexFile>>* out_dex_files,
std::string* error_msg) {
- ScopedAssertNoThreadSuspension nts(Thread::Current(), __FUNCTION__);
+ ScopedAssertNoThreadSuspension nts(__FUNCTION__);
const ImageHeader& header = space->GetImageHeader();
mirror::Object* dex_caches_object = header.GetImageRoot(ImageHeader::kDexCaches);
DCHECK(dex_caches_object != nullptr);
@@ -1923,7 +1923,7 @@
ReaderMutexLock mu(self, *Locks::classlinker_classes_lock_);
// Not safe to have thread suspension when we are holding a lock.
if (self != nullptr) {
- ScopedAssertNoThreadSuspension nts(self, __FUNCTION__);
+ ScopedAssertNoThreadSuspension nts(__FUNCTION__);
VisitClassesInternal(visitor);
} else {
VisitClassesInternal(visitor);
@@ -1965,9 +1965,8 @@
void ClassLinker::VisitClassesWithoutClassesLock(ClassVisitor* visitor) {
// TODO: it may be possible to avoid secondary storage if we iterate over dex caches. The problem
// is avoiding duplicates.
- Thread* const self = Thread::Current();
if (!kMovingClasses) {
- ScopedAssertNoThreadSuspension nts(self, __FUNCTION__);
+ ScopedAssertNoThreadSuspension nts(__FUNCTION__);
GetClassesInToVector accumulator;
VisitClasses(&accumulator);
for (mirror::Class* klass : accumulator.classes_) {
@@ -1976,6 +1975,7 @@
}
}
} else {
+ Thread* const self = Thread::Current();
StackHandleScope<1> hs(self);
auto classes = hs.NewHandle<mirror::ObjectArray<mirror::Class>>(nullptr);
// We size the array assuming classes won't be added to the class table during the visit.
@@ -3047,7 +3047,7 @@
{
// Note: We cannot have thread suspension until the field and method arrays are setup or else
// Class::VisitFieldRoots may miss some fields or methods.
- ScopedAssertNoThreadSuspension nts(self, __FUNCTION__);
+ ScopedAssertNoThreadSuspension nts(__FUNCTION__);
// Load static fields.
// We allow duplicate definitions of the same field in a class_data_item
// but ignore the repeated indexes here, b/21868015.
@@ -3113,7 +3113,7 @@
// TODO These should really use the iterators.
for (size_t i = 0; it.HasNextDirectMethod(); i++, it.Next()) {
ArtMethod* method = klass->GetDirectMethodUnchecked(i, image_pointer_size_);
- LoadMethod(self, dex_file, it, klass, method);
+ LoadMethod(dex_file, it, klass, method);
LinkCode(method, oat_class, class_def_method_index);
uint32_t it_method_index = it.GetMemberIndex();
if (last_dex_method_index == it_method_index) {
@@ -3128,7 +3128,7 @@
}
for (size_t i = 0; it.HasNextVirtualMethod(); i++, it.Next()) {
ArtMethod* method = klass->GetVirtualMethodUnchecked(i, image_pointer_size_);
- LoadMethod(self, dex_file, it, klass, method);
+ LoadMethod(dex_file, it, klass, method);
DCHECK_EQ(class_def_method_index, it.NumDirectMethods() + i);
LinkCode(method, oat_class, class_def_method_index);
class_def_method_index++;
@@ -3149,8 +3149,7 @@
dst->SetAccessFlags(it.GetFieldAccessFlags());
}
-void ClassLinker::LoadMethod(Thread* self,
- const DexFile& dex_file,
+void ClassLinker::LoadMethod(const DexFile& dex_file,
const ClassDataItemIterator& it,
Handle<mirror::Class> klass,
ArtMethod* dst) {
@@ -3158,7 +3157,7 @@
const DexFile::MethodId& method_id = dex_file.GetMethodId(dex_method_idx);
const char* method_name = dex_file.StringDataByIdx(method_id.name_idx_);
- ScopedAssertNoThreadSuspension ants(self, "LoadMethod");
+ ScopedAssertNoThreadSuspension ants("LoadMethod");
dst->SetDexMethodIndex(dex_method_idx);
dst->SetDeclaringClass(klass.Get());
dst->SetCodeItemOffset(it.GetMethodCodeItemOffset());
@@ -3692,7 +3691,7 @@
mirror::ClassLoader* class_loader) {
Thread* self = Thread::Current();
WriterMutexLock mu(self, *Locks::classlinker_classes_lock_);
- ScopedAssertNoThreadSuspension ants(self, "Moving image classes to class table");
+ ScopedAssertNoThreadSuspension ants("Moving image classes to class table");
ClassTable* const class_table = InsertClassTableForClassLoader(class_loader);
@@ -3747,7 +3746,7 @@
}
mirror::Class* ClassLinker::LookupClassFromBootImage(const char* descriptor) {
- ScopedAssertNoThreadSuspension ants(Thread::Current(), "Image class lookup");
+ ScopedAssertNoThreadSuspension ants("Image class lookup");
std::vector<mirror::ObjectArray<mirror::DexCache>*> dex_caches_vector =
GetImageDexCaches(Runtime::Current()->GetHeap()->GetBootImageSpaces());
for (mirror::ObjectArray<mirror::DexCache>* dex_caches : dex_caches_vector) {
@@ -6503,7 +6502,7 @@
size_t new_ifcount;
{
- ScopedAssertNoThreadSuspension nts(self, "Copying mirror::Class*'s for FillIfTable");
+ ScopedAssertNoThreadSuspension nts("Copying mirror::Class*'s for FillIfTable");
std::vector<mirror::Class*> to_add;
for (size_t i = 0; i < num_interfaces; i++) {
mirror::Class* interface = have_interfaces ? interfaces->Get(i) :
@@ -8266,7 +8265,7 @@
std::set<DexCacheResolvedClasses> ClassLinker::GetResolvedClasses(bool ignore_boot_classes) {
ScopedTrace trace(__PRETTY_FUNCTION__);
ScopedObjectAccess soa(Thread::Current());
- ScopedAssertNoThreadSuspension ants(soa.Self(), __FUNCTION__);
+ ScopedAssertNoThreadSuspension ants(__FUNCTION__);
std::set<DexCacheResolvedClasses> ret;
VLOG(class_linker) << "Collecting resolved classes";
const uint64_t start_time = NanoTime();
@@ -8340,7 +8339,7 @@
Thread* const self = Thread::Current();
std::unordered_map<std::string, const DexFile*> location_to_dex_file;
ScopedObjectAccess soa(self);
- ScopedAssertNoThreadSuspension ants(soa.Self(), __FUNCTION__);
+ ScopedAssertNoThreadSuspension ants(__FUNCTION__);
ReaderMutexLock mu(self, *DexLock());
for (const ClassLinker::DexCacheData& data : GetDexCachesData()) {
if (!self->IsJWeakCleared(data.weak_root)) {
diff --git a/runtime/class_linker.h b/runtime/class_linker.h
index 4bd1bd2..b0ed969 100644
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -768,8 +768,7 @@
void LoadField(const ClassDataItemIterator& it, Handle<mirror::Class> klass, ArtField* dst)
REQUIRES_SHARED(Locks::mutator_lock_);
- void LoadMethod(Thread* self,
- const DexFile& dex_file,
+ void LoadMethod(const DexFile& dex_file,
const ClassDataItemIterator& it,
Handle<mirror::Class> klass, ArtMethod* dst)
REQUIRES_SHARED(Locks::mutator_lock_);
diff --git a/runtime/debugger.cc b/runtime/debugger.cc
index 9f3c2aa..2821004 100644
--- a/runtime/debugger.cc
+++ b/runtime/debugger.cc
@@ -1986,7 +1986,7 @@
if (error != JDWP::ERR_NONE) {
return JDWP::ERR_INVALID_OBJECT;
}
- ScopedAssertNoThreadSuspension ants(soa.Self(), "Debugger: GetThreadGroup");
+ ScopedAssertNoThreadSuspension ants("Debugger: GetThreadGroup");
// Okay, so it's an object, but is it actually a thread?
DecodeThread(soa, thread_id, &error);
if (error == JDWP::ERR_THREAD_NOT_ALIVE) {
@@ -2036,7 +2036,7 @@
if (error != JDWP::ERR_NONE) {
return error;
}
- ScopedAssertNoThreadSuspension ants(soa.Self(), "Debugger: GetThreadGroupName");
+ ScopedAssertNoThreadSuspension ants("Debugger: GetThreadGroupName");
ArtField* f = soa.DecodeField(WellKnownClasses::java_lang_ThreadGroup_name);
CHECK(f != nullptr);
mirror::String* s = reinterpret_cast<mirror::String*>(f->GetObject(thread_group));
@@ -2055,7 +2055,7 @@
}
mirror::Object* parent;
{
- ScopedAssertNoThreadSuspension ants(soa.Self(), "Debugger: GetThreadGroupParent");
+ ScopedAssertNoThreadSuspension ants("Debugger: GetThreadGroupParent");
ArtField* f = soa.DecodeField(WellKnownClasses::java_lang_ThreadGroup_parent);
CHECK(f != nullptr);
parent = f->GetObject(thread_group);
diff --git a/runtime/entrypoints/entrypoint_utils-inl.h b/runtime/entrypoints/entrypoint_utils-inl.h
index d03a9d8..1bf5c53 100644
--- a/runtime/entrypoints/entrypoint_utils-inl.h
+++ b/runtime/entrypoints/entrypoint_utils-inl.h
@@ -48,7 +48,7 @@
// This method is being used by artQuickResolutionTrampoline, before it sets up
// the passed parameters in a GC friendly way. Therefore we must never be
// suspended while executing it.
- ScopedAssertNoThreadSuspension sants(Thread::Current(), __FUNCTION__);
+ ScopedAssertNoThreadSuspension sants(__FUNCTION__);
uint32_t method_index = inline_info.GetMethodIndexAtDepth(encoding, inlining_depth);
InvokeType invoke_type = static_cast<InvokeType>(
diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc
index 600aff1..cb5226b 100644
--- a/runtime/gc/heap.cc
+++ b/runtime/gc/heap.cc
@@ -1011,7 +1011,7 @@
DecrementDisableMovingGC(self);
} else {
// GCs can move objects, so don't allow this.
- ScopedAssertNoThreadSuspension ants(self, "Visiting objects");
+ ScopedAssertNoThreadSuspension ants("Visiting objects");
DCHECK(region_space_ == nullptr);
VisitObjectsInternal(callback, arg);
}
diff --git a/runtime/instrumentation.cc b/runtime/instrumentation.cc
index 388561b..ff43389 100644
--- a/runtime/instrumentation.cc
+++ b/runtime/instrumentation.cc
@@ -966,7 +966,7 @@
ArtMethod* callee) const {
// We cannot have thread suspension since that would cause the this_object parameter to
// potentially become a dangling pointer. An alternative could be to put it in a handle instead.
- ScopedAssertNoThreadSuspension ants(thread, __FUNCTION__);
+ ScopedAssertNoThreadSuspension ants(__FUNCTION__);
for (InstrumentationListener* listener : invoke_virtual_or_interface_listeners_) {
if (listener != nullptr) {
listener->InvokeVirtualOrInterface(thread, this_object, caller, dex_pc, callee);
diff --git a/runtime/interpreter/interpreter_common.h b/runtime/interpreter/interpreter_common.h
index 9d76685..814adf7 100644
--- a/runtime/interpreter/interpreter_common.h
+++ b/runtime/interpreter/interpreter_common.h
@@ -147,8 +147,7 @@
jit::Jit* jit = Runtime::Current()->GetJit();
if (jit != nullptr) {
if (type == kVirtual || type == kInterface) {
- jit->InvokeVirtualOrInterface(
- self, receiver, sf_method, shadow_frame.GetDexPC(), called_method);
+ jit->InvokeVirtualOrInterface(receiver, sf_method, shadow_frame.GetDexPC(), called_method);
}
jit->AddSamples(self, sf_method, 1, /*with_backedges*/false);
}
@@ -195,7 +194,7 @@
jit::Jit* jit = Runtime::Current()->GetJit();
if (jit != nullptr) {
jit->InvokeVirtualOrInterface(
- self, receiver, shadow_frame.GetMethod(), shadow_frame.GetDexPC(), called_method);
+ receiver, shadow_frame.GetMethod(), shadow_frame.GetDexPC(), called_method);
jit->AddSamples(self, shadow_frame.GetMethod(), 1, /*with_backedges*/false);
}
instrumentation::Instrumentation* instrumentation = Runtime::Current()->GetInstrumentation();
diff --git a/runtime/jit/jit.cc b/runtime/jit/jit.cc
index cff2354..d984f45 100644
--- a/runtime/jit/jit.cc
+++ b/runtime/jit/jit.cc
@@ -431,7 +431,7 @@
const uint8_t* native_pc = nullptr;
{
- ScopedAssertNoThreadSuspension sts(thread, "Holding OSR method");
+ ScopedAssertNoThreadSuspension sts("Holding OSR method");
const OatQuickMethodHeader* osr_method = jit->GetCodeCache()->LookupOsrMethodHeader(method);
if (osr_method == nullptr) {
// No osr method yet, just return to the interpreter.
@@ -683,12 +683,11 @@
}
}
-void Jit::InvokeVirtualOrInterface(Thread* thread,
- mirror::Object* this_object,
+void Jit::InvokeVirtualOrInterface(mirror::Object* this_object,
ArtMethod* caller,
uint32_t dex_pc,
ArtMethod* callee ATTRIBUTE_UNUSED) {
- ScopedAssertNoThreadSuspension ants(thread, __FUNCTION__);
+ ScopedAssertNoThreadSuspension ants(__FUNCTION__);
DCHECK(this_object != nullptr);
ProfilingInfo* info = caller->GetProfilingInfo(kRuntimePointerSize);
if (info != nullptr) {
diff --git a/runtime/jit/jit.h b/runtime/jit/jit.h
index 417a185..37edfd1 100644
--- a/runtime/jit/jit.h
+++ b/runtime/jit/jit.h
@@ -108,8 +108,7 @@
void AddSamples(Thread* self, ArtMethod* method, uint16_t samples, bool with_backedges)
REQUIRES_SHARED(Locks::mutator_lock_);
- void InvokeVirtualOrInterface(Thread* thread,
- mirror::Object* this_object,
+ void InvokeVirtualOrInterface(mirror::Object* this_object,
ArtMethod* caller,
uint32_t dex_pc,
ArtMethod* callee)
diff --git a/runtime/mirror/class.cc b/runtime/mirror/class.cc
index c979c28..b69f06b 100644
--- a/runtime/mirror/class.cc
+++ b/runtime/mirror/class.cc
@@ -784,7 +784,7 @@
}
// Though GetDirectInterface() should not cause thread suspension when called
// from here, it takes a Handle as an argument, so we need to wrap `k`.
- ScopedAssertNoThreadSuspension ants(self, __FUNCTION__);
+ ScopedAssertNoThreadSuspension ants(__FUNCTION__);
StackHandleScope<1> hs(self);
Handle<mirror::Class> h_k(hs.NewHandle(k));
// Is this field in any of this class' interfaces?
diff --git a/runtime/native/java_lang_Class.cc b/runtime/native/java_lang_Class.cc
index d89a334..2b17f4e 100644
--- a/runtime/native/java_lang_Class.cc
+++ b/runtime/native/java_lang_Class.cc
@@ -668,8 +668,7 @@
caller.Assign(GetCallingClass(soa.Self(), 1));
}
if (UNLIKELY(caller.Get() != nullptr && !VerifyAccess(
- soa.Self(), receiver.Get(), declaring_class, constructor->GetAccessFlags(),
- caller.Get()))) {
+ receiver.Get(), declaring_class, constructor->GetAccessFlags(), caller.Get()))) {
soa.Self()->ThrowNewExceptionF(
"Ljava/lang/IllegalAccessException;", "%s is not accessible from %s",
PrettyMethod(constructor).c_str(), PrettyClass(caller.Get()).c_str());
diff --git a/runtime/reflection.cc b/runtime/reflection.cc
index f2af3da..67e3fe8 100644
--- a/runtime/reflection.cc
+++ b/runtime/reflection.cc
@@ -625,8 +625,12 @@
// If method is not set to be accessible, verify it can be accessed by the caller.
mirror::Class* calling_class = nullptr;
- if (!accessible && !VerifyAccess(soa.Self(), receiver, declaring_class, m->GetAccessFlags(),
- &calling_class, num_frames)) {
+ if (!accessible && !VerifyAccess(soa.Self(),
+ receiver,
+ declaring_class,
+ m->GetAccessFlags(),
+ &calling_class,
+ num_frames)) {
ThrowIllegalAccessException(
StringPrintf("Class %s cannot access %s method %s of class %s",
calling_class == nullptr ? "null" : PrettyClass(calling_class).c_str(),
@@ -857,15 +861,17 @@
return false;
}
*calling_class = klass;
- return VerifyAccess(self, obj, declaring_class, access_flags, klass);
+ return VerifyAccess(obj, declaring_class, access_flags, klass);
}
-bool VerifyAccess(Thread* self, mirror::Object* obj, mirror::Class* declaring_class,
- uint32_t access_flags, mirror::Class* calling_class) {
+bool VerifyAccess(mirror::Object* obj,
+ mirror::Class* declaring_class,
+ uint32_t access_flags,
+ mirror::Class* calling_class) {
if (calling_class == declaring_class) {
return true;
}
- ScopedAssertNoThreadSuspension sants(self, "verify-access");
+ ScopedAssertNoThreadSuspension sants("verify-access");
if ((access_flags & kAccPrivate) != 0) {
return false;
}
diff --git a/runtime/reflection.h b/runtime/reflection.h
index 579c6b1..208b533 100644
--- a/runtime/reflection.h
+++ b/runtime/reflection.h
@@ -74,8 +74,10 @@
REQUIRES_SHARED(Locks::mutator_lock_);
// This version takes a known calling class.
-bool VerifyAccess(Thread* self, mirror::Object* obj, mirror::Class* declaring_class,
- uint32_t access_flags, mirror::Class* calling_class)
+bool VerifyAccess(mirror::Object* obj,
+ mirror::Class* declaring_class,
+ uint32_t access_flags,
+ mirror::Class* calling_class)
REQUIRES_SHARED(Locks::mutator_lock_);
// Get the calling class by using a stack visitor, may return null for unattached native threads.
diff --git a/runtime/thread.h b/runtime/thread.h
index d248123..1036d00 100644
--- a/runtime/thread.h
+++ b/runtime/thread.h
@@ -1546,19 +1546,25 @@
class SCOPED_CAPABILITY ScopedAssertNoThreadSuspension {
public:
- ScopedAssertNoThreadSuspension(Thread* self, const char* cause) ACQUIRE(Roles::uninterruptible_)
- : self_(self), old_cause_(self->StartAssertNoThreadSuspension(cause)) {
+ ALWAYS_INLINE ScopedAssertNoThreadSuspension(const char* cause) ACQUIRE(Roles::uninterruptible_) {
+ if (kIsDebugBuild) {
+ self_ = Thread::Current();
+ old_cause_ = self_->StartAssertNoThreadSuspension(cause);
+ } else {
+ Roles::uninterruptible_.Acquire(); // No-op.
+ }
}
- ~ScopedAssertNoThreadSuspension() RELEASE(Roles::uninterruptible_) {
- self_->EndAssertNoThreadSuspension(old_cause_);
- }
- Thread* Self() {
- return self_;
+ ALWAYS_INLINE ~ScopedAssertNoThreadSuspension() RELEASE(Roles::uninterruptible_) {
+ if (kIsDebugBuild) {
+ self_->EndAssertNoThreadSuspension(old_cause_);
+ } else {
+ Roles::uninterruptible_.Release(); // No-op.
+ }
}
private:
- Thread* const self_;
- const char* const old_cause_;
+ Thread* self_;
+ const char* old_cause_;
};
class ScopedStackedShadowFramePusher {