Tidy MethodProtoHelper.
Move to place of only use, class_linker.cc. Be lazy in computing the name.
Before:
WaitTime: 2699
WaitTime: 2791
WaitTime: 2653
WaitTime: 2929
WaitTime: 2651
WaitTime: 2971
After:
WaitTime: 2749
WaitTime: 2786
WaitTime: 2852
WaitTime: 2856
WaitTime: 2703
WaitTime: 2784
Bug: 18054905
Bug: 16828525
(cherry picked from commit 03b6eafba8ace9a9c4d5ee9c47723d1910ccd7a8)
Change-Id: I1438efbda58369ddd0ac36eda8a5a0a6c6fdff77
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index c0cccd6..e178da5 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -4575,6 +4575,49 @@
return LinkInterfaceMethods(self, klass, interfaces, out_imt); // Link interface method last.
}
+// Comparator for name and signature of a method, used in finding overriding methods. Implementation
+// avoids the use of handles, if it didn't then rather than compare dex files we could compare dex
+// caches in the implementation below.
+class MethodNameAndSignatureComparator FINAL {
+ public:
+ explicit MethodNameAndSignatureComparator(mirror::ArtMethod* method)
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) :
+ dex_file_(method->GetDexFile()), mid_(&dex_file_->GetMethodId(method->GetDexMethodIndex())),
+ name_(nullptr), name_len_(0) {
+ DCHECK(!method->IsProxyMethod()) << PrettyMethod(method);
+ }
+
+ bool HasSameNameAndSignature(mirror::ArtMethod* other)
+ SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
+ DCHECK(!other->IsProxyMethod()) << PrettyMethod(other);
+ const DexFile* other_dex_file = other->GetDexFile();
+ const DexFile::MethodId& other_mid = other_dex_file->GetMethodId(other->GetDexMethodIndex());
+ if (dex_file_ == other_dex_file) {
+ return mid_->name_idx_ == other_mid.name_idx_ && mid_->proto_idx_ == other_mid.proto_idx_;
+ }
+ if (name_ == nullptr) {
+ name_ = dex_file_->StringDataAndUtf16LengthByIdx(mid_->name_idx_, &name_len_);
+ }
+ uint32_t other_name_len;
+ const char* other_name = other_dex_file->StringDataAndUtf16LengthByIdx(other_mid.name_idx_,
+ &other_name_len);
+ if (name_len_ != other_name_len || strcmp(name_, other_name) != 0) {
+ return false;
+ }
+ return dex_file_->GetMethodSignature(*mid_) == other_dex_file->GetMethodSignature(other_mid);
+ }
+
+ private:
+ // Dex file for the method to compare against.
+ const DexFile* const dex_file_;
+ // MethodId for the method to compare against.
+ const DexFile::MethodId* const mid_;
+ // Lazily computed name from the dex file's strings.
+ const char* name_;
+ // Lazily computed name length.
+ uint32_t name_len_;
+};
+
bool ClassLinker::LinkVirtualMethods(Thread* self, Handle<mirror::Class> klass) {
const size_t num_virtual_methods = klass->NumVirtualMethods();
if (klass->HasSuperClass()) {
@@ -4605,12 +4648,15 @@
// See if any of our virtual methods override the superclass.
for (size_t i = 0; i < num_virtual_methods; ++i) {
mirror::ArtMethod* local_method = klass->GetVirtualMethodDuringLinking(i);
- MethodProtoHelper local_helper(local_method);
+ MethodNameAndSignatureComparator
+ virtual_method_name_comparator(local_method->GetInterfaceMethodIfProxy());
size_t j = 0;
for (; j < actual_count; ++j) {
mirror::ArtMethod* super_method = vtable->GetWithoutChecks(j);
- MethodProtoHelper super_helper(super_method);
- if (local_helper.HasSameNameAndSignature(super_helper)) {
+ if (super_method->GetDeclaringClass() == klass.Get()) {
+ continue; // A previously overridden method.
+ }
+ if (virtual_method_name_comparator.HasSameNameAndSignature(super_method)) {
if (klass->CanAccessMember(super_method->GetDeclaringClass(),
super_method->GetAccessFlags())) {
if (super_method->IsFinal()) {
@@ -4859,7 +4905,7 @@
}
for (size_t j = 0; j < num_methods; ++j) {
mirror::ArtMethod* interface_method = iftable->GetInterface(i)->GetVirtualMethod(j);
- MethodProtoHelper interface_helper(interface_method);
+ MethodNameAndSignatureComparator interface_name_comparator(interface_method);
int32_t k;
// For each method listed in the interface's method list, find the
// matching method in our class's method list. We want to favor the
@@ -4871,8 +4917,10 @@
// matter which direction we go. We walk it backward anyway.)
for (k = input_array->GetLength() - 1; k >= 0; --k) {
mirror::ArtMethod* vtable_method = input_array->GetWithoutChecks(k);
- MethodProtoHelper vtable_helper(vtable_method);
- if (interface_helper.HasSameNameAndSignature(vtable_helper)) {
+ mirror::ArtMethod* vtable_method_for_name_comparison =
+ vtable_method->GetInterfaceMethodIfProxy();
+ if (interface_name_comparator.HasSameNameAndSignature(
+ vtable_method_for_name_comparison)) {
if (!vtable_method->IsAbstract() && !vtable_method->IsPublic()) {
ThrowIllegalAccessError(
klass.Get(),
@@ -4891,7 +4939,10 @@
} else if (imt_ref != conflict_method) {
// If we are not a conflict and we have the same signature and name as the imt entry,
// it must be that we overwrote a superclass vtable entry.
- if (MethodProtoHelper(imt_ref).HasSameNameAndSignature(vtable_helper)) {
+ MethodNameAndSignatureComparator
+ imt_ref_name_comparator(imt_ref->GetInterfaceMethodIfProxy());
+ if (imt_ref_name_comparator.HasSameNameAndSignature(
+ vtable_method_for_name_comparison)) {
out_imt->SetReference(imt_index, vtable_method);
} else {
out_imt->SetReference(imt_index, conflict_method);
@@ -4903,8 +4954,7 @@
if (k < 0 && !super_interface) {
mirror::ArtMethod* miranda_method = nullptr;
for (mirror::ArtMethod* mir_method : miranda_list) {
- MethodProtoHelper vtable_helper(mir_method);
- if (interface_helper.HasSameNameAndSignature(vtable_helper)) {
+ if (interface_name_comparator.HasSameNameAndSignature(mir_method)) {
miranda_method = mir_method;
break;
}
diff --git a/runtime/method_helper-inl.h b/runtime/method_helper-inl.h
index e680307..9af835f 100644
--- a/runtime/method_helper-inl.h
+++ b/runtime/method_helper-inl.h
@@ -74,23 +74,6 @@
return s;
}
-inline MethodProtoHelper::MethodProtoHelper(mirror::ArtMethod* method) {
- method = method->GetInterfaceMethodIfProxy();
- dex_file_ = method->GetDexFile();
- mid_ = &dex_file_->GetMethodId(method->GetDexMethodIndex());
- name_ = dex_file_->StringDataAndUtf16LengthByIdx(mid_->name_idx_, &name_len_);
-}
-
-inline bool MethodProtoHelper::HasSameNameAndSignature(const MethodProtoHelper& other) const {
- if (name_len_ != other.name_len_ || strcmp(name_, other.name_) != 0) {
- return false;
- }
- if (dex_file_ == other.dex_file_) {
- return mid_->name_idx_ == other.mid_->name_idx_ && mid_->proto_idx_ == other.mid_->proto_idx_;
- }
- return dex_file_->GetMethodSignature(*mid_) == other.dex_file_->GetMethodSignature(*other.mid_);
-}
-
} // namespace art
#endif // ART_RUNTIME_METHOD_HELPER_INL_H_
diff --git a/runtime/method_helper.h b/runtime/method_helper.h
index 37295c0..f71d273 100644
--- a/runtime/method_helper.h
+++ b/runtime/method_helper.h
@@ -24,19 +24,6 @@
namespace art {
-class MethodProtoHelper {
- public:
- ALWAYS_INLINE MethodProtoHelper(mirror::ArtMethod* method)
- SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
- ALWAYS_INLINE bool HasSameNameAndSignature(const MethodProtoHelper& other) const;
-
- private:
- const DexFile* dex_file_;
- const DexFile::MethodId* mid_;
- const char* name_;
- uint32_t name_len_;
-};
-
class MethodHelper {
public:
explicit MethodHelper(Handle<mirror::ArtMethod> m) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
diff --git a/runtime/mirror/art_method-inl.h b/runtime/mirror/art_method-inl.h
index 1af78f3..e0912dd 100644
--- a/runtime/mirror/art_method-inl.h
+++ b/runtime/mirror/art_method-inl.h
@@ -527,11 +527,15 @@
return GetInterfaceMethodIfProxy()->GetDeclaringClass()->GetDexCache();
}
+inline bool ArtMethod::IsProxyMethod() {
+ return GetDeclaringClass()->IsProxyClass();
+}
+
inline ArtMethod* ArtMethod::GetInterfaceMethodIfProxy() {
- mirror::Class* klass = GetDeclaringClass();
- if (LIKELY(!klass->IsProxyClass())) {
+ if (LIKELY(!IsProxyMethod())) {
return this;
}
+ mirror::Class* klass = GetDeclaringClass();
mirror::ArtMethod* interface_method = GetDexCacheResolvedMethods()->Get(GetDexMethodIndex());
DCHECK(interface_method != nullptr);
DCHECK_EQ(interface_method,
diff --git a/runtime/mirror/art_method.cc b/runtime/mirror/art_method.cc
index bf28287..5b833b9 100644
--- a/runtime/mirror/art_method.cc
+++ b/runtime/mirror/art_method.cc
@@ -103,10 +103,6 @@
return num_registers;
}
-bool ArtMethod::IsProxyMethod() {
- return GetDeclaringClass()->IsProxyClass();
-}
-
ArtMethod* ArtMethod::FindOverriddenMethod() {
if (IsStatic()) {
return NULL;
diff --git a/runtime/mirror/class.h b/runtime/mirror/class.h
index 2d536ad..5b87cbd 100644
--- a/runtime/mirror/class.h
+++ b/runtime/mirror/class.h
@@ -1146,11 +1146,11 @@
// The following data exist in real class objects.
// Embedded Imtable, for class object that's not an interface, fixed size.
- ImTableEntry embedded_imtable_[0];
+ // ImTableEntry embedded_imtable_[0];
// Embedded Vtable, for class object that's not an interface, variable size.
- VTableEntry embedded_vtable_[0];
+ // VTableEntry embedded_vtable_[0];
// Static fields, variable size.
- uint32_t fields_[0];
+ // uint32_t fields_[0];
// java.lang.Class
static GcRoot<Class> java_lang_Class_;
