From ec3835751d678c0b4a3fda47d11ffa107cd27e23 Mon Sep 17 00:00:00 2001 From: hpayer Date: Thu, 22 Sep 2016 07:32:28 -0700 Subject: [PATCH] [heap] Reland Concurrently free empty typed slot set chunks. BUG=chromium:648568 Review-Url: https://codereview.chromium.org/2365603002 Cr-Commit-Position: refs/heads/master@{#39630} --- src/heap/mark-compact.cc | 6 ++++ src/heap/remembered-set.cc | 3 +- src/heap/remembered-set.h | 13 ++++---- src/heap/slot-set.h | 38 +++++++++++++++++++++++- src/heap/spaces.cc | 13 ++++---- src/heap/spaces.h | 4 +-- test/unittests/heap/slot-set-unittest.cc | 37 +++++++++++++---------- 7 files changed, 83 insertions(+), 31 deletions(-) diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc index 08472e5b10..aae6593cc5 100644 --- a/src/heap/mark-compact.cc +++ b/src/heap/mark-compact.cc @@ -3865,6 +3865,12 @@ int MarkCompactCollector::Sweeper::ParallelSweepPage(Page* page, } else { max_freed = RawSweep(page, REBUILD_FREE_LIST, free_space_mode); } + + // After finishing sweeping of a page we clean up its remembered set. + if (page->typed_old_to_new_slots()) { + page->typed_old_to_new_slots()->FreeToBeFreedChunks(); + } + { base::LockGuard guard(&mutex_); swept_list_[identity].Add(page); diff --git a/src/heap/remembered-set.cc b/src/heap/remembered-set.cc index 6575d55d52..467f725008 100644 --- a/src/heap/remembered-set.cc +++ b/src/heap/remembered-set.cc @@ -36,7 +36,8 @@ void RememberedSet::ClearInvalidSlots(Heap* heap) { } else { return REMOVE_SLOT; } - }); + }, + TypedSlotSet::PREFREE_EMPTY_CHUNKS); } } for (MemoryChunk* chunk : *heap->map_space()) { diff --git a/src/heap/remembered-set.h b/src/heap/remembered-set.h index bdfae032ac..75aa524cde 100644 --- a/src/heap/remembered-set.h +++ b/src/heap/remembered-set.h @@ -149,10 +149,13 @@ class RememberedSet { static void RemoveRangeTyped(MemoryChunk* page, Address start, Address end) { TypedSlotSet* slots = GetTypedSlotSet(page); if (slots != nullptr) { - slots->Iterate([start, end](SlotType slot_type, Address host_addr, - Address slot_addr) { - return start <= slot_addr && slot_addr < end ? REMOVE_SLOT : KEEP_SLOT; - }); + slots->Iterate( + [start, end](SlotType slot_type, Address host_addr, + Address slot_addr) { + return start <= slot_addr && slot_addr < end ? REMOVE_SLOT + : KEEP_SLOT; + }, + TypedSlotSet::PREFREE_EMPTY_CHUNKS); } } @@ -173,7 +176,7 @@ class RememberedSet { static void IterateTyped(MemoryChunk* chunk, Callback callback) { TypedSlotSet* slots = GetTypedSlotSet(chunk); if (slots != nullptr) { - int new_count = slots->Iterate(callback); + int new_count = slots->Iterate(callback, TypedSlotSet::KEEP_EMPTY_CHUNKS); if (new_count == 0) { ReleaseTypedSlotSet(chunk); } diff --git a/src/heap/slot-set.h b/src/heap/slot-set.h index de732db454..f70def43ea 100644 --- a/src/heap/slot-set.h +++ b/src/heap/slot-set.h @@ -5,6 +5,8 @@ #ifndef V8_SLOT_SET_H #define V8_SLOT_SET_H +#include + #include "src/allocation.h" #include "src/base/atomic-utils.h" #include "src/base/bits.h" @@ -261,6 +263,8 @@ enum SlotType { // typed slots contain V8 internal pointers that are not directly exposed to JS. class TypedSlotSet { public: + enum IterationMode { PREFREE_EMPTY_CHUNKS, KEEP_EMPTY_CHUNKS }; + typedef std::pair TypeAndOffset; struct TypedSlot { @@ -328,6 +332,10 @@ class TypedSlotSet { void Insert(SlotType type, uint32_t host_offset, uint32_t offset) { TypedSlot slot(type, host_offset, offset); Chunk* top_chunk = chunk_.Value(); + if (!top_chunk) { + top_chunk = new Chunk(nullptr, kInitialBufferSize); + chunk_.SetValue(top_chunk); + } if (!top_chunk->AddSlot(slot)) { Chunk* new_top_chunk = new Chunk(top_chunk, NextCapacity(top_chunk->capacity.Value())); @@ -348,13 +356,15 @@ class TypedSlotSet { // else return REMOVE_SLOT; // }); template - int Iterate(Callback callback) { + int Iterate(Callback callback, IterationMode mode) { STATIC_ASSERT(CLEARED_SLOT < 8); Chunk* chunk = chunk_.Value(); + Chunk* previous = nullptr; int new_count = 0; while (chunk != nullptr) { TypedSlot* buffer = chunk->buffer.Value(); int count = chunk->count.Value(); + bool empty = true; for (int i = 0; i < count; i++) { // Order is important here. We have to read out the slot type last to // observe the concurrent removal case consistently. @@ -365,16 +375,40 @@ class TypedSlotSet { Address addr = page_start_ + type_and_offset.second; if (callback(type, host_addr, addr) == KEEP_SLOT) { new_count++; + empty = false; } else { buffer[i].Clear(); } } } + + if (mode == PREFREE_EMPTY_CHUNKS && empty) { + // We remove the chunk from the list but let it still point its next + // chunk to allow concurrent iteration. + if (previous) { + previous->next.SetValue(chunk->next.Value()); + } else { + chunk_.SetValue(chunk->next.Value()); + } + base::LockGuard guard(&to_be_freed_chunks_mutex_); + to_be_freed_chunks_.push(chunk); + } else { + previous = chunk; + } chunk = chunk->next.Value(); } return new_count; } + void FreeToBeFreedChunks() { + base::LockGuard guard(&to_be_freed_chunks_mutex_); + while (!to_be_freed_chunks_.empty()) { + Chunk* top = to_be_freed_chunks_.top(); + to_be_freed_chunks_.pop(); + delete top; + } + } + private: static const int kInitialBufferSize = 100; static const int kMaxBufferSize = 16 * KB; @@ -413,6 +447,8 @@ class TypedSlotSet { Address page_start_; base::AtomicValue chunk_; + base::Mutex to_be_freed_chunks_mutex_; + std::stack to_be_freed_chunks_; }; } // namespace internal diff --git a/src/heap/spaces.cc b/src/heap/spaces.cc index 8982a29532..1149dc51c3 100644 --- a/src/heap/spaces.cc +++ b/src/heap/spaces.cc @@ -510,7 +510,7 @@ MemoryChunk* MemoryChunk::Initialize(Heap* heap, Address base, size_t size, chunk->InitializeReservedMemory(); chunk->old_to_new_slots_ = nullptr; chunk->old_to_old_slots_ = nullptr; - chunk->typed_old_to_new_slots_ = nullptr; + chunk->typed_old_to_new_slots_.SetValue(nullptr); chunk->typed_old_to_old_slots_ = nullptr; chunk->skip_list_ = nullptr; chunk->write_barrier_counter_ = kWriteBarrierCounterGranularity; @@ -1077,7 +1077,7 @@ void MemoryChunk::ReleaseAllocatedMemory() { } if (old_to_new_slots_ != nullptr) ReleaseOldToNewSlots(); if (old_to_old_slots_ != nullptr) ReleaseOldToOldSlots(); - if (typed_old_to_new_slots_ != nullptr) ReleaseTypedOldToNewSlots(); + if (typed_old_to_new_slots_.Value() != nullptr) ReleaseTypedOldToNewSlots(); if (typed_old_to_old_slots_ != nullptr) ReleaseTypedOldToOldSlots(); if (local_tracker_ != nullptr) ReleaseLocalTracker(); } @@ -1113,13 +1113,14 @@ void MemoryChunk::ReleaseOldToOldSlots() { } void MemoryChunk::AllocateTypedOldToNewSlots() { - DCHECK(nullptr == typed_old_to_new_slots_); - typed_old_to_new_slots_ = new TypedSlotSet(address()); + DCHECK(nullptr == typed_old_to_new_slots_.Value()); + typed_old_to_new_slots_.SetValue(new TypedSlotSet(address())); } void MemoryChunk::ReleaseTypedOldToNewSlots() { - delete typed_old_to_new_slots_; - typed_old_to_new_slots_ = nullptr; + TypedSlotSet* typed_old_to_new_slots = typed_old_to_new_slots_.Value(); + delete typed_old_to_new_slots; + typed_old_to_new_slots_.SetValue(nullptr); } void MemoryChunk::AllocateTypedOldToOldSlots() { diff --git a/src/heap/spaces.h b/src/heap/spaces.h index 41e0727a3f..80f89f23ce 100644 --- a/src/heap/spaces.h +++ b/src/heap/spaces.h @@ -456,7 +456,7 @@ class MemoryChunk { inline SlotSet* old_to_new_slots() { return old_to_new_slots_; } inline SlotSet* old_to_old_slots() { return old_to_old_slots_; } inline TypedSlotSet* typed_old_to_new_slots() { - return typed_old_to_new_slots_; + return typed_old_to_new_slots_.Value(); } inline TypedSlotSet* typed_old_to_old_slots() { return typed_old_to_old_slots_; @@ -656,7 +656,7 @@ class MemoryChunk { // is ceil(size() / kPageSize). SlotSet* old_to_new_slots_; SlotSet* old_to_old_slots_; - TypedSlotSet* typed_old_to_new_slots_; + base::AtomicValue typed_old_to_new_slots_; TypedSlotSet* typed_old_to_old_slots_; SkipList* skip_list_; diff --git a/test/unittests/heap/slot-set-unittest.cc b/test/unittests/heap/slot-set-unittest.cc index 9bf85071a6..8db5b1a8fd 100644 --- a/test/unittests/heap/slot-set-unittest.cc +++ b/test/unittests/heap/slot-set-unittest.cc @@ -152,24 +152,29 @@ TEST(TypedSlotSet, Iterate) { ++added; } int iterated = 0; - set.Iterate([&iterated, kDelta, kHostDelta](SlotType type, Address host_addr, - Address addr) { - uint32_t i = static_cast(reinterpret_cast(addr)); - uint32_t j = static_cast(reinterpret_cast(host_addr)); - EXPECT_EQ(i % CLEARED_SLOT, static_cast(type)); - EXPECT_EQ(0, i % kDelta); - EXPECT_EQ(0, j % kHostDelta); - ++iterated; - return i % 2 == 0 ? KEEP_SLOT : REMOVE_SLOT; - }); + set.Iterate( + [&iterated, kDelta, kHostDelta](SlotType type, Address host_addr, + Address addr) { + uint32_t i = static_cast(reinterpret_cast(addr)); + uint32_t j = + static_cast(reinterpret_cast(host_addr)); + EXPECT_EQ(i % CLEARED_SLOT, static_cast(type)); + EXPECT_EQ(0, i % kDelta); + EXPECT_EQ(0, j % kHostDelta); + ++iterated; + return i % 2 == 0 ? KEEP_SLOT : REMOVE_SLOT; + }, + TypedSlotSet::KEEP_EMPTY_CHUNKS); EXPECT_EQ(added, iterated); iterated = 0; - set.Iterate([&iterated](SlotType type, Address host_addr, Address addr) { - uint32_t i = static_cast(reinterpret_cast(addr)); - EXPECT_EQ(0, i % 2); - ++iterated; - return KEEP_SLOT; - }); + set.Iterate( + [&iterated](SlotType type, Address host_addr, Address addr) { + uint32_t i = static_cast(reinterpret_cast(addr)); + EXPECT_EQ(0, i % 2); + ++iterated; + return KEEP_SLOT; + }, + TypedSlotSet::KEEP_EMPTY_CHUNKS); EXPECT_EQ(added / 2, iterated); }