From a7b672c0690dfb8d2ebeea666ba9ea268a67bfe2 Mon Sep 17 00:00:00 2001 From: William Hesse Date: Fri, 15 May 2020 09:11:06 +0000 Subject: [PATCH] Revert "[vm, gc] Allow for more parallelism in old-space GCs." This reverts commit ae0740032ac912fb49be30789cff3f8fc8517c69. Reason for revert: This is crashing on mac-release. Crash logs: https://logs.chromium.org/logs/dart/buildbucket/cr-buildbucket.appspot.com/8880254409914739888/+/steps/test_results/0/logs/new_test_failures__logs_/0 Original change's description: > [vm, gc] Allow for more parallelism in old-space GCs. > > Allow work stealing of new-space page when marking roots. > > Change-Id: I18fc22934bea26a37341216e88b13d1fb6f83b36 > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/125767 > Reviewed-by: Martin Kustermann > Reviewed-by: Siva Annamalai > Commit-Queue: Ryan Macnak TBR=kustermann@google.com,aam@google.com,rmacnak@google.com,asiva@google.com Change-Id: I84c27cdd73a971f893ec968daaf5893c62c3a2e5 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/148129 Reviewed-by: William Hesse Reviewed-by: Martin Kustermann --- runtime/vm/heap/marker.cc | 28 ++----- runtime/vm/heap/marker.h | 2 - runtime/vm/heap/scavenger.cc | 136 ++++++++++++++++++++++++++++++++++ runtime/vm/heap/scavenger.h | 139 +---------------------------------- 4 files changed, 145 insertions(+), 160 deletions(-) diff --git a/runtime/vm/heap/marker.cc b/runtime/vm/heap/marker.cc index 76aa630db21..c99181ef9d8 100644 --- a/runtime/vm/heap/marker.cc +++ b/runtime/vm/heap/marker.cc @@ -332,38 +332,21 @@ void GCMarker::Epilogue() {} enum RootSlices { kIsolate = 0, - kNumRootSlices = 1, + kNewSpace = 1, + kNumRootSlices = 2, }; void GCMarker::ResetSlices() { - new_page_ = heap_->new_space()->head(); root_slices_started_ = 0; root_slices_finished_ = 0; weak_slices_started_ = 0; } void GCMarker::IterateRoots(ObjectPointerVisitor* visitor) { - for (;;) { - NewPage* page; - { - MonitorLocker ml(&root_slices_monitor_); - page = new_page_; - if (page != nullptr) { - new_page_ = page->next(); - } - } - if (page == nullptr) { - break; // Finished visiting new space. - } - - TIMELINE_FUNCTION_GC_DURATION(Thread::Current(), "ProcessNewSpace"); - page->VisitObjectPointers(visitor); - } - for (;;) { intptr_t slice = root_slices_started_.fetch_add(1); if (slice >= kNumRootSlices) { - break; // No more slices. + return; // No more slices. } switch (slice) { @@ -374,6 +357,11 @@ void GCMarker::IterateRoots(ObjectPointerVisitor* visitor) { visitor, ValidationPolicy::kDontValidateFrames); break; } + case kNewSpace: { + TIMELINE_FUNCTION_GC_DURATION(Thread::Current(), "ProcessNewSpace"); + heap_->new_space()->VisitObjectPointers(visitor); + break; + } default: UNREACHABLE(); } diff --git a/runtime/vm/heap/marker.h b/runtime/vm/heap/marker.h index b70b20d9330..7f78a58aa0d 100644 --- a/runtime/vm/heap/marker.h +++ b/runtime/vm/heap/marker.h @@ -19,7 +19,6 @@ class ObjectPointerVisitor; class PageSpace; template class MarkingVisitorBase; -class NewPage; class Thread; // The class GCMarker is used to mark reachable old generation objects as part @@ -66,7 +65,6 @@ class GCMarker { MarkingStack deferred_marking_stack_; MarkingVisitorBase** visitors_; - NewPage* new_page_; Monitor root_slices_monitor_; RelaxedAtomic root_slices_started_; intptr_t root_slices_finished_; diff --git a/runtime/vm/heap/scavenger.cc b/runtime/vm/heap/scavenger.cc index 489e47dd547..bcc6fc7e513 100644 --- a/runtime/vm/heap/scavenger.cc +++ b/runtime/vm/heap/scavenger.cc @@ -100,6 +100,142 @@ static inline void objcpy(void* dst, const void* src, size_t size) { } while (size > 0); } +static constexpr intptr_t kNewPageSize = 512 * KB; +static constexpr intptr_t kNewPageSizeInWords = kNewPageSize / kWordSize; +static constexpr intptr_t kNewPageMask = ~(kNewPageSize - 1); + +// A page containing new generation objects. +class NewPage { + public: + static NewPage* Allocate(); + void Deallocate(); + + uword start() const { return memory_->start(); } + uword end() const { return memory_->end(); } + bool Contains(uword addr) const { return memory_->Contains(addr); } + void WriteProtect(bool read_only) { + memory_->Protect(read_only ? VirtualMemory::kReadOnly + : VirtualMemory::kReadWrite); + } + + NewPage* next() const { return next_; } + void set_next(NewPage* next) { next_ = next; } + + Thread* owner() const { return owner_; } + + uword object_start() const { return start() + ObjectStartOffset(); } + uword object_end() const { return owner_ != nullptr ? owner_->top() : top_; } + void VisitObjects(ObjectVisitor* visitor) const { + uword addr = object_start(); + uword end = object_end(); + while (addr < end) { + ObjectPtr obj = ObjectLayout::FromAddr(addr); + visitor->VisitObject(obj); + addr += obj->ptr()->HeapSize(); + } + } + void VisitObjectPointers(ObjectPointerVisitor* visitor) const { + uword addr = object_start(); + uword end = object_end(); + while (addr < end) { + ObjectPtr obj = ObjectLayout::FromAddr(addr); + intptr_t size = obj->ptr()->VisitPointers(visitor); + addr += size; + } + } + + static intptr_t ObjectStartOffset() { + return Utils::RoundUp(sizeof(NewPage), kObjectAlignment) + + kNewObjectAlignmentOffset; + } + + static NewPage* Of(ObjectPtr obj) { + ASSERT(obj->IsHeapObject()); + ASSERT(obj->IsNewObject()); + return Of(static_cast(obj)); + } + static NewPage* Of(uword addr) { + return reinterpret_cast(addr & kNewPageMask); + } + + // Remember the limit to which objects have been copied. + void RecordSurvivors() { survivor_end_ = object_end(); } + + // Move survivor end to the end of the to_ space, making all surviving + // objects candidates for promotion next time. + void EarlyTenure() { survivor_end_ = end_; } + + uword promo_candidate_words() const { + return (survivor_end_ - object_start()) / kWordSize; + } + + void Acquire(Thread* thread) { + ASSERT(owner_ == nullptr); + owner_ = thread; + thread->set_top(top_); + thread->set_end(end_); + } + void Release(Thread* thread) { + ASSERT(owner_ == thread); + owner_ = nullptr; + top_ = thread->top(); + thread->set_top(0); + thread->set_end(0); + } + void Release() { + if (owner_ != nullptr) { + Release(owner_); + } + } + + uword TryAllocateGC(intptr_t size) { + ASSERT(owner_ == nullptr); + uword result = top_; + uword new_top = result + size; + if (LIKELY(new_top < end_)) { + top_ = new_top; + return result; + } + return 0; + } + + void Unallocate(uword addr, intptr_t size) { + ASSERT((addr + size) == top_); + top_ -= size; + } + + bool IsSurvivor(uword raw_addr) const { return raw_addr < survivor_end_; } + bool IsResolved() const { return top_ == resolved_top_; } + + private: + VirtualMemory* memory_; + NewPage* next_; + + // The thread using this page for allocation, otherwise NULL. + Thread* owner_; + + // The address of the next allocation. If owner is non-NULL, this value is + // stale and the current value is at owner->top_. Called "NEXT" in the + // original Cheney paper. + uword top_; + + // The address after the last allocatable byte in this page. + uword end_; + + // Objects below this address have survived a scavenge. + uword survivor_end_; + + // A pointer to the first unprocessed object. Resolution completes when this + // value meets the allocation top. Called "SCAN" in the original Cheney paper. + uword resolved_top_; + + template + friend class ScavengerVisitorBase; + + DISALLOW_ALLOCATION(); + DISALLOW_IMPLICIT_CONSTRUCTORS(NewPage); +}; + template class ScavengerVisitorBase : public ObjectPointerVisitor { public: diff --git a/runtime/vm/heap/scavenger.h b/runtime/vm/heap/scavenger.h index a6ff9ac7578..29469776b3f 100644 --- a/runtime/vm/heap/scavenger.h +++ b/runtime/vm/heap/scavenger.h @@ -24,146 +24,11 @@ namespace dart { class Heap; class Isolate; class JSONObject; +class NewPage; class ObjectSet; template class ScavengerVisitorBase; -static constexpr intptr_t kNewPageSize = 512 * KB; -static constexpr intptr_t kNewPageSizeInWords = kNewPageSize / kWordSize; -static constexpr intptr_t kNewPageMask = ~(kNewPageSize - 1); - -// A page containing new generation objects. -class NewPage { - public: - static NewPage* Allocate(); - void Deallocate(); - - uword start() const { return memory_->start(); } - uword end() const { return memory_->end(); } - bool Contains(uword addr) const { return memory_->Contains(addr); } - void WriteProtect(bool read_only) { - memory_->Protect(read_only ? VirtualMemory::kReadOnly - : VirtualMemory::kReadWrite); - } - - NewPage* next() const { return next_; } - void set_next(NewPage* next) { next_ = next; } - - Thread* owner() const { return owner_; } - - uword object_start() const { return start() + ObjectStartOffset(); } - uword object_end() const { return owner_ != nullptr ? owner_->top() : top_; } - void VisitObjects(ObjectVisitor* visitor) const { - uword addr = object_start(); - uword end = object_end(); - while (addr < end) { - ObjectPtr obj = ObjectLayout::FromAddr(addr); - visitor->VisitObject(obj); - addr += obj->ptr()->HeapSize(); - } - } - void VisitObjectPointers(ObjectPointerVisitor* visitor) const { - uword addr = object_start(); - uword end = object_end(); - while (addr < end) { - ObjectPtr obj = ObjectLayout::FromAddr(addr); - intptr_t size = obj->ptr()->VisitPointers(visitor); - addr += size; - } - } - - static intptr_t ObjectStartOffset() { - return Utils::RoundUp(sizeof(NewPage), kObjectAlignment) + - kNewObjectAlignmentOffset; - } - - static NewPage* Of(ObjectPtr obj) { - ASSERT(obj->IsHeapObject()); - ASSERT(obj->IsNewObject()); - return Of(static_cast(obj)); - } - static NewPage* Of(uword addr) { - return reinterpret_cast(addr & kNewPageMask); - } - - // Remember the limit to which objects have been copied. - void RecordSurvivors() { survivor_end_ = object_end(); } - - // Move survivor end to the end of the to_ space, making all surviving - // objects candidates for promotion next time. - void EarlyTenure() { survivor_end_ = end_; } - - uword promo_candidate_words() const { - return (survivor_end_ - object_start()) / kWordSize; - } - - void Acquire(Thread* thread) { - ASSERT(owner_ == nullptr); - owner_ = thread; - thread->set_top(top_); - thread->set_end(end_); - } - void Release(Thread* thread) { - ASSERT(owner_ == thread); - owner_ = nullptr; - top_ = thread->top(); - thread->set_top(0); - thread->set_end(0); - } - void Release() { - if (owner_ != nullptr) { - Release(owner_); - } - } - - uword TryAllocateGC(intptr_t size) { - ASSERT(owner_ == nullptr); - uword result = top_; - uword new_top = result + size; - if (LIKELY(new_top < end_)) { - top_ = new_top; - return result; - } - return 0; - } - - void Unallocate(uword addr, intptr_t size) { - ASSERT((addr + size) == top_); - top_ -= size; - } - - bool IsSurvivor(uword raw_addr) const { return raw_addr < survivor_end_; } - bool IsResolved() const { return top_ == resolved_top_; } - - private: - VirtualMemory* memory_; - NewPage* next_; - - // The thread using this page for allocation, otherwise NULL. - Thread* owner_; - - // The address of the next allocation. If owner is non-NULL, this value is - // stale and the current value is at owner->top_. Called "NEXT" in the - // original Cheney paper. - uword top_; - - // The address after the last allocatable byte in this page. - uword end_; - - // Objects below this address have survived a scavenge. - uword survivor_end_; - - // A pointer to the first unprocessed object. Resolution completes when this - // value meets the allocation top. Called "SCAN" in the original Cheney paper. - uword resolved_top_; - - template - friend class ScavengerVisitorBase; - - DISALLOW_ALLOCATION(); - DISALLOW_IMPLICIT_CONSTRUCTORS(NewPage); -}; - class SemiSpace { public: static void Init(); @@ -343,8 +208,6 @@ class Scavenger { return max_pool_size; } - NewPage* head() const { return to_->head(); } - private: // Ids for time and data records in Heap::GCStats. enum {