diff options
| author | 2018-05-25 00:20:27 -0700 | |
|---|---|---|
| committer | 2018-05-29 10:20:59 -0700 | |
| commit | 0d896bd7da4f7b11559aa88aad82e9a9e170a4e4 (patch) | |
| tree | 44a9278d98ace169e8296feba98038c173f29eaf | |
| parent | 267c83529850f51cd690b3e31882aaae98601afd (diff) | |
Add Method/Field iterator to ClassAccessor
Enables ranged based for loops on fields and methods.
For visiting both fields and methods, VisitFieldsAndMethods will
be faster because of not needing to decode the fields twice for
seeking purposes.
Added test.
Bug: 79758018
Bug: 77709234
Test: test-art-host-gtest
Change-Id: I593e23ccd138b87a27d8bab6927ff2b685c057f3
| -rw-r--r-- | compiler/dex/dex_to_dex_compiler.cc | 4 | ||||
| -rw-r--r-- | compiler/dex/dex_to_dex_decompiler_test.cc | 7 | ||||
| -rw-r--r-- | compiler/driver/compiler_driver.cc | 20 | ||||
| -rw-r--r-- | compiler/verifier_deps_test.cc | 5 | ||||
| -rw-r--r-- | libdexfile/Android.bp | 1 | ||||
| -rw-r--r-- | libdexfile/dex/class_accessor-inl.h | 76 | ||||
| -rw-r--r-- | libdexfile/dex/class_accessor.h | 138 | ||||
| -rw-r--r-- | libdexfile/dex/class_accessor_test.cc | 79 | ||||
| -rw-r--r-- | profman/profman.cc | 4 | ||||
| -rw-r--r-- | tools/dexanalyze/dexanalyze_experiments.cc | 4 |
10 files changed, 270 insertions, 68 deletions
diff --git a/compiler/dex/dex_to_dex_compiler.cc b/compiler/dex/dex_to_dex_compiler.cc index 68155d844a..fb6a72b1c5 100644 --- a/compiler/dex/dex_to_dex_compiler.cc +++ b/compiler/dex/dex_to_dex_compiler.cc @@ -635,13 +635,13 @@ void DexToDexCompiler::SetDexFiles(const std::vector<const DexFile*>& dex_files) std::unordered_set<const DexFile::CodeItem*> seen_code_items; for (const DexFile* dex_file : dex_files) { for (ClassAccessor accessor : dex_file->GetClasses()) { - accessor.VisitMethods([&](const ClassAccessor::Method& method) { + for (const ClassAccessor::Method& method : accessor.GetMethods()) { const DexFile::CodeItem* code_item = method.GetCodeItem(); // Detect the shared code items. if (!seen_code_items.insert(code_item).second) { shared_code_items_.insert(code_item); } - }); + } } } VLOG(compiler) << "Shared code items " << shared_code_items_.size(); diff --git a/compiler/dex/dex_to_dex_decompiler_test.cc b/compiler/dex/dex_to_dex_decompiler_test.cc index 082e6091d2..75de238211 100644 --- a/compiler/dex/dex_to_dex_decompiler_test.cc +++ b/compiler/dex/dex_to_dex_decompiler_test.cc @@ -85,10 +85,9 @@ class DexToDexDecompilerTest : public CommonCompilerTest { for (uint32_t i = 0; i < updated_dex_file->NumClassDefs(); ++i) { // Unquicken each method. ClassAccessor accessor(*updated_dex_file, updated_dex_file->GetClassDef(i)); - accessor.VisitMethods([&](const ClassAccessor::Method& method) { + for (const ClassAccessor::Method& method : accessor.GetMethods()) { CompiledMethod* compiled_method = compiler_driver_->GetCompiledMethod( - MethodReference(updated_dex_file, - method.GetIndex())); + method.GetReference()); ArrayRef<const uint8_t> table; if (compiled_method != nullptr) { table = compiled_method->GetVmapTable(); @@ -97,7 +96,7 @@ class DexToDexDecompilerTest : public CommonCompilerTest { *accessor.GetCodeItem(method), table, /* decompile_return_instruction */ true); - }); + } } // Make sure after unquickening we go back to the same contents as the original dex file. diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc index decb330f3b..16f2d0f2cc 100644 --- a/compiler/driver/compiler_driver.cc +++ b/compiler/driver/compiler_driver.cc @@ -790,8 +790,7 @@ static void ResolveConstStrings(CompilerDriver* driver, // FIXME: Make sure that inlining honors this. b/26687569 continue; } - accessor.VisitMethods([&](const ClassAccessor::Method& method) - REQUIRES_SHARED(Locks::mutator_lock_) { + for (const ClassAccessor::Method& method : accessor.GetMethods()) { // Resolve const-strings in the code. Done to have deterministic allocation behavior. Right // now this is single-threaded for simplicity. // TODO: Collect the relevant string indices in parallel, then allocate them sequentially @@ -812,7 +811,7 @@ static void ResolveConstStrings(CompilerDriver* driver, break; } } - }); + } } } } @@ -880,10 +879,9 @@ static void InitializeTypeCheckBitstrings(CompilerDriver* driver, } // Direct and virtual methods. - accessor.VisitMethods([&](const ClassAccessor::Method& method) - REQUIRES_SHARED(Locks::mutator_lock_) { + for (const ClassAccessor::Method& method : accessor.GetMethods()) { InitializeTypeCheckBitstrings(driver, class_linker, dex_cache, *dex_file, method); - }); + } } } } @@ -1949,9 +1947,9 @@ bool CompilerDriver::FastVerify(jobject jclass_loader, // - We're only going to compile methods that did verify. // - Quickening will not do checkcast ellision. // TODO(ngeoffray): Reconsider this once we refactor compiler filters. - accessor.VisitMethods([&](const ClassAccessor::Method& method) { + for (const ClassAccessor::Method& method : accessor.GetMethods()) { verification_results_->CreateVerifiedMethodFor(method.GetReference()); - }); + } } } else if (!compiler_only_verifies) { // Make sure later compilation stages know they should not try to verify @@ -2747,12 +2745,12 @@ static void CompileDexFile(CompilerDriver* driver, // Compile direct and virtual methods. int64_t previous_method_idx = -1; - accessor.VisitMethods([&](const ClassAccessor::Method& method) { + for (const ClassAccessor::Method& method : accessor.GetMethods()) { const uint32_t method_idx = method.GetIndex(); if (method_idx == previous_method_idx) { // smali can create dex files with two encoded_methods sharing the same method_idx // http://code.google.com/p/smali/issues/detail?id=119 - return; + continue; } previous_method_idx = method_idx; compile_fn(soa.Self(), @@ -2767,7 +2765,7 @@ static void CompileDexFile(CompilerDriver* driver, dex_to_dex_compilation_level, compilation_enabled, dex_cache); - }); + } }; context.ForAllLambda(0, dex_file.NumClassDefs(), compile, thread_count); } diff --git a/compiler/verifier_deps_test.cc b/compiler/verifier_deps_test.cc index 103862beff..c0892ff466 100644 --- a/compiler/verifier_deps_test.cc +++ b/compiler/verifier_deps_test.cc @@ -155,8 +155,7 @@ class VerifierDepsTest : public CommonCompilerTest { bool has_failures = true; bool found_method = false; - accessor.VisitMethods([&](const ClassAccessor::Method& method) - REQUIRES_SHARED(Locks::mutator_lock_) { + for (const ClassAccessor::Method& method : accessor.GetMethods()) { ArtMethod* resolved_method = class_linker_->ResolveMethod<ClassLinker::ResolveMode::kNoChecks>( method.GetIndex(), @@ -186,7 +185,7 @@ class VerifierDepsTest : public CommonCompilerTest { has_failures = verifier.HasFailures(); found_method = true; } - }); + } CHECK(found_method) << "Expected to find method " << method_name; return !has_failures; } diff --git a/libdexfile/Android.bp b/libdexfile/Android.bp index 3818624d7a..06fd19e2fe 100644 --- a/libdexfile/Android.bp +++ b/libdexfile/Android.bp @@ -112,6 +112,7 @@ art_cc_test { ], srcs: [ "dex/art_dex_file_loader_test.cc", + "dex/class_accessor_test.cc", "dex/code_item_accessors_test.cc", "dex/compact_dex_file_test.cc", "dex/compact_offset_table_test.cc", diff --git a/libdexfile/dex/class_accessor-inl.h b/libdexfile/dex/class_accessor-inl.h index a082142366..49ca98d47f 100644 --- a/libdexfile/dex/class_accessor-inl.h +++ b/libdexfile/dex/class_accessor-inl.h @@ -50,6 +50,19 @@ inline const uint8_t* ClassAccessor::Field::Read(const uint8_t* ptr) { return ptr; } +template <typename DataType, typename Visitor> +inline const uint8_t* ClassAccessor::VisitMembers(size_t count, + const Visitor& visitor, + const uint8_t* ptr, + DataType* data) const { + DCHECK(data != nullptr); + for ( ; count != 0; --count) { + ptr = data->Read(ptr); + visitor(*data); + } + return ptr; +} + template <typename StaticFieldVisitor, typename InstanceFieldVisitor, typename DirectMethodVisitor, @@ -59,35 +72,15 @@ inline void ClassAccessor::VisitFieldsAndMethods( const InstanceFieldVisitor& instance_field_visitor, const DirectMethodVisitor& direct_method_visitor, const VirtualMethodVisitor& virtual_method_visitor) const { - const uint8_t* ptr = ptr_pos_; - { - Field data; - for (size_t i = 0; i < num_static_fields_; ++i) { - ptr = data.Read(ptr); - static_field_visitor(data); - } - } - { - Field data; - for (size_t i = 0; i < num_instance_fields_; ++i) { - ptr = data.Read(ptr); - instance_field_visitor(data); - } - } - { - Method data(dex_file_, /*is_static_or_direct*/ true); - for (size_t i = 0; i < num_direct_methods_; ++i) { - ptr = data.Read(ptr); - direct_method_visitor(data); - } - } - { - Method data(dex_file_, /*is_static_or_direct*/ false); - for (size_t i = 0; i < num_virtual_methods_; ++i) { - ptr = data.Read(ptr); - virtual_method_visitor(data); - } - } + Field field(dex_file_); + const uint8_t* ptr = VisitMembers(num_static_fields_, static_field_visitor, ptr_pos_, &field); + field.NextSection(); + ptr = VisitMembers(num_instance_fields_, instance_field_visitor, ptr, &field); + + Method method(dex_file_, /*is_static_or_direct*/ true); + ptr = VisitMembers(num_direct_methods_, direct_method_visitor, ptr, &method); + method.NextSection(); + ptr = VisitMembers(num_virtual_methods_, virtual_method_visitor, ptr, &method); } template <typename DirectMethodVisitor, @@ -110,12 +103,6 @@ inline void ClassAccessor::VisitFields(const StaticFieldVisitor& static_field_vi VoidFunctor()); } -// Visit direct and virtual methods. -template <typename MethodVisitor> -inline void ClassAccessor::VisitMethods(const MethodVisitor& method_visitor) const { - VisitMethods(method_visitor, method_visitor); -} - inline const DexFile::CodeItem* ClassAccessor::GetCodeItem(const Method& method) const { return dex_file_.GetCodeItem(method.GetCodeItemOffset()); } @@ -132,6 +119,25 @@ inline const DexFile::CodeItem* ClassAccessor::Method::GetCodeItem() const { return dex_file_.GetCodeItem(code_off_); } +inline IterationRange<ClassAccessor::DataIterator<ClassAccessor::Field>> ClassAccessor::GetFields() + const { + const uint32_t limit = num_static_fields_ + num_instance_fields_; + return { DataIterator<Field>(dex_file_, 0u, num_static_fields_, limit, ptr_pos_), + DataIterator<Field>(dex_file_, limit, num_static_fields_, limit, ptr_pos_) }; +} + +inline IterationRange<ClassAccessor::DataIterator<ClassAccessor::Method>> + ClassAccessor::GetMethods() const { + // Skip over the fields. + Field field(dex_file_); + const size_t skip_count = num_static_fields_ + num_instance_fields_; + const uint8_t* ptr_pos = VisitMembers(skip_count, VoidFunctor(), ptr_pos_, &field); + // Return the iterator pair for all the methods. + const uint32_t limit = num_direct_methods_ + num_virtual_methods_; + return { DataIterator<Method>(dex_file_, 0u, num_direct_methods_, limit, ptr_pos), + DataIterator<Method>(dex_file_, limit, num_direct_methods_, limit, ptr_pos) }; +} + } // namespace art #endif // ART_LIBDEXFILE_DEX_CLASS_ACCESSOR_INL_H_ diff --git a/libdexfile/dex/class_accessor.h b/libdexfile/dex/class_accessor.h index 72bc50b98c..7455704a7f 100644 --- a/libdexfile/dex/class_accessor.h +++ b/libdexfile/dex/class_accessor.h @@ -45,7 +45,7 @@ class ClassAccessor { return (GetAccessFlags() & kAccFinal) != 0; } - public: + protected: uint32_t index_ = 0u; uint32_t access_flags_ = 0u; }; @@ -72,8 +72,13 @@ class ClassAccessor { const DexFile::CodeItem* GetCodeItem() const; + bool IsStaticOrDirect() const { + return is_static_or_direct_; + } + private: - explicit Method(const DexFile& dex_file, bool is_static_or_direct) + explicit Method(const DexFile& dex_file, + bool is_static_or_direct = true) : dex_file_(dex_file), is_static_or_direct_(is_static_or_direct) {} @@ -94,8 +99,14 @@ class ClassAccessor { } } + void NextSection() { + DCHECK(is_static_or_direct_) << "Already in the virtual methods section"; + is_static_or_direct_ = false; + index_ = 0u; + } + const DexFile& dex_file_; - const bool is_static_or_direct_; + bool is_static_or_direct_ = true; uint32_t code_off_ = 0u; friend class ClassAccessor; @@ -103,12 +114,113 @@ class ClassAccessor { // A decoded version of the field of a class_data_item. class Field : public BaseItem { + public: + explicit Field(const DexFile& dex_file) : dex_file_(dex_file) {} + + const DexFile& GetDexFile() const { + return dex_file_; + } + private: const uint8_t* Read(const uint8_t* ptr); + void NextSection() { + index_ = 0u; + } + + const DexFile& dex_file_; friend class ClassAccessor; }; + template <typename DataType> + class DataIterator : public std::iterator<std::forward_iterator_tag, DataType> { + public: + using value_type = typename std::iterator<std::forward_iterator_tag, DataType>::value_type; + using difference_type = + typename std::iterator<std::forward_iterator_tag, value_type>::difference_type; + + DataIterator(const DexFile& dex_file, + uint32_t position, + uint32_t partition_pos, + uint32_t iterator_end, + const uint8_t* ptr_pos) + : data_(dex_file), + position_(position), + partition_pos_(partition_pos), + iterator_end_(iterator_end), + ptr_pos_(ptr_pos) { + ReadData(); + } + + bool IsValid() const { + return position_ < iterator_end_; + } + + // Value after modification. + DataIterator& operator++() { + ++position_; + ReadData(); + return *this; + } + + const value_type& operator*() const { + return data_; + } + + const value_type* operator->() const { + return &data_; + } + + bool operator==(const DataIterator& rhs) const { + DCHECK_EQ(&data_.dex_file_, &rhs.data_.dex_file_) << "Comparing different dex files."; + return position_ == rhs.position_; + } + + bool operator!=(const DataIterator& rhs) const { + return !(*this == rhs); + } + + bool operator<(const DataIterator& rhs) const { + DCHECK_EQ(&data_.dex_file_, &rhs.data_.dex_file_) << "Comparing different dex files."; + return position_ < rhs.position_; + } + + bool operator>(const DataIterator& rhs) const { + return rhs < *this; + } + + bool operator<=(const DataIterator& rhs) const { + return !(rhs < *this); + } + + bool operator>=(const DataIterator& rhs) const { + return !(*this < rhs); + } + + private: + // Read data at current position. + void ReadData() { + if (IsValid()) { + // At the end of the first section, go to the next section. + if (position_ == partition_pos_) { + data_.NextSection(); + } + DCHECK(ptr_pos_ != nullptr); + ptr_pos_ = data_.Read(ptr_pos_); + } + } + + DataType data_; + // Iterator position. + uint32_t position_; + // At partition_pos_, we go to the next section. + const uint32_t partition_pos_; + // At iterator_end_, the iterator is no longer valid. + const uint32_t iterator_end_; + // Internal data pointer. + const uint8_t* ptr_pos_; + }; + // Not explicit specifically for range-based loops. ALWAYS_INLINE ClassAccessor(const ClassIteratorData& data); @@ -118,7 +230,6 @@ class ClassAccessor { const DexFile::CodeItem* GetCodeItem(const Method& method) const; // Iterator data is not very iterator friendly, use visitors to get around this. - // No thread safety analysis since the visitor may require capabilities. template <typename StaticFieldVisitor, typename InstanceFieldVisitor, typename DirectMethodVisitor, @@ -126,8 +237,7 @@ class ClassAccessor { void VisitFieldsAndMethods(const StaticFieldVisitor& static_field_visitor, const InstanceFieldVisitor& instance_field_visitor, const DirectMethodVisitor& direct_method_visitor, - const VirtualMethodVisitor& virtual_method_visitor) const - NO_THREAD_SAFETY_ANALYSIS; + const VirtualMethodVisitor& virtual_method_visitor) const; template <typename DirectMethodVisitor, typename VirtualMethodVisitor> @@ -139,9 +249,11 @@ class ClassAccessor { void VisitFields(const StaticFieldVisitor& static_field_visitor, const InstanceFieldVisitor& instance_field_visitor) const; - // Visit direct and virtual methods. - template <typename MethodVisitor> - void VisitMethods(const MethodVisitor& method_visitor) const; + // Return the iteration range for all the fields. + IterationRange<DataIterator<Field>> GetFields() const; + + // Return the iteration range for all the methods. + IterationRange<DataIterator<Method>> GetMethods() const; uint32_t NumStaticFields() const { return num_static_fields_; @@ -170,6 +282,14 @@ class ClassAccessor { } protected: + // Template visitor to reduce copy paste for visiting elements. + // No thread safety analysis since the visitor may require capabilities. + template <typename DataType, typename Visitor> + const uint8_t* VisitMembers(size_t count, + const Visitor& visitor, + const uint8_t* ptr, + DataType* data) const NO_THREAD_SAFETY_ANALYSIS; + const DexFile& dex_file_; const dex::TypeIndex descriptor_index_ = {}; const uint8_t* ptr_pos_ = nullptr; // Pointer into stream of class_data_item. diff --git a/libdexfile/dex/class_accessor_test.cc b/libdexfile/dex/class_accessor_test.cc new file mode 100644 index 0000000000..95380d8140 --- /dev/null +++ b/libdexfile/dex/class_accessor_test.cc @@ -0,0 +1,79 @@ +/* + * Copyright (C) 2018 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 "dex/class_accessor-inl.h" + +#include "base/common_art_test.h" + +namespace art { + +class ClassAccessorTest : public CommonArtTest {}; + +TEST_F(ClassAccessorTest, TestVisiting) { + std::vector<std::unique_ptr<const DexFile>> dex_files( + OpenDexFiles(GetLibCoreDexFileNames()[0].c_str())); + ASSERT_GT(dex_files.size(), 0u); + for (const std::unique_ptr<const DexFile>& dex_file : dex_files) { + uint32_t class_def_idx = 0u; + ASSERT_GT(dex_file->NumClassDefs(), 0u); + for (ClassAccessor accessor : dex_file->GetClasses()) { + const DexFile::ClassDef& class_def = dex_file->GetClassDef(class_def_idx); + EXPECT_EQ(accessor.GetDescriptor(), dex_file->StringByTypeIdx(class_def.class_idx_)); + ++class_def_idx; + // Check iterators against visitors. + auto methods = accessor.GetMethods(); + auto fields = accessor.GetFields(); + auto method_it = methods.begin(); + auto field_it = fields.begin(); + accessor.VisitFieldsAndMethods( + // Static fields. + [&](const ClassAccessor::Field& field) { + EXPECT_EQ(field.GetIndex(), field_it->GetIndex()); + EXPECT_EQ(field.GetAccessFlags(), field_it->GetAccessFlags()); + ++field_it; + }, + // Instance fields. + [&](const ClassAccessor::Field& field) { + EXPECT_EQ(field.GetIndex(), field_it->GetIndex()); + EXPECT_EQ(field.GetAccessFlags(), field_it->GetAccessFlags()); + ++field_it; + }, + // Direct methods. + [&](const ClassAccessor::Method& method) { + EXPECT_TRUE(method.IsStaticOrDirect()); + EXPECT_EQ(method.IsStaticOrDirect(), method_it->IsStaticOrDirect()); + EXPECT_EQ(method.GetIndex(), method_it->GetIndex()); + EXPECT_EQ(method.GetAccessFlags(), method_it->GetAccessFlags()); + EXPECT_EQ(method.GetCodeItem(), method_it->GetCodeItem()); + ++method_it; + }, + // Virtual methods. + [&](const ClassAccessor::Method& method) { + EXPECT_FALSE(method.IsStaticOrDirect()); + EXPECT_EQ(method.IsStaticOrDirect(), method_it->IsStaticOrDirect()); + EXPECT_EQ(method.GetIndex(), method_it->GetIndex()); + EXPECT_EQ(method.GetAccessFlags(), method_it->GetAccessFlags()); + EXPECT_EQ(method.GetCodeItem(), method_it->GetCodeItem()); + ++method_it; + }); + ASSERT_TRUE(field_it == fields.end()); + ASSERT_TRUE(method_it == methods.end()); + } + EXPECT_EQ(class_def_idx, dex_file->NumClassDefs()); + } +} + +} // namespace art diff --git a/profman/profman.cc b/profman/profman.cc index 661132d94f..096e5dc3bd 100644 --- a/profman/profman.cc +++ b/profman/profman.cc @@ -931,12 +931,12 @@ class ProfMan FINAL { std::vector<ProfileMethodInfo> methods; if (method_str == kClassAllMethods) { ClassAccessor accessor(*dex_file, *dex_file->FindClassDef(class_ref.TypeIndex())); - accessor.VisitMethods([&](const ClassAccessor::Method& method) { + for (const ClassAccessor::Method& method : accessor.GetMethods()) { if (method.GetCodeItemOffset() != 0) { // Add all of the methods that have code to the profile. methods.push_back(ProfileMethodInfo(method.GetReference())); } - }); + } } // TODO: Check return values? profile->AddMethods(methods, static_cast<ProfileCompilationInfo::MethodHotness::Flag>(flags)); diff --git a/tools/dexanalyze/dexanalyze_experiments.cc b/tools/dexanalyze/dexanalyze_experiments.cc index 82cb0d28ab..0f20a99f05 100644 --- a/tools/dexanalyze/dexanalyze_experiments.cc +++ b/tools/dexanalyze/dexanalyze_experiments.cc @@ -130,7 +130,7 @@ void CountDexIndices::ProcessDexFile(const DexFile& dex_file) { for (ClassAccessor accessor : dex_file.GetClasses()) { std::set<size_t> unique_method_ids; std::set<size_t> unique_string_ids; - accessor.VisitMethods([&](const ClassAccessor::Method& method) { + for (const ClassAccessor::Method& method : accessor.GetMethods()) { dex_code_bytes_ += method.GetInstructions().InsnsSizeInBytes(); unique_code_items.insert(method.GetCodeItemOffset()); for (const DexInstructionPcPair& inst : method.GetInstructions()) { @@ -188,7 +188,7 @@ void CountDexIndices::ProcessDexFile(const DexFile& dex_file) { break; } } - }); + } total_unique_method_idx_ += unique_method_ids.size(); total_unique_string_ids_ += unique_string_ids.size(); } |