From 151f2214d95f6003fe067fa2ebcd8ddad11e735c Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Tue, 6 May 2014 11:27:27 -0700 Subject: Improve ValidateSuperClassDescriptors performance. ValidateSuperClassDescriptors uses FindClass with the 2 class loaders that are being used in validating method parameter types. The use of 2 class loaders ensures at least one miss with LookupClass and thereby a call to ClassLoader.loadClass which will then defer to the parent class loader eating time. This change modifies the behavior to instead lookup types with a dex cache, so that resolution and load class are only performed once per type. Bug: 12804658 Change-Id: Ia7be1f7bab8175a6934fd59fc54e0829beed0198 --- runtime/class_linker.cc | 93 +++++++++++-------------------------------------- runtime/object_utils.h | 30 ++++++++++++++-- 2 files changed, 49 insertions(+), 74 deletions(-) diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index dbea0d854..e3c162bde 100644 --- a/runtime/class_linker.cc +++ b/runtime/class_linker.cc @@ -3309,33 +3309,36 @@ bool ClassLinker::ValidateSuperClassDescriptors(const SirtRef& kl if (klass->IsInterface()) { return true; } - Thread* self = Thread::Current(); - // begin with the methods local to the superclass + // Begin with the methods local to the superclass. + MethodHelper mh; + MethodHelper super_mh; if (klass->HasSuperClass() && klass->GetClassLoader() != klass->GetSuperClass()->GetClassLoader()) { - SirtRef super(self, klass->GetSuperClass()); - for (int i = super->GetVTable()->GetLength() - 1; i >= 0; --i) { - mirror::ArtMethod* method = klass->GetVTable()->Get(i); - if (method != super->GetVTable()->Get(i) && - !IsSameMethodSignatureInDifferentClassContexts(self, method, super.get(), klass.get())) { + for (int i = klass->GetSuperClass()->GetVTable()->GetLength() - 1; i >= 0; --i) { + mh.ChangeMethod(klass->GetVTable()->GetWithoutChecks(i)); + super_mh.ChangeMethod(klass->GetSuperClass()->GetVTable()->GetWithoutChecks(i)); + bool is_override = mh.GetMethod() != super_mh.GetMethod(); + if (is_override && !mh.HasSameSignatureWithDifferentClassLoaders(&super_mh)) { ThrowLinkageError(klass.get(), "Class %s method %s resolves differently in superclass %s", - PrettyDescriptor(klass.get()).c_str(), PrettyMethod(method).c_str(), - PrettyDescriptor(super.get()).c_str()); + PrettyDescriptor(klass.get()).c_str(), + PrettyMethod(mh.GetMethod()).c_str(), + PrettyDescriptor(klass->GetSuperClass()).c_str()); return false; } } } for (int32_t i = 0; i < klass->GetIfTableCount(); ++i) { - SirtRef interface(self, klass->GetIfTable()->GetInterface(i)); - if (klass->GetClassLoader() != interface->GetClassLoader()) { - for (size_t j = 0; j < interface->NumVirtualMethods(); ++j) { - mirror::ArtMethod* method = klass->GetIfTable()->GetMethodArray(i)->Get(j); - if (!IsSameMethodSignatureInDifferentClassContexts(self, method, interface.get(), - method->GetDeclaringClass())) { + if (klass->GetClassLoader() != klass->GetIfTable()->GetInterface(i)->GetClassLoader()) { + uint32_t num_methods = klass->GetIfTable()->GetInterface(i)->NumVirtualMethods(); + for (uint32_t j = 0; j < num_methods; ++j) { + mh.ChangeMethod(klass->GetIfTable()->GetMethodArray(i)->GetWithoutChecks(j)); + super_mh.ChangeMethod(klass->GetIfTable()->GetInterface(i)->GetVirtualMethod(j)); + bool is_override = mh.GetMethod() != super_mh.GetMethod(); + if (is_override && !mh.HasSameSignatureWithDifferentClassLoaders(&super_mh)) { ThrowLinkageError(klass.get(), "Class %s method %s resolves differently in interface %s", - PrettyDescriptor(method->GetDeclaringClass()).c_str(), - PrettyMethod(method).c_str(), - PrettyDescriptor(interface.get()).c_str()); + PrettyDescriptor(klass.get()).c_str(), + PrettyMethod(mh.GetMethod()).c_str(), + PrettyDescriptor(klass->GetIfTable()->GetInterface(i)).c_str()); return false; } } @@ -3344,60 +3347,6 @@ bool ClassLinker::ValidateSuperClassDescriptors(const SirtRef& kl return true; } -// Returns true if classes referenced by the signature of the method are the -// same classes in klass1 as they are in klass2. -bool ClassLinker::IsSameMethodSignatureInDifferentClassContexts(Thread* self, - mirror::ArtMethod* method, - mirror::Class* klass1, - mirror::Class* klass2) { - if (klass1 == klass2) { - return true; - } - CHECK(klass1 != nullptr); - CHECK(klass2 != nullptr); - SirtRef loader1(self, klass1->GetClassLoader()); - SirtRef loader2(self, klass2->GetClassLoader()); - const DexFile& dex_file = *method->GetDeclaringClass()->GetDexCache()->GetDexFile(); - const DexFile::ProtoId& proto_id = - dex_file.GetMethodPrototype(dex_file.GetMethodId(method->GetDexMethodIndex())); - for (DexFileParameterIterator it(dex_file, proto_id); it.HasNext(); it.Next()) { - const char* descriptor = it.GetDescriptor(); - if (descriptor == nullptr) { - break; - } - if (descriptor[0] == 'L' || descriptor[0] == '[') { - // Found a non-primitive type. - if (!IsSameDescriptorInDifferentClassContexts(self, descriptor, loader1, loader2)) { - return false; - } - } - } - // Check the return type - const char* descriptor = dex_file.GetReturnTypeDescriptor(proto_id); - if (descriptor[0] == 'L' || descriptor[0] == '[') { - if (!IsSameDescriptorInDifferentClassContexts(self, descriptor, loader1, loader2)) { - return false; - } - } - return true; -} - -// Returns true if the descriptor resolves to the same class in the context of loader1 and loader2. -bool ClassLinker::IsSameDescriptorInDifferentClassContexts(Thread* self, const char* descriptor, - SirtRef& loader1, - SirtRef& loader2) { - CHECK(descriptor != nullptr); - SirtRef found1(self, FindClass(self, descriptor, loader1)); - if (found1.get() == nullptr) { - self->ClearException(); - } - mirror::Class* found2 = FindClass(self, descriptor, loader2); - if (found2 == nullptr) { - self->ClearException(); - } - return found1.get() == found2; -} - bool ClassLinker::EnsureInitialized(const SirtRef& c, bool can_init_fields, bool can_init_parents) { DCHECK(c.get() != NULL); diff --git a/runtime/object_utils.h b/runtime/object_utils.h index 072f074e8..504537a66 100644 --- a/runtime/object_utils.h +++ b/runtime/object_utils.h @@ -520,8 +520,7 @@ class MethodHelper { return GetParamPrimitiveType(param) == Primitive::kPrimNot; } - bool HasSameNameAndSignature(MethodHelper* other) - SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { + bool HasSameNameAndSignature(MethodHelper* other) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { const DexFile& dex_file = GetDexFile(); const DexFile::MethodId& mid = dex_file.GetMethodId(method_->GetDexMethodIndex()); if (GetDexCache() == other->GetDexCache()) { @@ -539,6 +538,33 @@ class MethodHelper { return dex_file.GetMethodSignature(mid) == other_dex_file.GetMethodSignature(other_mid); } + bool HasSameSignatureWithDifferentClassLoaders(MethodHelper* other) + SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { + if (UNLIKELY(GetReturnType() != other->GetReturnType())) { + return false; + } + const DexFile::TypeList* types = GetParameterTypeList(); + const DexFile::TypeList* other_types = other->GetParameterTypeList(); + if (types == nullptr) { + return (other_types == nullptr) || (other_types->Size() == 0); + } else if (UNLIKELY(other_types == nullptr)) { + return types->Size() == 0; + } + uint32_t num_types = types->Size(); + if (UNLIKELY(num_types != other_types->Size())) { + return false; + } + for (uint32_t i = 0; i < num_types; ++i) { + mirror::Class* param_type = GetClassFromTypeIdx(types->GetTypeItem(i).type_idx_); + mirror::Class* other_param_type = + other->GetClassFromTypeIdx(other_types->GetTypeItem(i).type_idx_); + if (UNLIKELY(param_type != other_param_type)) { + return false; + } + } + return true; + } + const DexFile::CodeItem* GetCodeItem() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { return GetDexFile().GetCodeItem(method_->GetCodeItemOffset()); -- cgit v1.2.3