Change image method and field visiting to use templates
Remove boilerplate code.
Test: test-art-host
Change-Id: I407efaa1e03f67f91e22e085c68a573defef2451
diff --git a/imgdiag/imgdiag.cc b/imgdiag/imgdiag.cc
index f3982ec..8112d3e 100644
--- a/imgdiag/imgdiag.cc
+++ b/imgdiag/imgdiag.cc
@@ -653,7 +653,7 @@
};
// Region analysis for ArtMethods.
-class ImgArtMethodVisitor : public ArtMethodVisitor {
+class ImgArtMethodVisitor {
public:
using ComputeDirtyFunc = std::function<void(ArtMethod*,
const uint8_t*,
@@ -664,9 +664,8 @@
dirty_func_(std::move(dirty_func)),
begin_image_ptr_(begin_image_ptr),
dirty_pages_(dirty_pages) { }
- ~ImgArtMethodVisitor() override { }
- void Visit(ArtMethod* method) override {
- dirty_func_(method, begin_image_ptr_, dirty_pages_);
+ void operator()(ArtMethod& method) const {
+ dirty_func_(&method, begin_image_ptr_, dirty_pages_);
}
private:
@@ -725,7 +724,7 @@
uint8_t* base,
PointerSize pointer_size)
REQUIRES_SHARED(Locks::mutator_lock_) {
- RegionCommon<ArtMethod>::image_header_.VisitPackedArtMethods(visitor, base, pointer_size);
+ RegionCommon<ArtMethod>::image_header_.VisitPackedArtMethods(*visitor, base, pointer_size);
}
void VisitEntry(ArtMethod* method ATTRIBUTE_UNUSED)
diff --git a/oatdump/oatdump.cc b/oatdump/oatdump.cc
index e395985..a401f6c 100644
--- a/oatdump/oatdump.cc
+++ b/oatdump/oatdump.cc
@@ -1933,10 +1933,13 @@
indent_os << "\n";
// TODO: Dump fields.
// Dump methods after.
- DumpArtMethodVisitor visitor(this);
- image_header_.VisitPackedArtMethods(&visitor,
- image_space_.Begin(),
- image_header_.GetPointerSize());
+ image_header_.VisitPackedArtMethods([&](ArtMethod& method)
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ std::ostream& indent_os = vios_.Stream();
+ indent_os << &method << " " << " ArtMethod: " << method.PrettyMethod() << "\n";
+ DumpMethod(&method, indent_os);
+ indent_os << "\n";
+ }, image_space_.Begin(), image_header_.GetPointerSize());
// Dump the large objects separately.
heap->GetLargeObjectsSpace()->GetLiveBitmap()->Walk(dump_visitor);
indent_os << "\n";
@@ -2022,21 +2025,6 @@
}
private:
- class DumpArtMethodVisitor : public ArtMethodVisitor {
- public:
- explicit DumpArtMethodVisitor(ImageDumper* image_dumper) : image_dumper_(image_dumper) {}
-
- void Visit(ArtMethod* method) override REQUIRES_SHARED(Locks::mutator_lock_) {
- std::ostream& indent_os = image_dumper_->vios_.Stream();
- indent_os << method << " " << " ArtMethod: " << ArtMethod::PrettyMethod(method) << "\n";
- image_dumper_->DumpMethod(method, indent_os);
- indent_os << "\n";
- }
-
- private:
- ImageDumper* const image_dumper_;
- };
-
static void PrettyObjectValue(std::ostream& os,
ObjPtr<mirror::Class> type,
ObjPtr<mirror::Object> value)
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 1dd856e..a27b28e 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -927,28 +927,6 @@
}
}
-// Set image methods' entry point to interpreter.
-class SetInterpreterEntrypointArtMethodVisitor : public ArtMethodVisitor {
- public:
- explicit SetInterpreterEntrypointArtMethodVisitor(PointerSize image_pointer_size)
- : image_pointer_size_(image_pointer_size) {}
-
- void Visit(ArtMethod* method) override REQUIRES_SHARED(Locks::mutator_lock_) {
- if (kIsDebugBuild && !method->IsRuntimeMethod()) {
- CHECK(method->GetDeclaringClass() != nullptr);
- }
- if (!method->IsNative() && !method->IsRuntimeMethod() && !method->IsResolutionMethod()) {
- method->SetEntryPointFromQuickCompiledCodePtrSize(GetQuickToInterpreterBridge(),
- image_pointer_size_);
- }
- }
-
- private:
- const PointerSize image_pointer_size_;
-
- DISALLOW_COPY_AND_ASSIGN(SetInterpreterEntrypointArtMethodVisitor);
-};
-
struct TrampolineCheckData {
const void* quick_resolution_trampoline;
const void* quick_imt_conflict_trampoline;
@@ -1334,23 +1312,6 @@
const Thread* self_;
};
-class VerifyDeclaringClassVisitor : public ArtMethodVisitor {
- public:
- VerifyDeclaringClassVisitor() REQUIRES_SHARED(Locks::mutator_lock_, Locks::heap_bitmap_lock_)
- : live_bitmap_(Runtime::Current()->GetHeap()->GetLiveBitmap()) {}
-
- void Visit(ArtMethod* method) override
- REQUIRES_SHARED(Locks::mutator_lock_, Locks::heap_bitmap_lock_) {
- ObjPtr<mirror::Class> klass = method->GetDeclaringClassUnchecked();
- if (klass != nullptr) {
- CHECK(live_bitmap_->Test(klass.Ptr())) << "Image method has unmarked declaring class";
- }
- }
-
- private:
- gc::accounting::HeapBitmap* const live_bitmap_;
-};
-
/*
* A class used to ensure that all strings in an AppImage have been properly
* interned, and is only ever run in debug mode.
@@ -1570,8 +1531,14 @@
if (kVerifyArtMethodDeclaringClasses) {
ScopedTrace timing("AppImage:VerifyDeclaringClasses");
ReaderMutexLock rmu(self, *Locks::heap_bitmap_lock_);
- VerifyDeclaringClassVisitor visitor;
- header.VisitPackedArtMethods(&visitor, space->Begin(), kRuntimePointerSize);
+ gc::accounting::HeapBitmap* live_bitmap = heap->GetLiveBitmap();
+ header.VisitPackedArtMethods([&](ArtMethod& method)
+ REQUIRES_SHARED(Locks::mutator_lock_, Locks::heap_bitmap_lock_) {
+ ObjPtr<mirror::Class> klass = method.GetDeclaringClassUnchecked();
+ if (klass != nullptr) {
+ CHECK(live_bitmap->Test(klass.Ptr())) << "Image method has unmarked declaring class";
+ }
+ }, space->Begin(), kRuntimePointerSize);
}
}
@@ -1954,25 +1921,13 @@
const Handle<mirror::ObjectArray<mirror::DexCache> >& dex_caches,
ClassTable* class_table, gc::space::ImageSpace* space)
REQUIRES_SHARED(Locks::mutator_lock_) {
- {
- class VerifyClassInTableArtMethodVisitor : public ArtMethodVisitor {
- public:
- explicit VerifyClassInTableArtMethodVisitor(ClassTable* table) : table_(table) {}
-
- void Visit(ArtMethod* method) override
- REQUIRES_SHARED(Locks::mutator_lock_, Locks::classlinker_classes_lock_) {
- ObjPtr<mirror::Class> klass = method->GetDeclaringClass();
- if (klass != nullptr && !Runtime::Current()->GetHeap()->ObjectIsInBootImageSpace(klass)) {
- CHECK_EQ(table_->LookupByDescriptor(klass), klass) << mirror::Class::PrettyClass(klass);
- }
- }
-
- private:
- ClassTable* const table_;
- };
- VerifyClassInTableArtMethodVisitor visitor(class_table);
- header.VisitPackedArtMethods(&visitor, space->Begin(), kRuntimePointerSize);
- }
+ header.VisitPackedArtMethods([&](ArtMethod& method) REQUIRES_SHARED(Locks::mutator_lock_) {
+ ObjPtr<mirror::Class> klass = method.GetDeclaringClass();
+ if (klass != nullptr && !Runtime::Current()->GetHeap()->ObjectIsInBootImageSpace(klass)) {
+ CHECK_EQ(class_table->LookupByDescriptor(klass), klass)
+ << mirror::Class::PrettyClass(klass);
+ }
+ }, space->Begin(), kRuntimePointerSize);
{
// Verify that all direct interfaces of classes in the class table are also resolved.
std::vector<ObjPtr<mirror::Class>> classes;
@@ -2179,8 +2134,16 @@
// Set entry point to interpreter if in InterpretOnly mode.
if (!runtime->IsAotCompiler() && runtime->GetInstrumentation()->InterpretOnly()) {
- SetInterpreterEntrypointArtMethodVisitor visitor(image_pointer_size_);
- header.VisitPackedArtMethods(&visitor, space->Begin(), image_pointer_size_);
+ // Set image methods' entry point to interpreter.
+ header.VisitPackedArtMethods([&](ArtMethod& method) REQUIRES_SHARED(Locks::mutator_lock_) {
+ if (!method.IsRuntimeMethod()) {
+ DCHECK(method.GetDeclaringClass() != nullptr);
+ if (!method.IsNative() && !method.IsResolutionMethod()) {
+ method.SetEntryPointFromQuickCompiledCodePtrSize(GetQuickToInterpreterBridge(),
+ image_pointer_size_);
+ }
+ }
+ }, space->Begin(), image_pointer_size_);
}
ClassTable* class_table = nullptr;
diff --git a/runtime/gc/space/image_space.cc b/runtime/gc/space/image_space.cc
index 1a145ec..5ebd4b3 100644
--- a/runtime/gc/space/image_space.cc
+++ b/runtime/gc/space/image_space.cc
@@ -634,45 +634,6 @@
NativeVisitor native_visitor_;
};
-template <typename ObjectVisitor>
-class ImageSpace::PatchArtFieldVisitor final : public ArtFieldVisitor {
- public:
- explicit PatchArtFieldVisitor(const ObjectVisitor& visitor) : visitor_(visitor) {}
-
- void Visit(ArtField* field) override REQUIRES_SHARED(Locks::mutator_lock_) {
- visitor_.template PatchGcRoot</*kMayBeNull=*/ false>(&field->DeclaringClassRoot());
- }
-
- private:
- const ObjectVisitor visitor_;
-};
-
-template <PointerSize kPointerSize, typename ObjectVisitor, typename CodeVisitor>
-class ImageSpace::PatchArtMethodVisitor final : public ArtMethodVisitor {
- public:
- explicit PatchArtMethodVisitor(const ObjectVisitor& object_visitor,
- const CodeVisitor& code_visitor)
- : object_visitor_(object_visitor),
- code_visitor_(code_visitor) {}
-
- void Visit(ArtMethod* method) override REQUIRES_SHARED(Locks::mutator_lock_) {
- object_visitor_.PatchGcRoot(&method->DeclaringClassRoot());
- void** data_address = PointerAddress(method, ArtMethod::DataOffset(kPointerSize));
- object_visitor_.PatchNativePointer(data_address);
- void** entrypoint_address =
- PointerAddress(method, ArtMethod::EntryPointFromQuickCompiledCodeOffset(kPointerSize));
- code_visitor_.PatchNativePointer(entrypoint_address);
- }
-
- private:
- void** PointerAddress(ArtMethod* method, MemberOffset offset) {
- return reinterpret_cast<void**>(reinterpret_cast<uint8_t*>(method) + offset.Uint32Value());
- }
-
- const ObjectVisitor object_visitor_;
- const CodeVisitor code_visitor_;
-};
-
template <typename ReferenceVisitor>
class ImageSpace::ClassTableVisitor final {
public:
@@ -1149,60 +1110,6 @@
Forward forward_;
};
- template <typename ForwardObject, typename ForwardNative, typename ForwardCode>
- class FixupArtMethodVisitor : public ArtMethodVisitor {
- public:
- template<typename... Args>
- explicit FixupArtMethodVisitor(PointerSize pointer_size,
- const ForwardObject& forward_object,
- const ForwardNative& forward_native,
- const ForwardCode& forward_code)
- : pointer_size_(pointer_size),
- forward_object_(forward_object),
- forward_native_(forward_native),
- forward_code_(forward_code) {}
-
- void Visit(ArtMethod* method) override NO_THREAD_SAFETY_ANALYSIS {
- // TODO: Separate visitor for runtime vs normal methods.
- if (UNLIKELY(method->IsRuntimeMethod())) {
- ImtConflictTable* table = method->GetImtConflictTable(pointer_size_);
- if (table != nullptr) {
- ImtConflictTable* new_table = forward_native_(table);
- if (table != new_table) {
- method->SetImtConflictTable(new_table, pointer_size_);
- }
- }
- const void* old_code = method->GetEntryPointFromQuickCompiledCodePtrSize(pointer_size_);
- const void* new_code = forward_code_(old_code);
- if (old_code != new_code) {
- method->SetEntryPointFromQuickCompiledCodePtrSize(new_code, pointer_size_);
- }
- } else {
- method->UpdateObjectsForImageRelocation(forward_object_);
- method->UpdateEntrypoints(forward_code_, pointer_size_);
- }
- }
-
- private:
- const PointerSize pointer_size_;
- const ForwardObject forward_object_;
- const ForwardNative forward_native_;
- const ForwardCode forward_code_;
- };
-
- template <typename Forward>
- class FixupArtFieldVisitor : public ArtFieldVisitor {
- public:
- explicit FixupArtFieldVisitor(Forward forward) : forward_(forward) {}
-
- void Visit(ArtField* field) override NO_THREAD_SAFETY_ANALYSIS {
- field->UpdateObjects(forward_);
- }
-
- private:
- Forward forward_;
- };
-
// Relocate an image space mapped at target_base which possibly used to be at a different base
// address. In place means modifying a single ImageSpace in place rather than relocating from
// one ImageSpace to another.
@@ -1361,18 +1268,34 @@
{
// Only touches objects in the app image, no need for mutator lock.
TimingLogger::ScopedTiming timing("Fixup methods", &logger);
- FixupArtMethodVisitor method_visitor(kPointerSize,
- forward_object,
- forward_metadata,
- forward_code);
- image_header.VisitPackedArtMethods(&method_visitor, target_base, kPointerSize);
+ image_header.VisitPackedArtMethods([&](ArtMethod& method) NO_THREAD_SAFETY_ANALYSIS {
+ // TODO: Consider a separate visitor for runtime vs normal methods.
+ if (UNLIKELY(method.IsRuntimeMethod())) {
+ ImtConflictTable* table = method.GetImtConflictTable(kPointerSize);
+ if (table != nullptr) {
+ ImtConflictTable* new_table = forward_metadata(table);
+ if (table != new_table) {
+ method.SetImtConflictTable(new_table, kPointerSize);
+ }
+ }
+ const void* old_code = method.GetEntryPointFromQuickCompiledCodePtrSize(kPointerSize);
+ const void* new_code = forward_code(old_code);
+ if (old_code != new_code) {
+ method.SetEntryPointFromQuickCompiledCodePtrSize(new_code, kPointerSize);
+ }
+ } else {
+ method.UpdateObjectsForImageRelocation(forward_object);
+ method.UpdateEntrypoints(forward_code, kPointerSize);
+ }
+ }, target_base, kPointerSize);
}
if (fixup_image) {
{
// Only touches objects in the app image, no need for mutator lock.
TimingLogger::ScopedTiming timing("Fixup fields", &logger);
- FixupArtFieldVisitor field_visitor(forward_object);
- image_header.VisitPackedArtFields(&field_visitor, target_base);
+ image_header.VisitPackedArtFields([&](ArtField& field) NO_THREAD_SAFETY_ANALYSIS {
+ field.UpdateObjects(forward_object);
+ }, target_base);
}
{
TimingLogger::ScopedTiming timing("Fixup imt", &logger);
@@ -1611,6 +1534,10 @@
const uint32_t diff_;
};
+ static void** PointerAddress(ArtMethod* method, MemberOffset offset) {
+ return reinterpret_cast<void**>(reinterpret_cast<uint8_t*>(method) + offset.Uint32Value());
+ }
+
template <PointerSize kPointerSize>
static void DoRelocateSpaces(const std::vector<std::unique_ptr<ImageSpace>>& spaces,
uint32_t diff) REQUIRES_SHARED(Locks::mutator_lock_) {
@@ -1624,9 +1551,7 @@
PatchRelocateVisitor patch_object_visitor(relocate_visitor, relocate_visitor);
mirror::Class* dcheck_class_class = nullptr; // Used only for a DCHECK().
- for (size_t s = 0u, size = spaces.size(); s != size; ++s) {
- const ImageSpace* space = spaces[s].get();
-
+ for (const std::unique_ptr<ImageSpace>& space : spaces) {
// First patch the image header. The `diff` is OK for patching 32-bit fields but
// the 64-bit method fields in the ImageHeader may need a negative `delta`.
reinterpret_cast<ImageHeader*>(space->Begin())->RelocateImage(
@@ -1635,11 +1560,19 @@
// Patch fields and methods.
const ImageHeader& image_header = space->GetImageHeader();
- PatchArtFieldVisitor<PatchRelocateVisitor> field_visitor(patch_object_visitor);
- image_header.VisitPackedArtFields(&field_visitor, space->Begin());
- PatchArtMethodVisitor<kPointerSize, PatchRelocateVisitor, PatchRelocateVisitor>
- method_visitor(patch_object_visitor, patch_object_visitor);
- image_header.VisitPackedArtMethods(&method_visitor, space->Begin(), kPointerSize);
+ image_header.VisitPackedArtFields([&](ArtField& field) REQUIRES_SHARED(Locks::mutator_lock_) {
+ patch_object_visitor.template PatchGcRoot</*kMayBeNull=*/ false>(
+ &field.DeclaringClassRoot());
+ }, space->Begin());
+ image_header.VisitPackedArtMethods([&](ArtMethod& method)
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ patch_object_visitor.PatchGcRoot(&method.DeclaringClassRoot());
+ void** data_address = PointerAddress(&method, ArtMethod::DataOffset(kPointerSize));
+ patch_object_visitor.PatchNativePointer(data_address);
+ void** entrypoint_address =
+ PointerAddress(&method, ArtMethod::EntryPointFromQuickCompiledCodeOffset(kPointerSize));
+ patch_object_visitor.PatchNativePointer(entrypoint_address);
+ }, space->Begin(), kPointerSize);
auto method_table_visitor = [&](ArtMethod* method) {
DCHECK(method != nullptr);
return relocate_visitor(method);
diff --git a/runtime/image-inl.h b/runtime/image-inl.h
index 2082064..0404e99 100644
--- a/runtime/image-inl.h
+++ b/runtime/image-inl.h
@@ -49,34 +49,36 @@
return image_roots;
}
-inline void ImageHeader::VisitPackedArtFields(ArtFieldVisitor* visitor, uint8_t* base) const {
+template <typename Visitor>
+inline void ImageHeader::VisitPackedArtFields(const Visitor& visitor, uint8_t* base) const {
const ImageSection& fields = GetFieldsSection();
for (size_t pos = 0u; pos < fields.Size(); ) {
auto* array = reinterpret_cast<LengthPrefixedArray<ArtField>*>(base + fields.Offset() + pos);
for (size_t i = 0u; i < array->size(); ++i) {
- visitor->Visit(&array->At(i, sizeof(ArtField)));
+ visitor(array->At(i, sizeof(ArtField)));
}
pos += array->ComputeSize(array->size());
}
}
-inline void ImageHeader::VisitPackedArtMethods(ArtMethodVisitor* visitor,
- uint8_t* base,
- PointerSize pointer_size) const {
+template <typename Visitor>
+inline void ImageHeader::VisitPackedArtMethods(const Visitor& visitor,
+ uint8_t* base,
+ PointerSize pointer_size) const {
const size_t method_alignment = ArtMethod::Alignment(pointer_size);
const size_t method_size = ArtMethod::Size(pointer_size);
const ImageSection& methods = GetMethodsSection();
for (size_t pos = 0u; pos < methods.Size(); ) {
auto* array = reinterpret_cast<LengthPrefixedArray<ArtMethod>*>(base + methods.Offset() + pos);
for (size_t i = 0u; i < array->size(); ++i) {
- visitor->Visit(&array->At(i, method_size, method_alignment));
+ visitor(array->At(i, method_size, method_alignment));
}
pos += array->ComputeSize(array->size(), method_size, method_alignment);
}
const ImageSection& runtime_methods = GetRuntimeMethodsSection();
for (size_t pos = 0u; pos < runtime_methods.Size(); ) {
auto* method = reinterpret_cast<ArtMethod*>(base + runtime_methods.Offset() + pos);
- visitor->Visit(method);
+ visitor(*method);
pos += method_size;
}
}
diff --git a/runtime/image.h b/runtime/image.h
index 6578287..88bba13 100644
--- a/runtime/image.h
+++ b/runtime/image.h
@@ -41,20 +41,6 @@
virtual void Visit(mirror::Object* object) = 0;
};
-class ArtMethodVisitor {
- public:
- virtual ~ArtMethodVisitor() {}
-
- virtual void Visit(ArtMethod* method) = 0;
-};
-
-class ArtFieldVisitor {
- public:
- virtual ~ArtFieldVisitor() {}
-
- virtual void Visit(ArtField* method) = 0;
-};
-
class PACKED(4) ImageSection {
public:
ImageSection() : offset_(0), size_(0) { }
@@ -379,13 +365,17 @@
// Visit ArtMethods in the section starting at base. Includes runtime methods.
// TODO: Delete base parameter if it is always equal to GetImageBegin.
- void VisitPackedArtMethods(ArtMethodVisitor* visitor,
+ // NO_THREAD_SAFETY_ANALYSIS for template visitor pattern.
+ template <typename Visitor>
+ void VisitPackedArtMethods(const Visitor& visitor,
uint8_t* base,
- PointerSize pointer_size) const;
+ PointerSize pointer_size) const NO_THREAD_SAFETY_ANALYSIS;
// Visit ArtMethods in the section starting at base.
// TODO: Delete base parameter if it is always equal to GetImageBegin.
- void VisitPackedArtFields(ArtFieldVisitor* visitor, uint8_t* base) const;
+ // NO_THREAD_SAFETY_ANALYSIS for template visitor pattern.
+ template <typename Visitor>
+ void VisitPackedArtFields(const Visitor& visitor, uint8_t* base) const NO_THREAD_SAFETY_ANALYSIS;
template <typename Visitor>
void VisitPackedImTables(const Visitor& visitor,