Reuse superclass `IfTable` with non-marker interfaces.
This re-enables `IfTable` sharing for the case that the
https://android-review.googlesource.com/1969179
removed because it was buggy. We implement it properly here
while also avoiding unnecessary IMT updates.
Test: m test-art-host-gtest
Test: testrunner.py --host --optimizing
Change-Id: I729b64cb85e95aedeb42a7d804d790bd773238c2
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index c64c137..813d8f0 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -6577,8 +6577,8 @@
// If there are no new interfaces, we can recycle parent's interface table if the class
// inherits no interfaces from the superclass (this is always the case for interfaces as
- // their superclass `java.lang.Object` does not implement any interface), or all interfaces
- // inherited from the superclass are just marker interfaces.
+ // their superclass `java.lang.Object` does not implement any interface), or there are no
+ // new virtuals, or all interfaces inherited from the superclass are just marker interfaces.
auto is_marker_iface = [=](size_t index) REQUIRES_SHARED(Locks::mutator_lock_) ALWAYS_INLINE {
return super_iftable->GetMethodArrayCount(index) == 0;
};
@@ -6587,7 +6587,8 @@
return true;
}
DCHECK(!klass->IsInterface());
- return std::all_of(CountIter(0), CountIter(super_ifcount), is_marker_iface);
+ return klass->NumDeclaredVirtualMethods() == 0u ||
+ std::all_of(CountIter(0), CountIter(super_ifcount), is_marker_iface);
};
if (num_interfaces == 0 && can_reuse_super_iftable()) {
return super_iftable;
@@ -6898,35 +6899,6 @@
} // namespace
-void ClassLinker::FillImtFromSuperClass(Handle<mirror::Class> klass,
- ArtMethod* unimplemented_method,
- ArtMethod* imt_conflict_method,
- bool* new_conflict,
- ArtMethod** imt) {
- DCHECK(klass->HasSuperClass());
- ObjPtr<mirror::Class> super_class = klass->GetSuperClass();
- if (super_class->ShouldHaveImt()) {
- ImTable* super_imt = super_class->GetImt(image_pointer_size_);
- for (size_t i = 0; i < ImTable::kSize; ++i) {
- imt[i] = super_imt->Get(i, image_pointer_size_);
- }
- } else {
- // No imt in the super class, need to reconstruct from the iftable.
- ObjPtr<mirror::IfTable> if_table = super_class->GetIfTable();
- if (if_table->Count() != 0) {
- // Ignore copied methods, we will handle these later.
- FillIMTFromIfTable(if_table,
- unimplemented_method,
- imt_conflict_method,
- klass.Get(),
- /*create_conflict_tables=*/false,
- /*ignore_copied_methods=*/true,
- /*out*/new_conflict,
- /*out*/imt);
- }
- }
-}
-
template <PointerSize kPointerSize>
class ClassLinker::LinkMethodsHelper {
public:
@@ -6969,6 +6941,7 @@
// and records data for copied methods which shall be referenced by the vtable.
size_t AssignVTableIndexes(ObjPtr<mirror::Class> klass,
ObjPtr<mirror::Class> super_class,
+ bool is_super_abstract,
size_t num_virtual_methods,
ObjPtr<mirror::IfTable> iftable)
REQUIRES_SHARED(Locks::mutator_lock_);
@@ -6982,8 +6955,11 @@
REQUIRES_SHARED(Locks::mutator_lock_) COLD_ATTR;
void ReallocMethods(ObjPtr<mirror::Class> klass) REQUIRES_SHARED(Locks::mutator_lock_);
- void FinalizeIfTable(ObjPtr<mirror::IfTable> iftable,
+ void FinalizeIfTable(ObjPtr<mirror::Class> klass,
+ ObjPtr<mirror::IfTable> iftable,
ObjPtr<mirror::PointerArray> vtable,
+ bool is_klass_abstract,
+ bool is_super_abstract,
bool* out_new_conflict,
ArtMethod** out_imt)
REQUIRES_SHARED(Locks::mutator_lock_);
@@ -7719,16 +7695,44 @@
template <PointerSize kPointerSize>
void ClassLinker::LinkMethodsHelper<kPointerSize>::FinalizeIfTable(
+ ObjPtr<mirror::Class> klass,
ObjPtr<mirror::IfTable> iftable,
ObjPtr<mirror::PointerArray> vtable,
+ bool is_klass_abstract,
+ bool is_super_abstract,
bool* out_new_conflict,
ArtMethod** out_imt) {
- ObjPtr<mirror::IfTable> super_iftable = klass_->GetSuperClass()->GetIfTable();
+ ObjPtr<mirror::IfTable> super_iftable = klass->GetSuperClass()->GetIfTable();
size_t ifcount = iftable->Count();
size_t super_ifcount = super_iftable->Count();
- ArtMethod* const unimplemented_method = runtime_->GetImtUnimplementedMethod();
- ArtMethod* const imt_conflict_method = runtime_->GetImtConflictMethod();
+ ArtMethod* unimplemented_method = nullptr;
+ ArtMethod* imt_conflict_method = nullptr;
+ uintptr_t imt_methods_begin = 0u;
+ size_t imt_methods_size = 0u;
+ DCHECK_EQ(klass->ShouldHaveImt(), !is_klass_abstract);
+ DCHECK_EQ(klass->GetSuperClass()->ShouldHaveImt(), !is_super_abstract);
+ if (!is_klass_abstract) {
+ unimplemented_method = runtime_->GetImtUnimplementedMethod();
+ imt_conflict_method = runtime_->GetImtConflictMethod();
+ if (is_super_abstract) {
+ // There was no IMT in superclass to copy to `out_imt[]`, so we need
+ // to fill it with all implementation methods from superclass.
+ DCHECK_EQ(imt_methods_begin, 0u);
+ imt_methods_size = std::numeric_limits<size_t>::max(); // No method at the last byte.
+ } else {
+ // If the superclass has IMT, we have already copied it to `out_imt[]` and
+ // we do not need to call `SetIMTRef()` for interfaces from superclass when
+ // the implementation method is already in the superclass, only for new methods.
+ // For simplicity, use the entire method array including direct methods.
+ LengthPrefixedArray<ArtMethod>* const new_methods = klass->GetMethodsPtr();
+ if (new_methods != nullptr) {
+ DCHECK_NE(new_methods->size(), 0u);
+ imt_methods_begin = reinterpret_cast<uintptr_t>(&new_methods->At(0));
+ imt_methods_size = new_methods->size() * kMethodSize;
+ }
+ }
+ }
// For interfaces inherited from superclass, the new method arrays are empty,
// so use vtable indexes from implementation methods from the superclass method array.
@@ -7747,17 +7751,21 @@
ArtMethod* implementation =
vtable->GetElementPtrSize<ArtMethod*, kPointerSize>(vtable_index);
method_array->SetElementPtrSize(j, implementation, kPointerSize);
- // Place method in imt if entry is empty, place conflict otherwise.
- ArtMethod** imt_ptr = &out_imt[iface->GetVirtualMethod(j, kPointerSize)->GetImtIndex()];
- class_linker_->SetIMTRef(unimplemented_method,
- imt_conflict_method,
- implementation,
- /*out*/out_new_conflict,
- /*out*/imt_ptr);
+ // Check if we need to update IMT with this method, see above.
+ if (reinterpret_cast<uintptr_t>(implementation) - imt_methods_begin < imt_methods_size) {
+ // Place method in imt if entry is empty, place conflict otherwise.
+ ArtMethod** imt_ptr = &out_imt[iface->GetVirtualMethod(j, kPointerSize)->GetImtIndex()];
+ class_linker_->SetIMTRef(unimplemented_method,
+ imt_conflict_method,
+ implementation,
+ /*out*/out_new_conflict,
+ /*out*/imt_ptr);
+ }
}
}
// New interface method arrays contain vtable indexes. Translate them to methods.
+ DCHECK_EQ(klass->ShouldHaveImt(), !is_klass_abstract);
for (size_t i = super_ifcount; i != ifcount; ++i) {
ObjPtr<mirror::PointerArray> method_array = iftable->GetMethodArrayOrNull(i);
if (method_array == nullptr) {
@@ -7770,13 +7778,15 @@
ArtMethod* implementation =
vtable->GetElementPtrSize<ArtMethod*, kPointerSize>(vtable_index);
method_array->SetElementPtrSize(j, implementation, kPointerSize);
- // Place method in imt if entry is empty, place conflict otherwise.
- ArtMethod** imt_ptr = &out_imt[iface->GetVirtualMethod(j, kPointerSize)->GetImtIndex()];
- class_linker_->SetIMTRef(unimplemented_method,
- imt_conflict_method,
- implementation,
- /*out*/out_new_conflict,
- /*out*/imt_ptr);
+ if (!is_klass_abstract) {
+ // Place method in IMT if entry is empty, place conflict otherwise.
+ ArtMethod** imt_ptr = &out_imt[iface->GetVirtualMethod(j, kPointerSize)->GetImtIndex()];
+ class_linker_->SetIMTRef(unimplemented_method,
+ imt_conflict_method,
+ implementation,
+ /*out*/out_new_conflict,
+ /*out*/imt_ptr);
+ }
}
}
}
@@ -7799,6 +7809,7 @@
size_t ClassLinker::LinkMethodsHelper<kPointerSize>::AssignVTableIndexes(
ObjPtr<mirror::Class> klass,
ObjPtr<mirror::Class> super_class,
+ bool is_super_abstract,
size_t num_virtual_methods,
ObjPtr<mirror::IfTable> iftable) {
DCHECK(!klass->IsInterface());
@@ -7815,16 +7826,18 @@
// methods with the same signature (walked as singly-linked lists).
uint8_t* raw_super_vtable;
size_t super_vtable_length;
- if (super_class->ShouldHaveEmbeddedVTable()) {
- raw_super_vtable = reinterpret_cast<uint8_t*>(super_class.Ptr()) +
- mirror::Class::EmbeddedVTableOffset(kPointerSize).Uint32Value();
- super_vtable_length = super_class->GetEmbeddedVTableLength();
- } else {
+ if (is_super_abstract) {
+ DCHECK(!super_class->ShouldHaveEmbeddedVTable());
ObjPtr<mirror::PointerArray> super_vtable = super_class->GetVTableDuringLinking();
DCHECK(super_vtable != nullptr);
raw_super_vtable = reinterpret_cast<uint8_t*>(super_vtable.Ptr()) +
mirror::Array::DataOffset(static_cast<size_t>(kPointerSize)).Uint32Value();
super_vtable_length = super_vtable->GetLength();
+ } else {
+ DCHECK(super_class->ShouldHaveEmbeddedVTable());
+ raw_super_vtable = reinterpret_cast<uint8_t*>(super_class.Ptr()) +
+ mirror::Class::EmbeddedVTableOffset(kPointerSize).Uint32Value();
+ super_vtable_length = super_class->GetEmbeddedVTableLength();
}
VTableAccessor super_vtable_accessor(raw_super_vtable, super_vtable_length);
static constexpr double kMinLoadFactor = 0.3;
@@ -8229,13 +8242,6 @@
}
return true;
} else if (LIKELY(klass->HasSuperClass())) {
- // Copy IMT from superclass. It shall be updated later if needed.
- class_linker_->FillImtFromSuperClass(klass,
- runtime_->GetImtUnimplementedMethod(),
- runtime_->GetImtConflictMethod(),
- out_new_conflict,
- out_imt);
-
// We set up the interface lookup table now because we need it to determine if we need
// to update any vtable entries with new default method implementations.
StackHandleScope<3> hs(self);
@@ -8249,13 +8255,41 @@
}
const size_t super_vtable_length = klass->GetSuperClass()->GetVTableLength();
- Handle<mirror::Class> super_class(hs.NewHandle(klass->GetSuperClass()));
+ Handle<mirror::Class> super_class = hs.NewHandle(klass->GetSuperClass());
- // If there are no new virtual methods and no new interfaces and no non-marker
- // interfaces in the superclass, we can simply reuse the vtable from superclass.
- // We may need to make a copy if it's embedded.
+ // Copy the IMT from superclass if present and needed. Update with new methods later.
+ bool is_klass_abstract = klass->IsAbstract();
+ bool is_super_abstract = super_class->IsAbstract();
+ DCHECK_EQ(klass->ShouldHaveImt(), !is_klass_abstract);
+ DCHECK_EQ(super_class->ShouldHaveImt(), !is_super_abstract);
+ if (!is_klass_abstract && !is_super_abstract) {
+ ImTable* super_imt = super_class->GetImt(kPointerSize);
+ for (size_t i = 0; i < ImTable::kSize; ++i) {
+ out_imt[i] = super_imt->Get(i, kPointerSize);
+ }
+ }
+
+ // 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 && iftable.Get() == super_class->GetIfTable()) {
- if (super_class->ShouldHaveEmbeddedVTable()) {
+ DCHECK_EQ(is_super_abstract, !super_class->ShouldHaveEmbeddedVTable());
+ if (is_super_abstract) {
+ DCHECK(super_class->IsAbstract() && !super_class->IsArrayClass());
+ ObjPtr<mirror::PointerArray> super_vtable = super_class->GetVTable();
+ CHECK(super_vtable != nullptr) << super_class->PrettyClass();
+ klass->SetVTable(super_vtable);
+ // No IMT in the super class, we need to reconstruct it from the iftable.
+ if (!is_klass_abstract && iftable->Count() != 0) {
+ class_linker_->FillIMTFromIfTable(iftable.Get(),
+ runtime_->GetImtUnimplementedMethod(),
+ runtime_->GetImtConflictMethod(),
+ klass.Get(),
+ /*create_conflict_tables=*/false,
+ /*ignore_copied_methods=*/false,
+ out_new_conflict,
+ out_imt);
+ }
+ } else {
ObjPtr<mirror::PointerArray> vtable =
class_linker_->AllocPointerArray(self, super_vtable_length);
if (UNLIKELY(vtable == nullptr)) {
@@ -8267,11 +8301,7 @@
i, super_class->GetEmbeddedVTableEntry(i, kPointerSize), kPointerSize);
}
klass->SetVTable(vtable);
- } else {
- DCHECK(super_class->IsAbstract() && !super_class->IsArrayClass());
- ObjPtr<mirror::PointerArray> super_vtable = super_class->GetVTable();
- CHECK(super_vtable != nullptr) << super_class->PrettyClass();
- klass->SetVTable(super_vtable);
+ // The IMT was already copied from superclass if `klass` is not abstract.
}
klass->SetIfTable(iftable.Get());
return true;
@@ -8286,8 +8316,8 @@
return false;
}
- size_t final_vtable_size =
- AssignVTableIndexes(klass.Get(), super_class.Get(), num_virtual_methods, iftable.Get());
+ size_t final_vtable_size = AssignVTableIndexes(
+ klass.Get(), super_class.Get(), is_super_abstract, num_virtual_methods, iftable.Get());
if (final_vtable_size == 0u) {
self->AssertPendingException();
return false;
@@ -8321,8 +8351,14 @@
}
}
- // Update the `iftable` with finalized virtual methods.
- FinalizeIfTable(iftable.Get(), vtable.Get(), out_new_conflict, out_imt);
+ // Update the `iftable` (and IMT) with finalized virtual methods.
+ FinalizeIfTable(klass.Get(),
+ iftable.Get(),
+ vtable.Get(),
+ is_klass_abstract,
+ is_super_abstract,
+ out_new_conflict,
+ out_imt);
klass->SetVTable(vtable.Get());
klass->SetIfTable(iftable.Get());
diff --git a/runtime/class_linker.h b/runtime/class_linker.h
index 06e6fca..ae83009 100644
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -1262,12 +1262,6 @@
/*out*/bool* new_conflict,
/*out*/ArtMethod** imt) REQUIRES_SHARED(Locks::mutator_lock_);
- void FillImtFromSuperClass(Handle<mirror::Class> klass,
- ArtMethod* unimplemented_method,
- ArtMethod* imt_conflict_method,
- bool* new_conflict,
- ArtMethod** imt) REQUIRES_SHARED(Locks::mutator_lock_);
-
// Check invoke type against the referenced class. Throws IncompatibleClassChangeError
// (if `kThrowOnError`) and returns true on mismatch (kInterface on a non-interface class,
// kVirtual on interface, kDefault on interface for dex files not supporting default methods),