From c8950822095a4f5e6e87cfe7427d1bbb2a78de43 Mon Sep 17 00:00:00 2001 From: Mathieu Chartier Date: Tue, 15 Sep 2015 14:07:10 -0700 Subject: [PATCH] Clean up mod union table Fixed some formatting and removed unused forward declarations. Bug: 19534862 Change-Id: Icfd5143e5bb7be0570248fd0f4bbf97125a9a35b --- runtime/gc/accounting/mod_union_table-inl.h | 3 +- runtime/gc/accounting/mod_union_table.cc | 102 +++++++++++++++------------- runtime/gc/accounting/mod_union_table.h | 19 ++---- 3 files changed, 64 insertions(+), 60 deletions(-) diff --git a/runtime/gc/accounting/mod_union_table-inl.h b/runtime/gc/accounting/mod_union_table-inl.h index c756127a5..3a09634c0 100644 --- a/runtime/gc/accounting/mod_union_table-inl.h +++ b/runtime/gc/accounting/mod_union_table-inl.h @@ -28,7 +28,8 @@ namespace accounting { // A mod-union table to record image references to the Zygote and alloc space. class ModUnionTableToZygoteAllocspace : public ModUnionTableReferenceCache { public: - explicit ModUnionTableToZygoteAllocspace(const std::string& name, Heap* heap, + explicit ModUnionTableToZygoteAllocspace(const std::string& name, + Heap* heap, space::ContinuousSpace* space) : ModUnionTableReferenceCache(name, heap, space) {} diff --git a/runtime/gc/accounting/mod_union_table.cc b/runtime/gc/accounting/mod_union_table.cc index 5151819d9..1361f7bcc 100644 --- a/runtime/gc/accounting/mod_union_table.cc +++ b/runtime/gc/accounting/mod_union_table.cc @@ -27,9 +27,7 @@ #include "gc/space/space.h" #include "mirror/object-inl.h" #include "space_bitmap-inl.h" -#include "thread.h" - -using ::art::mirror::Object; +#include "thread-inl.h" namespace art { namespace gc { @@ -38,10 +36,10 @@ namespace accounting { class ModUnionAddToCardSetVisitor { public: explicit ModUnionAddToCardSetVisitor(ModUnionTable::CardSet* const cleared_cards) - : cleared_cards_(cleared_cards) { - } + : cleared_cards_(cleared_cards) {} - inline void operator()(uint8_t* card, uint8_t expected_value, + inline void operator()(uint8_t* card, + uint8_t expected_value, uint8_t new_value ATTRIBUTE_UNUSED) const { if (expected_value == CardTable::kCardDirty) { cleared_cards_->insert(card); @@ -55,10 +53,10 @@ class ModUnionAddToCardSetVisitor { class ModUnionAddToCardBitmapVisitor { public: ModUnionAddToCardBitmapVisitor(ModUnionTable::CardBitmap* bitmap, CardTable* card_table) - : bitmap_(bitmap), card_table_(card_table) { - } + : bitmap_(bitmap), card_table_(card_table) {} - inline void operator()(uint8_t* card, uint8_t expected_value, + inline void operator()(uint8_t* card, + uint8_t expected_value, uint8_t new_value ATTRIBUTE_UNUSED) const { if (expected_value == CardTable::kCardDirty) { // We want the address the card represents, not the address of the card. @@ -93,12 +91,13 @@ class ModUnionUpdateObjectReferencesVisitor { space::ContinuousSpace* from_space, space::ContinuousSpace* immune_space, bool* contains_reference_to_other_space) - : visitor_(visitor), from_space_(from_space), immune_space_(immune_space), - contains_reference_to_other_space_(contains_reference_to_other_space) { - } + : visitor_(visitor), + from_space_(from_space), + immune_space_(immune_space), + contains_reference_to_other_space_(contains_reference_to_other_space) {} // Extra parameters are required since we use this same visitor signature for checking objects. - void operator()(Object* obj, MemberOffset offset, bool is_static ATTRIBUTE_UNUSED) const + void operator()(mirror::Object* obj, MemberOffset offset, bool is_static ATTRIBUTE_UNUSED) const SHARED_REQUIRES(Locks::mutator_lock_) { MarkReference(obj->GetFieldObjectReferenceAddr(offset)); } @@ -144,14 +143,18 @@ class ModUnionScanImageRootVisitor { space::ContinuousSpace* from_space, space::ContinuousSpace* immune_space, bool* contains_reference_to_other_space) - : visitor_(visitor), from_space_(from_space), immune_space_(immune_space), + : visitor_(visitor), + from_space_(from_space), + immune_space_(immune_space), contains_reference_to_other_space_(contains_reference_to_other_space) {} - void operator()(Object* root) const + void operator()(mirror::Object* root) const REQUIRES(Locks::heap_bitmap_lock_) SHARED_REQUIRES(Locks::mutator_lock_) { DCHECK(root != nullptr); - ModUnionUpdateObjectReferencesVisitor ref_visitor(visitor_, from_space_, immune_space_, + ModUnionUpdateObjectReferencesVisitor ref_visitor(visitor_, + from_space_, + immune_space_, contains_reference_to_other_space_); root->VisitReferences(ref_visitor, VoidFunctor()); } @@ -176,7 +179,7 @@ class AddToReferenceArrayVisitor { public: AddToReferenceArrayVisitor(ModUnionTableReferenceCache* mod_union_table, MarkObjectVisitor* visitor, - std::vector*>* references, + std::vector*>* references, bool* has_target_reference) : mod_union_table_(mod_union_table), visitor_(visitor), @@ -184,9 +187,9 @@ class AddToReferenceArrayVisitor { has_target_reference_(has_target_reference) {} // Extra parameters are required since we use this same visitor signature for checking objects. - void operator()(Object* obj, MemberOffset offset, bool is_static ATTRIBUTE_UNUSED) const + void operator()(mirror::Object* obj, MemberOffset offset, bool is_static ATTRIBUTE_UNUSED) const SHARED_REQUIRES(Locks::mutator_lock_) { - mirror::HeapReference* ref_ptr = obj->GetFieldObjectReferenceAddr(offset); + mirror::HeapReference* ref_ptr = obj->GetFieldObjectReferenceAddr(offset); mirror::Object* ref = ref_ptr->AsMirrorPtr(); // Only add the reference if it is non null and fits our criteria. if (ref != nullptr && mod_union_table_->ShouldAddReference(ref)) { @@ -214,7 +217,7 @@ class AddToReferenceArrayVisitor { private: ModUnionTableReferenceCache* const mod_union_table_; MarkObjectVisitor* const visitor_; - std::vector*>* const references_; + std::vector*>* const references_; bool* const has_target_reference_; }; @@ -222,14 +225,14 @@ class ModUnionReferenceVisitor { public: ModUnionReferenceVisitor(ModUnionTableReferenceCache* const mod_union_table, MarkObjectVisitor* visitor, - std::vector*>* references, + std::vector*>* references, bool* has_target_reference) : mod_union_table_(mod_union_table), visitor_(visitor), references_(references), has_target_reference_(has_target_reference) {} - void operator()(Object* obj) const + void operator()(mirror::Object* obj) const SHARED_REQUIRES(Locks::heap_bitmap_lock_, Locks::mutator_lock_) { // We don't have an early exit since we use the visitor pattern, an early // exit should significantly speed this up. @@ -243,23 +246,23 @@ class ModUnionReferenceVisitor { private: ModUnionTableReferenceCache* const mod_union_table_; MarkObjectVisitor* const visitor_; - std::vector*>* const references_; + std::vector*>* const references_; bool* const has_target_reference_; }; class CheckReferenceVisitor { public: CheckReferenceVisitor(ModUnionTableReferenceCache* mod_union_table, - const std::set& references) + const std::set& references) : mod_union_table_(mod_union_table), - references_(references) { - } + references_(references) {} // Extra parameters are required since we use this same visitor signature for checking objects. - void operator()(Object* obj, MemberOffset offset, bool is_static ATTRIBUTE_UNUSED) const + void operator()(mirror::Object* obj, MemberOffset offset, bool is_static ATTRIBUTE_UNUSED) const SHARED_REQUIRES(Locks::heap_bitmap_lock_, Locks::mutator_lock_) { mirror::Object* ref = obj->GetFieldObject(offset); - if (ref != nullptr && mod_union_table_->ShouldAddReference(ref) && + if (ref != nullptr && + mod_union_table_->ShouldAddReference(ref) && references_.find(ref) == references_.end()) { Heap* heap = mod_union_table_->GetHeap(); space::ContinuousSpace* from_space = heap->FindContinuousSpaceFromObject(obj, false); @@ -290,18 +293,17 @@ class CheckReferenceVisitor { private: ModUnionTableReferenceCache* const mod_union_table_; - const std::set& references_; + const std::set& references_; }; class ModUnionCheckReferences { public: ModUnionCheckReferences(ModUnionTableReferenceCache* mod_union_table, - const std::set& references) + const std::set& references) REQUIRES(Locks::heap_bitmap_lock_) - : mod_union_table_(mod_union_table), references_(references) { - } + : mod_union_table_(mod_union_table), references_(references) {} - void operator()(Object* obj) const NO_THREAD_SAFETY_ANALYSIS { + void operator()(mirror::Object* obj) const NO_THREAD_SAFETY_ANALYSIS { Locks::heap_bitmap_lock_->AssertSharedHeld(Thread::Current()); CheckReferenceVisitor visitor(mod_union_table_, references_); obj->VisitReferences(visitor, VoidFunctor()); @@ -309,13 +311,13 @@ class ModUnionCheckReferences { private: ModUnionTableReferenceCache* const mod_union_table_; - const std::set& references_; + const std::set& references_; }; void ModUnionTableReferenceCache::Verify() { // Start by checking that everything in the mod union table is marked. for (const auto& ref_pair : references_) { - for (mirror::HeapReference* ref : ref_pair.second) { + for (mirror::HeapReference* ref : ref_pair.second) { CHECK(heap_->IsLiveObjectLocked(ref->AsMirrorPtr())); } } @@ -326,8 +328,8 @@ void ModUnionTableReferenceCache::Verify() { for (const auto& ref_pair : references_) { const uint8_t* card = ref_pair.first; if (*card == CardTable::kCardClean) { - std::set reference_set; - for (mirror::HeapReference* obj_ptr : ref_pair.second) { + std::set reference_set; + for (mirror::HeapReference* obj_ptr : ref_pair.second) { reference_set.insert(obj_ptr->AsMirrorPtr()); } ModUnionCheckReferences visitor(this, reference_set); @@ -351,7 +353,7 @@ void ModUnionTableReferenceCache::Dump(std::ostream& os) { uintptr_t start = reinterpret_cast(card_table->AddrFromCard(card_addr)); uintptr_t end = start + CardTable::kCardSize; os << reinterpret_cast(start) << "-" << reinterpret_cast(end) << "->{"; - for (mirror::HeapReference* ref : ref_pair.second) { + for (mirror::HeapReference* ref : ref_pair.second) { os << reinterpret_cast(ref->AsMirrorPtr()) << ","; } os << "},"; @@ -360,7 +362,7 @@ void ModUnionTableReferenceCache::Dump(std::ostream& os) { void ModUnionTableReferenceCache::UpdateAndMarkReferences(MarkObjectVisitor* visitor) { CardTable* const card_table = heap_->GetCardTable(); - std::vector*> cards_references; + std::vector*> cards_references; // If has_target_reference is true then there was a GcRoot compressed reference which wasn't // added. In this case we need to keep the card dirty. // We don't know if the GcRoot addresses will remain constant, for example, classloaders have a @@ -375,7 +377,7 @@ void ModUnionTableReferenceCache::UpdateAndMarkReferences(MarkObjectVisitor* vis uintptr_t start = reinterpret_cast(card_table->AddrFromCard(card)); uintptr_t end = start + CardTable::kCardSize; space::ContinuousSpace* space = - heap_->FindContinuousSpaceFromObject(reinterpret_cast(start), false); + heap_->FindContinuousSpaceFromObject(reinterpret_cast(start), false); DCHECK(space != nullptr); ContinuousSpaceBitmap* live_bitmap = space->GetLiveBitmap(); live_bitmap->VisitMarkedRange(start, end, add_visitor); @@ -402,12 +404,12 @@ void ModUnionTableReferenceCache::UpdateAndMarkReferences(MarkObjectVisitor* vis cleared_cards_ = std::move(new_cleared_cards); size_t count = 0; for (auto it = references_.begin(); it != references_.end();) { - std::vector*>& references = it->second; + std::vector*>& references = it->second; // Since there is no card mark for setting a reference to null, we check each reference. // If all of the references of a card are null then we can remove that card. This is racy // with the mutators, but handled by rescanning dirty cards. bool all_null = true; - for (mirror::HeapReference* obj_ptr : references) { + for (mirror::HeapReference* obj_ptr : references) { if (obj_ptr->AsMirrorPtr() != nullptr) { all_null = false; visitor->MarkHeapReference(obj_ptr); @@ -426,7 +428,8 @@ void ModUnionTableReferenceCache::UpdateAndMarkReferences(MarkObjectVisitor* vis } } -ModUnionTableCardCache::ModUnionTableCardCache(const std::string& name, Heap* heap, +ModUnionTableCardCache::ModUnionTableCardCache(const std::string& name, + Heap* heap, space::ContinuousSpace* space) : ModUnionTable(name, heap, space) { // Normally here we could use End() instead of Limit(), but for testing we may want to have a @@ -441,10 +444,15 @@ ModUnionTableCardCache::ModUnionTableCardCache(const std::string& name, Heap* he class CardBitVisitor { public: - CardBitVisitor(MarkObjectVisitor* visitor, space::ContinuousSpace* space, - space::ContinuousSpace* immune_space, ModUnionTable::CardBitmap* card_bitmap) - : visitor_(visitor), space_(space), immune_space_(immune_space), - bitmap_(space->GetLiveBitmap()), card_bitmap_(card_bitmap) { + CardBitVisitor(MarkObjectVisitor* visitor, + space::ContinuousSpace* space, + space::ContinuousSpace* immune_space, + ModUnionTable::CardBitmap* card_bitmap) + : visitor_(visitor), + space_(space), + immune_space_(immune_space), + bitmap_(space->GetLiveBitmap()), + card_bitmap_(card_bitmap) { DCHECK(immune_space_ != nullptr); } diff --git a/runtime/gc/accounting/mod_union_table.h b/runtime/gc/accounting/mod_union_table.h index 5888193e7..a7a4246fc 100644 --- a/runtime/gc/accounting/mod_union_table.h +++ b/runtime/gc/accounting/mod_union_table.h @@ -29,26 +29,17 @@ namespace art { namespace mirror { - class Object; +class Object; } // namespace mirror namespace gc { - -namespace collector { - class MarkSweep; -} // namespace collector namespace space { class ContinuousSpace; - class Space; } // namespace space - class Heap; namespace accounting { -class Bitmap; -class HeapBitmap; - // The mod-union table is the union of modified cards. It is used to allow the card table to be // cleared between GC phases, reducing the number of dirty cards that need to be scanned. class ModUnionTable { @@ -60,8 +51,7 @@ class ModUnionTable { explicit ModUnionTable(const std::string& name, Heap* heap, space::ContinuousSpace* space) : name_(name), heap_(heap), - space_(space) { - } + space_(space) {} virtual ~ModUnionTable() {} @@ -89,12 +79,15 @@ class ModUnionTable { virtual bool ContainsCardFor(uintptr_t addr) = 0; virtual void Dump(std::ostream& os) = 0; + space::ContinuousSpace* GetSpace() { return space_; } + Heap* GetHeap() const { return heap_; } + const std::string& GetName() const { return name_; } @@ -111,6 +104,7 @@ class ModUnionTableReferenceCache : public ModUnionTable { explicit ModUnionTableReferenceCache(const std::string& name, Heap* heap, space::ContinuousSpace* space) : ModUnionTable(name, heap, space) {} + virtual ~ModUnionTableReferenceCache() {} // Clear and store cards for a space. @@ -151,6 +145,7 @@ class ModUnionTableCardCache : public ModUnionTable { // Note: There is assumption that the space End() doesn't change. explicit ModUnionTableCardCache(const std::string& name, Heap* heap, space::ContinuousSpace* space); + virtual ~ModUnionTableCardCache() {} // Clear and store cards for a space. -- 2.11.0