Speed up verified methods
Switch to an array of atomic pointers instead of a map. Removes lock
and map lookup. Also address comments from previous CL.
GetVerifiedMethod: 1.59% -> 0.18% of compilation time.
Install time seems to goes down by around 1%.
Also has significant RAM savings (FB host compile):
dex2oat native alloc: 84695472B -> 71268736B
For the JIT case, the old method is used to prevent any increase in
RAM usage.
Bug: 32641252
Test: test-art-host
Change-Id: I47b4b8a4a3cb3f8ef23e36a888b8885e12168787
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