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
diff --git a/compiler/dex/dex_to_dex_compiler.cc b/compiler/dex/dex_to_dex_compiler.cc
index 68155d8..fb6a72b 100644
--- a/compiler/dex/dex_to_dex_compiler.cc
+++ b/compiler/dex/dex_to_dex_compiler.cc
@@ -635,13 +635,13 @@
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 082e609..75de238 100644
--- a/compiler/dex/dex_to_dex_decompiler_test.cc
+++ b/compiler/dex/dex_to_dex_decompiler_test.cc
@@ -85,10 +85,9 @@
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 @@
*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 decb330..16f2d0f 100644
--- a/compiler/driver/compiler_driver.cc
+++ b/compiler/driver/compiler_driver.cc
@@ -790,8 +790,7 @@
// 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 @@
break;
}
}
- });
+ }
}
}
}
@@ -880,10 +879,9 @@
}
// 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 @@
// - 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 @@
// 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 @@
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 103862b..c0892ff 100644
--- a/compiler/verifier_deps_test.cc
+++ b/compiler/verifier_deps_test.cc
@@ -155,8 +155,7 @@
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 @@
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 3818624..06fd19e 100644
--- a/libdexfile/Android.bp
+++ b/libdexfile/Android.bp
@@ -112,6 +112,7 @@
],
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 a082142..49ca98d 100644
--- a/libdexfile/dex/class_accessor-inl.h
+++ b/libdexfile/dex/class_accessor-inl.h
@@ -50,6 +50,19 @@
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 @@
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 @@
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 @@
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 72bc50b..7455704 100644
--- a/libdexfile/dex/class_accessor.h
+++ b/libdexfile/dex/class_accessor.h
@@ -45,7 +45,7 @@
return (GetAccessFlags() & kAccFinal) != 0;
}
- public:
+ protected:
uint32_t index_ = 0u;
uint32_t access_flags_ = 0u;
};
@@ -72,8 +72,13 @@
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 @@
}
}
+ 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 @@
// 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 @@
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 @@
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 @@
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 @@
}
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 0000000..95380d8
--- /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 661132d..096e5dc 100644
--- a/profman/profman.cc
+++ b/profman/profman.cc
@@ -931,12 +931,12 @@
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 82cb0d2..0f20a99 100644
--- a/tools/dexanalyze/dexanalyze_experiments.cc
+++ b/tools/dexanalyze/dexanalyze_experiments.cc
@@ -130,7 +130,7 @@
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 @@
break;
}
}
- });
+ }
total_unique_method_idx_ += unique_method_ids.size();
total_unique_string_ids_ += unique_string_ids.size();
}
