Avoid read barrier in `ArtMethod::IsOverridableByDefaultMethod()`.
Also clean up a few other things related to method linking.
Test: m test-art-host-gtest
Test: testrunner.py --host --optimizing
Bug: 181943478
Bug: 119486698
Change-Id: I888dd63db272075cad524e561b3f6271445e8b00
diff --git a/runtime/art_method-inl.h b/runtime/art_method-inl.h
index 29886ab..5c9c739 100644
--- a/runtime/art_method-inl.h
+++ b/runtime/art_method-inl.h
@@ -102,6 +102,13 @@
return type;
}
+inline bool ArtMethod::IsOverridableByDefaultMethod() {
+ // It is safe to avoid the read barrier here since the constant interface flag
+ // in the `Class` object is stored before creating the `ArtMethod` and storing
+ // the declaring class reference. See `ReadBarrierOption`.
+ return GetDeclaringClass<kWithoutReadBarrier>()->IsInterface();
+}
+
inline bool ArtMethod::CheckIncompatibleClassChange(InvokeType type) {
switch (type) {
case kStatic:
diff --git a/runtime/art_method.cc b/runtime/art_method.cc
index 25e9726..f6f8b5f 100644
--- a/runtime/art_method.cc
+++ b/runtime/art_method.cc
@@ -393,10 +393,6 @@
self->PopManagedStackFragment(fragment);
}
-bool ArtMethod::IsOverridableByDefaultMethod() {
- return GetDeclaringClass()->IsInterface();
-}
-
bool ArtMethod::IsSignaturePolymorphic() {
// Methods with a polymorphic signature have constraints that they
// are native and varargs and belong to either MethodHandle or VarHandle.
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index cc0dc9d..5ae6513 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -1127,6 +1127,31 @@
return ComputeModifiedUtf8Hash(name);
}
+ALWAYS_INLINE
+static bool MethodSignatureEquals(ArtMethod* lhs, ArtMethod* rhs)
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ DCHECK(!lhs->IsRuntimeMethod());
+ DCHECK(!lhs->IsProxyMethod());
+ DCHECK(!lhs->IsObsolete());
+ DCHECK(!rhs->IsRuntimeMethod());
+ DCHECK(!rhs->IsProxyMethod());
+ DCHECK(!rhs->IsObsolete());
+ // Do not use `ArtMethod::GetDexFile()` to avoid unnecessary obsolete method checks.
+ // It is safe to avoid the read barrier here, see `ArtMethod::GetDexFile()`.
+ const DexFile& lhs_dex_file = lhs->GetDeclaringClass<kWithoutReadBarrier>()->GetDexFile();
+ const DexFile& rhs_dex_file = rhs->GetDeclaringClass<kWithoutReadBarrier>()->GetDexFile();
+ const dex::MethodId& lhs_mid = lhs_dex_file.GetMethodId(lhs->GetDexMethodIndex());
+ const dex::MethodId& rhs_mid = rhs_dex_file.GetMethodId(rhs->GetDexMethodIndex());
+ if (&lhs_dex_file == &rhs_dex_file) {
+ return lhs_mid.name_idx_ == rhs_mid.name_idx_ &&
+ lhs_mid.proto_idx_ == rhs_mid.proto_idx_;
+ } else {
+ return
+ lhs_dex_file.GetMethodNameView(lhs_mid) == rhs_dex_file.GetMethodNameView(rhs_mid) &&
+ lhs_dex_file.GetMethodSignature(lhs_mid) == rhs_dex_file.GetMethodSignature(rhs_mid);
+ }
+}
+
static void InitializeObjectVirtualMethodHashes(ObjPtr<mirror::Class> java_lang_Object,
PointerSize pointer_size,
/*out*/ ArrayRef<uint32_t> virtual_method_hashes)
@@ -6405,9 +6430,9 @@
Handle<mirror::Class> klass,
Handle<mirror::IfTable> iftable) {
DCHECK(!klass->IsInterface());
- const bool has_superclass = klass->HasSuperClass();
- const size_t ifcount = klass->GetIfTableCount();
- const size_t super_ifcount = has_superclass ? klass->GetSuperClass()->GetIfTableCount() : 0U;
+ DCHECK(klass->HasSuperClass());
+ const size_t ifcount = iftable->Count();
+ const size_t super_ifcount = klass->GetSuperClass()->GetIfTableCount();
for (size_t i = 0; i < ifcount; ++i) {
size_t num_methods = iftable->GetInterface(i)->NumDeclaredVirtualMethods();
if (num_methods > 0) {
@@ -7238,7 +7263,7 @@
private:
// Assign vtable indexes to declared virtual methods for a non-interface class other
// than `java.lang.Object`. Returns the number of vtable entries on success, 0 on failure.
- size_t AssignVtableIndexes(ObjPtr<mirror::Class> klass,
+ size_t AssignVTableIndexes(ObjPtr<mirror::Class> klass,
ObjPtr<mirror::Class> super_class,
size_t num_virtual_methods)
REQUIRES_SHARED(Locks::mutator_lock_);
@@ -7429,19 +7454,7 @@
// NO_THREAD_SAFETY_ANALYSIS: This is called from unannotated `HashSet<>` functions.
bool operator()(uint32_t lhs_index, ArtMethod* rhs) const NO_THREAD_SAFETY_ANALYSIS {
- ArtMethod* lhs = accessor_.GetVTableEntry(lhs_index);
- const DexFile* lhs_dex_file = lhs->GetDexFile();
- const DexFile* rhs_dex_file = rhs->GetDexFile();
- const dex::MethodId& lhs_mid = lhs_dex_file->GetMethodId(lhs->GetDexMethodIndex());
- const dex::MethodId& rhs_mid = rhs_dex_file->GetMethodId(rhs->GetDexMethodIndex());
- if (lhs_dex_file == rhs_dex_file) {
- return lhs_mid.name_idx_ == rhs_mid.name_idx_ &&
- lhs_mid.proto_idx_ == rhs_mid.proto_idx_;
- } else {
- return
- lhs_dex_file->GetMethodNameView(lhs_mid) == rhs_dex_file->GetMethodNameView(rhs_mid) &&
- lhs_dex_file->GetMethodSignature(lhs_mid) == rhs_dex_file->GetMethodSignature(rhs_mid);
- }
+ return MethodSignatureEquals(accessor_.GetVTableEntry(lhs_index), rhs);
}
// NO_THREAD_SAFETY_ANALYSIS: This is called from unannotated `HashSet<>` functions.
@@ -7860,7 +7873,7 @@
}
template <PointerSize kPointerSize>
-size_t ClassLinker::LinkMethodsHelper<kPointerSize>::AssignVtableIndexes(
+size_t ClassLinker::LinkMethodsHelper<kPointerSize>::AssignVTableIndexes(
ObjPtr<mirror::Class> klass, ObjPtr<mirror::Class> super_class, size_t num_virtual_methods) {
DCHECK(!klass->IsInterface());
DCHECK(klass->HasSuperClass());
@@ -7903,7 +7916,7 @@
hash_table_ptr,
hash_table_size,
allocator_.Adapter());
- ScopedArenaVector<uint32_t> same_signature_virtual_methods_lists_(allocator_.Adapter());
+ ArrayRef<uint32_t> same_signature_vtable_lists;
// Insert the first `mirror::Object::kVTableLength` indexes with pre-calculated hashes.
DCHECK_GE(super_vtable_length, mirror::Object::kVTableLength);
for (uint32_t i = 0; i != mirror::Object::kVTableLength; ++i) {
@@ -7922,11 +7935,13 @@
size_t hash = ComputeMethodHash(super_vtable_accessor.GetVTableEntry(i));
auto [it, inserted] = super_vtable_signatures.InsertWithHash(i, hash);
if (UNLIKELY(!inserted)) {
- if (same_signature_virtual_methods_lists_.empty()) {
- same_signature_virtual_methods_lists_.resize(super_vtable_length, dex::kDexNoIndex);
+ if (same_signature_vtable_lists.empty()) {
+ same_signature_vtable_lists = ArrayRef<uint32_t>(
+ allocator_.AllocArray<uint32_t>(super_vtable_length), super_vtable_length);
+ std::fill_n(same_signature_vtable_lists.data(), super_vtable_length, dex::kDexNoIndex);
}
DCHECK_LT(*it, i);
- same_signature_virtual_methods_lists_[i] = *it;
+ same_signature_vtable_lists[i] = *it;
*it = i;
}
}
@@ -7951,13 +7966,13 @@
// superclass method would have been incorrectly overridden.
bool overrides = klass->CanAccessMember(super_method->GetDeclaringClass(),
super_method->GetAccessFlags());
- if (UNLIKELY(!same_signature_virtual_methods_lists_.empty())) {
+ if (UNLIKELY(!same_signature_vtable_lists.empty())) {
// We override only the first accessible virtual method from superclass.
// TODO: Override all methods that need to be overridden according to JLS. b/211854716
size_t current_index = super_index;
- while (same_signature_virtual_methods_lists_[current_index] != dex::kDexNoIndex) {
- DCHECK_LT(same_signature_virtual_methods_lists_[current_index], current_index);
- current_index = same_signature_virtual_methods_lists_[current_index];
+ while (same_signature_vtable_lists[current_index] != dex::kDexNoIndex) {
+ DCHECK_LT(same_signature_vtable_lists[current_index], current_index);
+ current_index = same_signature_vtable_lists[current_index];
ArtMethod* current_method = super_vtable_accessor.GetVTableEntry(current_index);
if (klass->CanAccessMember(current_method->GetDeclaringClass(),
current_method->GetAccessFlags())) {
@@ -8063,15 +8078,14 @@
self->AssertPendingException();
return false;
}
- // TODO: Delay setting the interface table until we're sure we shall not throw an exception.
- klass->SetIfTable(iftable.Get());
const size_t super_vtable_length = klass->GetSuperClass()->GetVTableLength();
Handle<mirror::Class> super_class(hs.NewHandle(klass->GetSuperClass()));
// If there are no new virtual methods and no new interfaces, we can simply reuse
// the vtable from superclass. We may need to make a copy if it's embedded.
- if (num_virtual_methods == 0 && super_class->GetIfTableCount() == klass->GetIfTableCount()) {
+ if (num_virtual_methods == 0 &&
+ static_cast<size_t>(super_class->GetIfTableCount()) == iftable->Count()) {
if (super_class->ShouldHaveEmbeddedVTable()) {
ObjPtr<mirror::PointerArray> vtable =
class_linker_->AllocPointerArray(self, super_vtable_length);
@@ -8092,11 +8106,21 @@
}
// The interface table from superclass has also been reused by `SetupInterfaceLookupTable()`.
DCHECK(iftable.Get() == super_class->GetIfTable()) << klass->PrettyDescriptor();
+ klass->SetIfTable(iftable.Get());
return true;
}
+ // Allocate method arrays, so that we can link interface methods without thread suspension,
+ // otherwise GC could miss visiting newly allocated copied methods.
+ // TODO: Do not allocate copied methods during linking, store only records about what
+ // we need to allocate and allocate it at the end. Start with superclass iftable and
+ // perform copy-on-write when needed to facilitate maximum memory sharing.
+ if (!class_linker_->AllocateIfTableMethodArrays(self, klass, iftable)) {
+ return false;
+ }
+
size_t final_vtable_size =
- AssignVtableIndexes(klass.Get(), super_class.Get(), num_virtual_methods);
+ AssignVTableIndexes(klass.Get(), super_class.Get(), num_virtual_methods);
if (final_vtable_size == 0u) {
self->AssertPendingException();
return false;
@@ -8117,14 +8141,8 @@
vtable->SetElementPtrSize(vtable_index, &virtual_method, kPointerSize);
}
- // Allocate method arrays, so that we can link interface methods without thread suspension,
- // otherwise GC could miss visiting newly allocated copied methods.
- // TODO: Do not allocate copied methods during linking, store only records about what
- // we need to allocate and allocate it at the end. Start with superclass iftable and
- // perform copy-on-write when needed to facilitate maximum memory sharing.
- if (!class_linker_->AllocateIfTableMethodArrays(self, klass, iftable)) {
- return false;
- }
+ // Commit the `iftable`, we shall not throw an exception anymore.
+ klass->SetIfTable(iftable.Get());
// For non-overridden vtable slots, copy a method from `super_class`.
for (size_t j = 0; j != super_vtable_length; ++j) {
@@ -8362,8 +8380,12 @@
// by the class.
//
// See if we can use the superclasses method and skip searching everything else.
- // Note: !found_impl && super_interface
- //
+ // Note: The `super_interface` can be true only for non-interface classes and,
+ // if it is, we search only declared virtual methods which are not overridable
+ // by default methods, so `!found_impl` implies `vtable_impl == nullptr`.
+ DCHECK(!is_interface);
+ DCHECK(using_virtuals);
+ DCHECK(vtable_impl == nullptr);
// If this is a super_interface method it is possible we shouldn't override it because a
// superclass could have implemented it directly. We get the method the superclass used
// to implement this to know if we can override it with a default method. Doing this is
@@ -8387,11 +8409,6 @@
// to the vtable twice, causing corruption (vtable entries having inconsistent and
// illegal states, incorrect vtable size, and incorrect or inconsistent iftable entries)
// in this class and any subclasses.
- DCHECK(vtable_impl == nullptr || vtable_impl == supers_method)
- << "vtable_impl was " << ArtMethod::PrettyMethod(vtable_impl)
- << " and not 'nullptr' or "
- << supers_method->PrettyMethod()
- << " as expected. IFTable appears to be corrupt!";
vtable_impl = supers_method;
}
}