diff options
| -rw-r--r-- | build/Android.gtest.mk | 1 | ||||
| -rw-r--r-- | compiler/Android.bp | 1 | ||||
| -rw-r--r-- | compiler/dex/verification_results.cc | 57 | ||||
| -rw-r--r-- | compiler/dex/verification_results.h | 17 | ||||
| -rw-r--r-- | compiler/utils/atomic_method_ref_map-inl.h | 83 | ||||
| -rw-r--r-- | compiler/utils/atomic_method_ref_map.h | 67 | ||||
| -rw-r--r-- | compiler/utils/atomic_method_ref_map_test.cc | 69 | ||||
| -rw-r--r-- | dex2oat/dex2oat.cc | 2 |
8 files changed, 250 insertions, 47 deletions
diff --git a/build/Android.gtest.mk b/build/Android.gtest.mk index 990262844c..1691dbb3bc 100644 --- a/build/Android.gtest.mk +++ b/build/Android.gtest.mk @@ -84,6 +84,7 @@ $(ART_TEST_TARGET_GTEST_VerifierDeps_DEX): $(ART_TEST_GTEST_VerifierDeps_SRC) $( # Dex file dependencies for each gtest. ART_GTEST_dex2oat_environment_tests_DEX_DEPS := Main MainStripped MultiDex MultiDexModifiedSecondary Nested +ART_GTEST_atomic_method_ref_map_test_DEX_DEPS := Interfaces ART_GTEST_class_linker_test_DEX_DEPS := Interfaces MethodTypes MultiDex MyClass Nested Statics StaticsFromCode ART_GTEST_compiler_driver_test_DEX_DEPS := AbstractMethod StaticLeafMethods ProfileTestMultiDex ART_GTEST_dex_cache_test_DEX_DEPS := Main Packages MethodTypes diff --git a/compiler/Android.bp b/compiler/Android.bp index b883e0881a..db55ea0ef7 100644 --- a/compiler/Android.bp +++ b/compiler/Android.bp @@ -349,6 +349,7 @@ art_cc_test { "optimizing/ssa_test.cc", "optimizing/stack_map_test.cc", "optimizing/suspend_check_test.cc", + "utils/atomic_method_ref_map_test.cc", "utils/dedupe_set_test.cc", "utils/intrusive_forward_list_test.cc", "utils/string_reference_test.cc", diff --git a/compiler/dex/verification_results.cc b/compiler/dex/verification_results.cc index 3fb10d89dd..669d8cd991 100644 --- a/compiler/dex/verification_results.cc +++ b/compiler/dex/verification_results.cc @@ -23,6 +23,7 @@ #include "driver/compiler_options.h" #include "thread.h" #include "thread-inl.h" +#include "utils/atomic_method_ref_map-inl.h" #include "verified_method.h" #include "verifier/method_verifier-inl.h" @@ -35,8 +36,11 @@ VerificationResults::VerificationResults(const CompilerOptions* compiler_options VerificationResults::~VerificationResults() { WriterMutexLock mu(Thread::Current(), verified_methods_lock_); - DeleteResults(preregistered_dex_files_); STLDeleteValues(&verified_methods_); + atomic_verified_methods_.Visit([](const MethodReference& ref ATTRIBUTE_UNUSED, + const VerifiedMethod* method) { + delete method; + }); } void VerificationResults::ProcessVerifiedMethod(verifier::MethodVerifier* method_verifier) { @@ -49,16 +53,17 @@ void VerificationResults::ProcessVerifiedMethod(verifier::MethodVerifier* method // We'll punt this later. return; } - bool inserted; - DexFileMethodArray* const array = GetMethodArray(ref.dex_file); + AtomicMap::InsertResult result = atomic_verified_methods_.Insert(ref, + /*expected*/ nullptr, + verified_method.get()); 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()); + bool inserted; + if (result != AtomicMap::kInsertResultInvalidDexFile) { + inserted = (result == AtomicMap::kInsertResultSuccess); if (!inserted) { - existing = slot->LoadSequentiallyConsistent(); - DCHECK_NE(verified_method.get(), existing); + // Rare case. + CHECK(atomic_verified_methods_.Get(ref, &existing)); + CHECK_NE(verified_method.get(), existing); } } else { WriterMutexLock mu(Thread::Current(), verified_methods_lock_); @@ -89,9 +94,9 @@ void VerificationResults::ProcessVerifiedMethod(verifier::MethodVerifier* method } const VerifiedMethod* VerificationResults::GetVerifiedMethod(MethodReference ref) { - DexFileMethodArray* array = GetMethodArray(ref.dex_file); - if (array != nullptr) { - return (*array)[ref.dex_method_index].LoadRelaxed(); + const VerifiedMethod* ret = nullptr; + if (atomic_verified_methods_.Get(ref, &ret)) { + return ret; } ReaderMutexLock mu(Thread::Current(), verified_methods_lock_); auto it = verified_methods_.find(ref); @@ -124,10 +129,8 @@ bool VerificationResults::IsCandidateForCompilation(MethodReference&, 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()); +void VerificationResults::AddDexFile(const DexFile* dex_file) { + atomic_verified_methods_.AddDexFile(dex_file); 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 @@ -135,31 +138,13 @@ void VerificationResults::PreRegisterDexFile(const DexFile* dex_file) { 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); + CHECK(atomic_verified_methods_.Insert(ref, nullptr, it->second) == + AtomicMap::kInsertResultSuccess); 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 b3356e0e10..ea38f4d537 100644 --- a/compiler/dex/verification_results.h +++ b/compiler/dex/verification_results.h @@ -26,6 +26,7 @@ #include "class_reference.h" #include "method_reference.h" #include "safe_map.h" +#include "utils/atomic_method_ref_map.h" namespace art { @@ -54,26 +55,22 @@ class VerificationResults { bool IsCandidateForCompilation(MethodReference& method_ref, const uint32_t access_flags); - // 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_); + // Add a dex file to enable using the atomic map. + void AddDexFile(const DexFile* dex_file) REQUIRES(!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 AtomicMap = AtomicMethodRefMap<const VerifiedMethod*>; using VerifiedMethodMap = SafeMap<MethodReference, const VerifiedMethod*, MethodReferenceComparator>; - 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_; + // Dex2oat can add dex files to atomic_verified_methods_ to avoid locking when calling + // GetVerifiedMethod. + AtomicMap atomic_verified_methods_; ReaderWriterMutex verified_methods_lock_ DEFAULT_MUTEX_ACQUIRED_AFTER; diff --git a/compiler/utils/atomic_method_ref_map-inl.h b/compiler/utils/atomic_method_ref_map-inl.h new file mode 100644 index 0000000000..70ea028b17 --- /dev/null +++ b/compiler/utils/atomic_method_ref_map-inl.h @@ -0,0 +1,83 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ART_COMPILER_UTILS_ATOMIC_METHOD_REF_MAP_INL_H_ +#define ART_COMPILER_UTILS_ATOMIC_METHOD_REF_MAP_INL_H_ + +#include "atomic_method_ref_map.h" + +#include "dex_file-inl.h" + +namespace art { + +template <typename T> +inline typename AtomicMethodRefMap<T>::InsertResult AtomicMethodRefMap<T>::Insert( + MethodReference ref, + const T& expected, + const T& desired) { + ElementArray* const array = GetArray(ref.dex_file); + if (array == nullptr) { + return kInsertResultInvalidDexFile; + } + return (*array)[ref.dex_method_index].CompareExchangeStrongSequentiallyConsistent( + expected, desired) + ? kInsertResultSuccess + : kInsertResultCASFailure; +} + +template <typename T> +inline bool AtomicMethodRefMap<T>::Get(MethodReference ref, T* out) const { + const ElementArray* const array = GetArray(ref.dex_file); + if (array == nullptr) { + return kInsertResultInvalidDexFile; + } + *out = (*array)[ref.dex_method_index].LoadRelaxed(); + return true; +} + +template <typename T> +inline void AtomicMethodRefMap<T>::AddDexFile(const DexFile* dex_file) { + arrays_.Put(dex_file, std::move(ElementArray(dex_file->NumMethodIds()))); +} + +template <typename T> +inline typename AtomicMethodRefMap<T>::ElementArray* AtomicMethodRefMap<T>::GetArray( + const DexFile* dex_file) { + auto it = arrays_.find(dex_file); + return (it != arrays_.end()) ? &it->second : nullptr; +} + +template <typename T> +inline const typename AtomicMethodRefMap<T>::ElementArray* AtomicMethodRefMap<T>::GetArray( + const DexFile* dex_file) const { + auto it = arrays_.find(dex_file); + return (it != arrays_.end()) ? &it->second : nullptr; +} + +template <typename T> template <typename Visitor> +inline void AtomicMethodRefMap<T>::Visit(const Visitor& visitor) { + for (auto& pair : arrays_) { + const DexFile* dex_file = pair.first; + const ElementArray& elements = pair.second; + for (size_t i = 0; i < elements.size(); ++i) { + visitor(MethodReference(dex_file, i), elements[i].LoadRelaxed()); + } + } +} + +} // namespace art + +#endif // ART_COMPILER_UTILS_ATOMIC_METHOD_REF_MAP_INL_H_ diff --git a/compiler/utils/atomic_method_ref_map.h b/compiler/utils/atomic_method_ref_map.h new file mode 100644 index 0000000000..f0db231e4a --- /dev/null +++ b/compiler/utils/atomic_method_ref_map.h @@ -0,0 +1,67 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ART_COMPILER_UTILS_ATOMIC_METHOD_REF_MAP_H_ +#define ART_COMPILER_UTILS_ATOMIC_METHOD_REF_MAP_H_ + +#include "base/dchecked_vector.h" +#include "method_reference.h" +#include "safe_map.h" + +namespace art { + +class DexFile; + +// Used by CompilerCallbacks to track verification information from the Runtime. +template <typename T> +class AtomicMethodRefMap { + public: + explicit AtomicMethodRefMap() {} + ~AtomicMethodRefMap() {} + + // Atomically swap the element in if the existing value matches expected. + enum InsertResult { + kInsertResultInvalidDexFile, + kInsertResultCASFailure, + kInsertResultSuccess, + }; + InsertResult Insert(MethodReference ref, const T& expected, const T& desired); + + // Retreive an item, returns false if the dex file is not added. + bool Get(MethodReference ref, T* out) const; + + // Dex files must be added before method references belonging to them can be used as keys. Not + // thread safe. + void AddDexFile(const DexFile* dex_file); + + // Visit all of the dex files and elements. + template <typename Visitor> + void Visit(const Visitor& visitor); + + private: + // Verified methods. The method array is fixed to avoid needing a lock to extend it. + using ElementArray = dchecked_vector<Atomic<T>>; + using DexFileArrays = SafeMap<const DexFile*, ElementArray>; + + const ElementArray* GetArray(const DexFile* dex_file) const; + ElementArray* GetArray(const DexFile* dex_file); + + DexFileArrays arrays_; +}; + +} // namespace art + +#endif // ART_COMPILER_UTILS_ATOMIC_METHOD_REF_MAP_H_ diff --git a/compiler/utils/atomic_method_ref_map_test.cc b/compiler/utils/atomic_method_ref_map_test.cc new file mode 100644 index 0000000000..c3e48ff3bb --- /dev/null +++ b/compiler/utils/atomic_method_ref_map_test.cc @@ -0,0 +1,69 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "atomic_method_ref_map-inl.h" + +#include <memory> + +#include "common_runtime_test.h" +#include "dex_file-inl.h" +#include "method_reference.h" +#include "scoped_thread_state_change-inl.h" + +namespace art { + +class AtomicMethodRefMapTest : public CommonRuntimeTest {}; + +TEST_F(AtomicMethodRefMapTest, RunTests) { + ScopedObjectAccess soa(Thread::Current()); + std::unique_ptr<const DexFile> dex(OpenTestDexFile("Interfaces")); + ASSERT_TRUE(dex != nullptr); + using Map = AtomicMethodRefMap<int>; + Map map; + int value = 123; + // Error case: Not already inserted. + EXPECT_FALSE(map.Get(MethodReference(dex.get(), 1), &value)); + // Error case: Dex file not registered. + EXPECT_TRUE(map.Insert(MethodReference(dex.get(), 1), 0, 1) == Map::kInsertResultInvalidDexFile); + map.AddDexFile(dex.get()); + EXPECT_GT(dex->NumMethodIds(), 10u); + // After we have added the get should succeed but return the default value. + EXPECT_TRUE(map.Get(MethodReference(dex.get(), 1), &value)); + EXPECT_EQ(value, 0); + // Actually insert an item and make sure we can retreive it. + static const int kInsertValue = 44; + EXPECT_TRUE(map.Insert(MethodReference(dex.get(), 1), 0, kInsertValue) == + Map::kInsertResultSuccess); + EXPECT_TRUE(map.Get(MethodReference(dex.get(), 1), &value)); + EXPECT_EQ(value, kInsertValue); + static const int kInsertValue2 = 123; + EXPECT_TRUE(map.Insert(MethodReference(dex.get(), 2), 0, kInsertValue2) == + Map::kInsertResultSuccess); + EXPECT_TRUE(map.Get(MethodReference(dex.get(), 1), &value)); + EXPECT_EQ(value, kInsertValue); + EXPECT_TRUE(map.Get(MethodReference(dex.get(), 2), &value)); + EXPECT_EQ(value, kInsertValue2); + // Error case: Incorrect expected value for CAS. + EXPECT_TRUE(map.Insert(MethodReference(dex.get(), 1), 0, kInsertValue + 1) == + Map::kInsertResultCASFailure); + // Correctly overwrite the value and verify. + EXPECT_TRUE(map.Insert(MethodReference(dex.get(), 1), kInsertValue, kInsertValue + 1) == + Map::kInsertResultSuccess); + EXPECT_TRUE(map.Get(MethodReference(dex.get(), 1), &value)); + EXPECT_EQ(value, kInsertValue + 1); +} + +} // namespace art diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc index 9e6032f83a..91a32f9353 100644 --- a/dex2oat/dex2oat.cc +++ b/dex2oat/dex2oat.cc @@ -1636,7 +1636,7 @@ class Dex2Oat FINAL { 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); + verification_results_->AddDexFile(dex_file); } return true; |