Remove read barriers in Class::IsClassClass and Object::IsClass
Remove unnecessary read barriers since Class.class is not movable.
Test: test-art-host
Bug: 114413743
Change-Id: Id68562606bab75bf5ba99a7e8a38e7db9d46df1d
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 511d468..44c73fb 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -431,6 +431,8 @@
heap->IncrementDisableMovingGC(self);
StackHandleScope<64> hs(self); // 64 is picked arbitrarily.
auto class_class_size = mirror::Class::ClassClassSize(image_pointer_size_);
+ // Allocate the object as non-movable so that there are no cases where Object::IsClass returns
+ // the incorrect result when comparing to-space vs from-space.
Handle<mirror::Class> java_lang_Class(hs.NewHandle(ObjPtr<mirror::Class>::DownCast(MakeObjPtr(
heap->AllocNonMovableObject<true>(self, nullptr, class_class_size, VoidFunctor())))));
CHECK(java_lang_Class != nullptr);
diff --git a/runtime/gc/space/image_space.cc b/runtime/gc/space/image_space.cc
index 3999e27..18e656c 100644
--- a/runtime/gc/space/image_space.cc
+++ b/runtime/gc/space/image_space.cc
@@ -1008,7 +1008,7 @@
}
if (obj->IsClass()) {
- mirror::Class* klass = obj->AsClass<kVerifyNone, kWithoutReadBarrier>();
+ mirror::Class* klass = obj->AsClass<kVerifyNone>();
// Fixup super class before visiting instance fields which require
// information from their super class to calculate offsets.
mirror::Class* super_class = klass->GetSuperClass<kVerifyNone, kWithoutReadBarrier>();
@@ -1026,8 +1026,8 @@
*this);
// Note that this code relies on no circular dependencies.
// We want to use our own class loader and not the one in the image.
- if (obj->IsClass<kVerifyNone, kWithoutReadBarrier>()) {
- mirror::Class* as_klass = obj->AsClass<kVerifyNone, kWithoutReadBarrier>();
+ if (obj->IsClass<kVerifyNone>()) {
+ mirror::Class* as_klass = obj->AsClass<kVerifyNone>();
FixupObjectAdapter visitor(boot_image_, boot_oat_, app_image_, app_oat_);
as_klass->FixupNativePointers<kVerifyNone, kWithoutReadBarrier>(as_klass,
pointer_size_,
diff --git a/runtime/jit/jit_code_cache.cc b/runtime/jit/jit_code_cache.cc
index 184aba8..bcbdc3b 100644
--- a/runtime/jit/jit_code_cache.cc
+++ b/runtime/jit/jit_code_cache.cc
@@ -530,7 +530,7 @@
// This does not need a read barrier because this is called by GC.
mirror::Class* cls = root_ptr->Read<kWithoutReadBarrier>();
if (cls != nullptr && cls != weak_sentinel) {
- DCHECK((cls->IsClass<kDefaultVerifyFlags, kWithoutReadBarrier>()));
+ DCHECK((cls->IsClass<kDefaultVerifyFlags>()));
// Look at the classloader of the class to know if it has been unloaded.
// This does not need a read barrier because this is called by GC.
mirror::Object* class_loader =
diff --git a/runtime/mirror/class-inl.h b/runtime/mirror/class-inl.h
index 51dc1a4..d3f8921 100644
--- a/runtime/mirror/class-inl.h
+++ b/runtime/mirror/class-inl.h
@@ -378,7 +378,7 @@
inline bool Class::IsVariableSize() {
// Classes, arrays, and strings vary in size, and so the object_size_ field cannot
// be used to Get their instance size
- return IsClassClass<kVerifyFlags, kReadBarrierOption>() ||
+ return IsClassClass<kVerifyFlags>() ||
IsArrayClass<kVerifyFlags, kReadBarrierOption>() ||
IsStringClass();
}
@@ -853,10 +853,12 @@
return size;
}
-template<VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption>
+template<VerifyObjectFlags kVerifyFlags>
inline bool Class::IsClassClass() {
- ObjPtr<Class> java_lang_Class = GetClass<kVerifyFlags, kReadBarrierOption>()->
- template GetClass<kVerifyFlags, kReadBarrierOption>();
+ // OK to look at from-space copies since java.lang.Class.class is not movable.
+ // See b/114413743
+ ObjPtr<Class> java_lang_Class = GetClass<kVerifyFlags, kWithoutReadBarrier>()->
+ template GetClass<kVerifyFlags, kWithoutReadBarrier>();
return this == java_lang_Class;
}
diff --git a/runtime/mirror/class.h b/runtime/mirror/class.h
index 811ee51..4015bd2 100644
--- a/runtime/mirror/class.h
+++ b/runtime/mirror/class.h
@@ -430,8 +430,7 @@
ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
bool IsArrayClass() REQUIRES_SHARED(Locks::mutator_lock_);
- template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
- ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
+ template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>
bool IsClassClass() REQUIRES_SHARED(Locks::mutator_lock_);
bool IsThrowableClass() REQUIRES_SHARED(Locks::mutator_lock_);
diff --git a/runtime/mirror/object-inl.h b/runtime/mirror/object-inl.h
index 40832bc..1b03956 100644
--- a/runtime/mirror/object-inl.h
+++ b/runtime/mirror/object-inl.h
@@ -137,17 +137,18 @@
return klass->IsAssignableFrom(GetClass<kVerifyFlags>());
}
-template<VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption>
+template<VerifyObjectFlags kVerifyFlags>
inline bool Object::IsClass() {
- constexpr auto kNewFlags = RemoveThisFlags(kVerifyFlags);
- Class* java_lang_Class = GetClass<kVerifyFlags, kReadBarrierOption>()->
- template GetClass<kVerifyFlags, kReadBarrierOption>();
- return GetClass<kNewFlags, kReadBarrierOption>() == java_lang_Class;
+ // OK to look at from-space copies since java.lang.Class.class is not movable.
+ // See b/114413743
+ ObjPtr<Class> klass = GetClass<kVerifyFlags, kWithoutReadBarrier>();
+ ObjPtr<Class> java_lang_Class = klass->template GetClass<kVerifyFlags, kWithoutReadBarrier>();
+ return klass == java_lang_Class;
}
-template<VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption>
+template<VerifyObjectFlags kVerifyFlags>
inline Class* Object::AsClass() {
- DCHECK((IsClass<kVerifyFlags, kReadBarrierOption>()));
+ DCHECK((IsClass<kVerifyFlags>()));
return down_cast<Class*>(this);
}
@@ -350,8 +351,8 @@
constexpr auto kNewFlags = RemoveThisFlags(kVerifyFlags);
if (IsArrayInstance<kVerifyFlags, kRBO>()) {
result = AsArray<kNewFlags, kRBO>()->template SizeOf<kNewFlags, kRBO>();
- } else if (IsClass<kNewFlags, kRBO>()) {
- result = AsClass<kNewFlags, kRBO>()->template SizeOf<kNewFlags, kRBO>();
+ } else if (IsClass<kNewFlags>()) {
+ result = AsClass<kNewFlags>()->template SizeOf<kNewFlags, kRBO>();
} else if (GetClass<kNewFlags, kRBO>()->IsStringClass()) {
result = AsString<kNewFlags, kRBO>()->template SizeOf<kNewFlags>();
} else {
@@ -867,7 +868,7 @@
// inheritance hierarchy and find reference offsets the hard way. In the static case, just
// consider this class.
for (ObjPtr<Class> klass = kIsStatic
- ? AsClass<kVerifyFlags, kReadBarrierOption>()
+ ? AsClass<kVerifyFlags>()
: GetClass<kVerifyFlags, kReadBarrierOption>();
klass != nullptr;
klass = kIsStatic ? nullptr : klass->GetSuperClass<kVerifyFlags, kReadBarrierOption>()) {
diff --git a/runtime/mirror/object-refvisitor-inl.h b/runtime/mirror/object-refvisitor-inl.h
index 39e32bf..bd23971 100644
--- a/runtime/mirror/object-refvisitor-inl.h
+++ b/runtime/mirror/object-refvisitor-inl.h
@@ -39,7 +39,7 @@
if (LIKELY(class_flags == kClassFlagNormal)) {
DCHECK((!klass->IsVariableSize<kVerifyFlags, kReadBarrierOption>()));
VisitInstanceFieldsReferences<kVerifyFlags, kReadBarrierOption>(klass, visitor);
- DCHECK((!klass->IsClassClass<kVerifyFlags, kReadBarrierOption>()));
+ DCHECK((!klass->IsClassClass<kVerifyFlags>()));
DCHECK(!klass->IsStringClass());
DCHECK(!klass->IsClassLoaderClass());
DCHECK((!klass->IsArrayClass<kVerifyFlags, kReadBarrierOption>()));
@@ -47,8 +47,8 @@
if ((class_flags & kClassFlagNoReferenceFields) == 0) {
DCHECK(!klass->IsStringClass());
if (class_flags == kClassFlagClass) {
- DCHECK((klass->IsClassClass<kVerifyFlags, kReadBarrierOption>()));
- ObjPtr<Class> as_klass = AsClass<kVerifyNone, kReadBarrierOption>();
+ DCHECK((klass->IsClassClass<kVerifyFlags>()));
+ ObjPtr<Class> as_klass = AsClass<kVerifyNone>();
as_klass->VisitReferences<kVisitNativeRoots, kVerifyFlags, kReadBarrierOption>(klass,
visitor);
} else if (class_flags == kClassFlagObjectArray) {
@@ -69,7 +69,7 @@
kReadBarrierOption>(klass, visitor);
}
} else if (kIsDebugBuild) {
- CHECK((!klass->IsClassClass<kVerifyFlags, kReadBarrierOption>()));
+ CHECK((!klass->IsClassClass<kVerifyFlags>()));
CHECK((!klass->IsObjectArrayClass<kVerifyFlags, kReadBarrierOption>()));
// String still has instance fields for reflection purposes but these don't exist in
// actual string instances.
diff --git a/runtime/mirror/object.h b/runtime/mirror/object.h
index 35946d7..48ce5c1 100644
--- a/runtime/mirror/object.h
+++ b/runtime/mirror/object.h
@@ -169,11 +169,9 @@
void NotifyAll(Thread* self) REQUIRES_SHARED(Locks::mutator_lock_);
void Wait(Thread* self, int64_t timeout, int32_t nanos) REQUIRES_SHARED(Locks::mutator_lock_);
- template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
- ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
+ template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>
bool IsClass() REQUIRES_SHARED(Locks::mutator_lock_);
- template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
- ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
+ template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>
Class* AsClass() REQUIRES_SHARED(Locks::mutator_lock_);
template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,