Merge "Speed up verified methods"
diff --git a/compiler/dex/verification_results.cc b/compiler/dex/verification_results.cc
index 511a787..3fb10d8 100644
--- a/compiler/dex/verification_results.cc
+++ b/compiler/dex/verification_results.cc
@@ -31,49 +31,68 @@
VerificationResults::VerificationResults(const CompilerOptions* compiler_options)
: compiler_options_(compiler_options),
verified_methods_lock_("compiler verified methods lock"),
- verified_methods_(),
- rejected_classes_lock_("compiler rejected classes lock"),
- rejected_classes_() {
-}
+ rejected_classes_lock_("compiler rejected classes lock") {}
VerificationResults::~VerificationResults() {
- Thread* self = Thread::Current();
- {
- WriterMutexLock mu(self, verified_methods_lock_);
- STLDeleteValues(&verified_methods_);
- }
+ WriterMutexLock mu(Thread::Current(), verified_methods_lock_);
+ DeleteResults(preregistered_dex_files_);
+ STLDeleteValues(&verified_methods_);
}
void VerificationResults::ProcessVerifiedMethod(verifier::MethodVerifier* method_verifier) {
DCHECK(method_verifier != nullptr);
MethodReference ref = method_verifier->GetMethodReference();
bool compile = IsCandidateForCompilation(ref, method_verifier->GetAccessFlags());
- const VerifiedMethod* verified_method = VerifiedMethod::Create(method_verifier, compile);
+ std::unique_ptr<const VerifiedMethod> verified_method(
+ VerifiedMethod::Create(method_verifier, compile));
if (verified_method == nullptr) {
// We'll punt this later.
return;
}
-
- WriterMutexLock mu(Thread::Current(), verified_methods_lock_);
- auto it = verified_methods_.find(ref);
- if (it != verified_methods_.end()) {
+ bool inserted;
+ DexFileMethodArray* const array = GetMethodArray(ref.dex_file);
+ const VerifiedMethod* existing = nullptr;
+ if (array != nullptr) {
+ DCHECK(array != nullptr);
+ Atomic<const VerifiedMethod*>* slot = &(*array)[ref.dex_method_index];
+ inserted = slot->CompareExchangeStrongSequentiallyConsistent(nullptr, verified_method.get());
+ if (!inserted) {
+ existing = slot->LoadSequentiallyConsistent();
+ DCHECK_NE(verified_method.get(), existing);
+ }
+ } else {
+ WriterMutexLock mu(Thread::Current(), verified_methods_lock_);
+ auto it = verified_methods_.find(ref);
+ inserted = it == verified_methods_.end();
+ if (inserted) {
+ verified_methods_.Put(ref, verified_method.get());
+ DCHECK(verified_methods_.find(ref) != verified_methods_.end());
+ } else {
+ existing = it->second;
+ }
+ }
+ if (inserted) {
+ // Successfully added, release the unique_ptr since we no longer have ownership.
+ DCHECK_EQ(GetVerifiedMethod(ref), verified_method.get());
+ verified_method.release();
+ } else {
// TODO: Investigate why are we doing the work again for this method and try to avoid it.
LOG(WARNING) << "Method processed more than once: " << ref.PrettyMethod();
if (!Runtime::Current()->UseJitCompilation()) {
- DCHECK_EQ(it->second->GetDevirtMap().size(), verified_method->GetDevirtMap().size());
- DCHECK_EQ(it->second->GetSafeCastSet().size(), verified_method->GetSafeCastSet().size());
+ DCHECK_EQ(existing->GetDevirtMap().size(), verified_method->GetDevirtMap().size());
+ DCHECK_EQ(existing->GetSafeCastSet().size(), verified_method->GetSafeCastSet().size());
}
- // Delete the new verified method since there was already an existing one registered. It
- // is unsafe to replace the existing one since the JIT may be using it to generate a
- // native GC map.
- delete verified_method;
- return;
+ // Let the unique_ptr delete the new verified method since there was already an existing one
+ // registered. It is unsafe to replace the existing one since the JIT may be using it to
+ // generate a native GC map.
}
- verified_methods_.Put(ref, verified_method);
- DCHECK(verified_methods_.find(ref) != verified_methods_.end());
}
const VerifiedMethod* VerificationResults::GetVerifiedMethod(MethodReference ref) {
+ DexFileMethodArray* array = GetMethodArray(ref.dex_file);
+ if (array != nullptr) {
+ return (*array)[ref.dex_method_index].LoadRelaxed();
+ }
ReaderMutexLock mu(Thread::Current(), verified_methods_lock_);
auto it = verified_methods_.find(ref);
return (it != verified_methods_.end()) ? it->second : nullptr;
@@ -105,4 +124,42 @@
return true;
}
+void VerificationResults::PreRegisterDexFile(const DexFile* dex_file) {
+ CHECK(preregistered_dex_files_.find(dex_file) == preregistered_dex_files_.end())
+ << dex_file->GetLocation();
+ DexFileMethodArray array(dex_file->NumMethodIds());
+ WriterMutexLock mu(Thread::Current(), verified_methods_lock_);
+ // There can be some verified methods that are already registered for the dex_file since we set
+ // up well known classes earlier. Remove these and put them in the array so that we don't
+ // accidentally miss seeing them.
+ for (auto it = verified_methods_.begin(); it != verified_methods_.end(); ) {
+ MethodReference ref = it->first;
+ if (ref.dex_file == dex_file) {
+ array[ref.dex_method_index].StoreSequentiallyConsistent(it->second);
+ it = verified_methods_.erase(it);
+ } else {
+ ++it;
+ }
+ }
+ preregistered_dex_files_.emplace(dex_file, std::move(array));
+}
+
+void VerificationResults::DeleteResults(DexFileResults& array) {
+ for (auto& pair : array) {
+ for (Atomic<const VerifiedMethod*>& method : pair.second) {
+ delete method.LoadSequentiallyConsistent();
+ }
+ }
+ array.clear();
+}
+
+VerificationResults::DexFileMethodArray* VerificationResults::GetMethodArray(
+ const DexFile* dex_file) {
+ auto it = preregistered_dex_files_.find(dex_file);
+ if (it != preregistered_dex_files_.end()) {
+ return &it->second;
+ }
+ return nullptr;
+}
+
} // namespace art
diff --git a/compiler/dex/verification_results.h b/compiler/dex/verification_results.h
index 6afd1ab..b3356e0 100644
--- a/compiler/dex/verification_results.h
+++ b/compiler/dex/verification_results.h
@@ -19,8 +19,8 @@
#include <stdint.h>
#include <set>
-#include <vector>
+#include "base/dchecked_vector.h"
#include "base/macros.h"
#include "base/mutex.h"
#include "class_reference.h"
@@ -38,35 +38,48 @@
// Used by CompilerCallbacks to track verification information from the Runtime.
class VerificationResults {
- public:
- explicit VerificationResults(const CompilerOptions* compiler_options);
- ~VerificationResults();
+ public:
+ explicit VerificationResults(const CompilerOptions* compiler_options);
+ ~VerificationResults();
- void ProcessVerifiedMethod(verifier::MethodVerifier* method_verifier)
- REQUIRES_SHARED(Locks::mutator_lock_)
- REQUIRES(!verified_methods_lock_);
+ void ProcessVerifiedMethod(verifier::MethodVerifier* method_verifier)
+ REQUIRES_SHARED(Locks::mutator_lock_)
+ REQUIRES(!verified_methods_lock_);
- const VerifiedMethod* GetVerifiedMethod(MethodReference ref)
- REQUIRES(!verified_methods_lock_);
+ const VerifiedMethod* GetVerifiedMethod(MethodReference ref)
+ REQUIRES(!verified_methods_lock_);
- void AddRejectedClass(ClassReference ref) REQUIRES(!rejected_classes_lock_);
- bool IsClassRejected(ClassReference ref) REQUIRES(!rejected_classes_lock_);
+ void AddRejectedClass(ClassReference ref) REQUIRES(!rejected_classes_lock_);
+ bool IsClassRejected(ClassReference ref) REQUIRES(!rejected_classes_lock_);
- bool IsCandidateForCompilation(MethodReference& method_ref,
- const uint32_t access_flags);
+ bool IsCandidateForCompilation(MethodReference& method_ref, const uint32_t access_flags);
- private:
- const CompilerOptions* const compiler_options_;
+ // Add a dex file array to the preregistered_dex_files_ array. These dex files require no locks to
+ // access. It is not safe to call if other callers are calling GetVerifiedMethod concurrently.
+ void PreRegisterDexFile(const DexFile* dex_file) REQUIRES(!verified_methods_lock_);
- // Verified methods.
- typedef SafeMap<MethodReference, const VerifiedMethod*,
- MethodReferenceComparator> VerifiedMethodMap;
- ReaderWriterMutex verified_methods_lock_ DEFAULT_MUTEX_ACQUIRED_AFTER;
- VerifiedMethodMap verified_methods_ GUARDED_BY(verified_methods_lock_);
+ private:
+ // Verified methods. The method array is fixed to avoid needing a lock to extend it.
+ using DexFileMethodArray = dchecked_vector<Atomic<const VerifiedMethod*>>;
+ using DexFileResults = std::map<const DexFile*, DexFileMethodArray>;
+ using VerifiedMethodMap = SafeMap<MethodReference,
+ const VerifiedMethod*,
+ MethodReferenceComparator>;
- // Rejected classes.
- ReaderWriterMutex rejected_classes_lock_ DEFAULT_MUTEX_ACQUIRED_AFTER;
- std::set<ClassReference> rejected_classes_ GUARDED_BY(rejected_classes_lock_);
+ static void DeleteResults(DexFileResults& array);
+
+ DexFileMethodArray* GetMethodArray(const DexFile* dex_file) REQUIRES(!verified_methods_lock_);
+ VerifiedMethodMap verified_methods_ GUARDED_BY(verified_methods_lock_);
+ const CompilerOptions* const compiler_options_;
+
+ // Dex2oat can preregister dex files to avoid locking when calling GetVerifiedMethod.
+ DexFileResults preregistered_dex_files_;
+
+ ReaderWriterMutex verified_methods_lock_ DEFAULT_MUTEX_ACQUIRED_AFTER;
+
+ // Rejected classes.
+ ReaderWriterMutex rejected_classes_lock_ DEFAULT_MUTEX_ACQUIRED_AFTER;
+ std::set<ClassReference> rejected_classes_ GUARDED_BY(rejected_classes_lock_);
};
} // namespace art
diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index 20b5bba..81baa80 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -1588,6 +1588,9 @@
dex_caches_.push_back(soa.AddLocalReference<jobject>(
class_linker->RegisterDexFile(*dex_file,
soa.Decode<mirror::ClassLoader>(class_loader_).Ptr())));
+ // Pre-register dex files so that we can access verification results without locks during
+ // compilation and verification.
+ verification_results_->PreRegisterDexFile(dex_file);
}
return true;
diff --git a/runtime/verifier/verifier_deps.cc b/runtime/verifier/verifier_deps.cc
index fefe5a3..01af5ec 100644
--- a/runtime/verifier/verifier_deps.cc
+++ b/runtime/verifier/verifier_deps.cc
@@ -84,16 +84,18 @@
ObjPtr<mirror::Class> klass) {
DCHECK(klass != nullptr);
ObjPtr<mirror::DexCache> dex_cache = klass->GetDexCache();
- // Array classes do not have a dex cache.
+ // Array and proxy classes do not have a dex cache.
if (!klass->IsArrayClass() && !klass->IsProxyClass()) {
DCHECK(dex_cache != nullptr) << klass->PrettyClass();
if (dex_cache->GetDexFile() == &dex_file) {
// FindStringId is slow, try to go through the class def if we have one.
const DexFile::ClassDef* class_def = klass->GetClassDef();
DCHECK(class_def != nullptr) << klass->PrettyClass();
- std::string temp;
const DexFile::TypeId& type_id = dex_file.GetTypeId(class_def->class_idx_);
- DCHECK_EQ(GetIdFromString(dex_file, klass->GetDescriptor(&temp)), type_id.descriptor_idx_);
+ if (kIsDebugBuild) {
+ std::string temp;
+ CHECK_EQ(GetIdFromString(dex_file, klass->GetDescriptor(&temp)), type_id.descriptor_idx_);
+ }
return type_id.descriptor_idx_;
}
}