From 8cf4ca8f75fe547fc1ad960936ea7d49dee3f14c Mon Sep 17 00:00:00 2001 From: Michael Lippautz Date: Fri, 3 Jul 2020 12:25:25 +0200 Subject: [PATCH] cppgc: Refactor visitation 3/3 Split off MarkingWorklists and from Marker and introduce MarkerBase. MarkerBase refers just to interfaces types for passing along visitors. The concrete Marker provides the impl for these interfaces. Unified heap marker uses different marking visitors internally but provides an implementation for the same interface. Change-Id: Ibc4b2c88e2e69bd303a95da7d167a701934f4a07 Bug: chromium:1056170 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2270539 Commit-Queue: Michael Lippautz Reviewed-by: Anton Bikineev Reviewed-by: Omer Katz Reviewed-by: Ulan Degenbaev Cr-Commit-Position: refs/heads/master@{#68676} --- BUILD.gn | 2 + src/heap/cppgc-js/cpp-heap.cc | 25 +++- src/heap/cppgc/heap-base.h | 6 +- src/heap/cppgc/marker.cc | 141 ++++++++---------- src/heap/cppgc/marker.h | 91 +++++------ src/heap/cppgc/marking-state.h | 22 +-- src/heap/cppgc/marking-worklists.cc | 32 ++++ src/heap/cppgc/marking-worklists.h | 72 +++++++++ src/heap/cppgc/write-barrier.cc | 13 +- test/unittests/heap/cppgc/marker-unittest.cc | 4 +- test/unittests/heap/cppgc/tests.h | 2 +- .../heap/cppgc/write-barrier-unittest.cc | 39 ++--- 12 files changed, 272 insertions(+), 177 deletions(-) create mode 100644 src/heap/cppgc/marking-worklists.cc create mode 100644 src/heap/cppgc/marking-worklists.h diff --git a/BUILD.gn b/BUILD.gn index 62007a242f..1627676872 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -4215,6 +4215,8 @@ v8_source_set("cppgc_base") { "src/heap/cppgc/marking-state.h", "src/heap/cppgc/marking-visitor.cc", "src/heap/cppgc/marking-visitor.h", + "src/heap/cppgc/marking-worklists.cc", + "src/heap/cppgc/marking-worklists.h", "src/heap/cppgc/object-allocator.cc", "src/heap/cppgc/object-allocator.h", "src/heap/cppgc/object-start-bitmap.h", diff --git a/src/heap/cppgc-js/cpp-heap.cc b/src/heap/cppgc-js/cpp-heap.cc index 04919c7f62..06116b36cb 100644 --- a/src/heap/cppgc-js/cpp-heap.cc +++ b/src/heap/cppgc-js/cpp-heap.cc @@ -59,21 +59,34 @@ class CppgcPlatformAdapter final : public cppgc::Platform { v8::Isolate* isolate_; }; -class UnifiedHeapMarker : public cppgc::internal::Marker { +class UnifiedHeapMarker : public cppgc::internal::MarkerBase { public: explicit UnifiedHeapMarker(cppgc::internal::HeapBase& heap); void AddObject(void*); - // TODO(chromium:1056170): Implement unified heap specific - // CreateMutatorThreadMarkingVisitor and AdvanceMarkingWithDeadline. + protected: + cppgc::Visitor& visitor() final { return marking_visitor_; } + cppgc::internal::ConservativeTracingVisitor& conservative_visitor() final { + return conservative_marking_visitor_; + } + heap::base::StackVisitor& stack_visitor() final { + return conservative_marking_visitor_; + } + + private: + // TODO(chromium:1056170): Implement unified heap specific marking visitors. + cppgc::internal::MarkingVisitor marking_visitor_; + cppgc::internal::ConservativeMarkingVisitor conservative_marking_visitor_; }; UnifiedHeapMarker::UnifiedHeapMarker(cppgc::internal::HeapBase& heap) - : cppgc::internal::Marker(heap) {} + : cppgc::internal::MarkerBase(heap), + marking_visitor_(heap, marking_state()), + conservative_marking_visitor_(heap, marking_state(), marking_visitor_) {} void UnifiedHeapMarker::AddObject(void* object) { - mutator_marking_state_->MarkAndPush( + mutator_marking_state_.MarkAndPush( cppgc::internal::HeapObjectHeader::FromPayload(object)); } @@ -97,7 +110,7 @@ void CppHeap::RegisterV8References( } void CppHeap::TracePrologue(TraceFlags flags) { - marker_ = std::make_unique(AsBase()); + marker_.reset(new UnifiedHeapMarker(AsBase())); const UnifiedHeapMarker::MarkingConfig marking_config{ UnifiedHeapMarker::MarkingConfig::CollectionType::kMajor, cppgc::Heap::StackState::kNoHeapPointers, diff --git a/src/heap/cppgc/heap-base.h b/src/heap/cppgc/heap-base.h index cc61ed32fc..7b476efcd4 100644 --- a/src/heap/cppgc/heap-base.h +++ b/src/heap/cppgc/heap-base.h @@ -35,7 +35,7 @@ namespace testing { class TestWithHeap; } -class Marker; +class MarkerBase; class PageBackend; class PreFinalizerHandler; class StatsCollector; @@ -90,7 +90,7 @@ class V8_EXPORT_PRIVATE HeapBase { return prefinalizer_handler_.get(); } - Marker* marker() const { return marker_.get(); } + MarkerBase* marker() const { return marker_.get(); } ObjectAllocator& object_allocator() { return object_allocator_; } @@ -128,7 +128,7 @@ class V8_EXPORT_PRIVATE HeapBase { std::unique_ptr stats_collector_; std::unique_ptr stack_; std::unique_ptr prefinalizer_handler_; - std::unique_ptr marker_; + std::unique_ptr marker_; ObjectAllocator object_allocator_; Sweeper sweeper_; diff --git a/src/heap/cppgc/marker.cc b/src/heap/cppgc/marker.cc index c7df664aef..def1767556 100644 --- a/src/heap/cppgc/marker.cc +++ b/src/heap/cppgc/marker.cc @@ -50,7 +50,7 @@ void ExitIncrementalMarkingIfNeeded(Marker::MarkingConfig config, } // Visit remembered set that was recorded in the generational barrier. -void VisitRememberedSlots(HeapBase& heap, MarkingVisitor* visitor) { +void VisitRememberedSlots(HeapBase& heap, MarkingState& marking_state) { #if defined(CPPGC_YOUNG_GENERATION) for (void* slot : heap.remembered_slots()) { auto& slot_header = BasePage::FromInnerAddress(&heap, slot) @@ -60,10 +60,11 @@ void VisitRememberedSlots(HeapBase& heap, MarkingVisitor* visitor) { // top level (with the guarantee that no objects are currently being in // construction). This can be ensured by running young GCs from safe points // or by reintroducing nested allocation scopes that avoid finalization. - DCHECK(!MarkingVisitor::IsInConstruction(slot_header)); + DCHECK( + !header.IsInConstruction()); void* value = *reinterpret_cast(slot); - visitor->DynamicallyMarkAddress(static_cast
(value)); + marking_state.DynamicallyMarkAddress(static_cast
(value)); } #endif } @@ -99,28 +100,25 @@ bool DrainWorklistWithDeadline(v8::base::TimeTicks deadline, Worklist* worklist, } // namespace -constexpr int Marker::kMutatorThreadId; - -Marker::Marker(HeapBase& heap) +MarkerBase::MarkerBase(HeapBase& heap) : heap_(heap), - mutator_marking_state_(std::make_unique( - heap, &marking_worklist_, ¬_fully_constructed_worklist_, - &weak_callback_worklist_, Marker::kMutatorThreadId)), - marking_visitor_(CreateMutatorThreadMarkingVisitor()), - conservative_marking_visitor_( - std::make_unique( - heap, *mutator_marking_state_.get(), *marking_visitor_.get())) {} + mutator_marking_state_( + heap, marking_worklists_.marking_worklist(), + marking_worklists_.not_fully_constructed_worklist(), + marking_worklists_.weak_callback_worklist(), + MarkingWorklists::kMutatorThreadId) {} -Marker::~Marker() { +MarkerBase::~MarkerBase() { // The fixed point iteration may have found not-fully-constructed objects. // Such objects should have already been found through the stack scan though // and should thus already be marked. - if (!not_fully_constructed_worklist_.IsEmpty()) { + if (!marking_worklists_.not_fully_constructed_worklist()->IsEmpty()) { #if DEBUG DCHECK_NE(MarkingConfig::StackState::kNoHeapPointers, config_.stack_state); - NotFullyConstructedItem item; - NotFullyConstructedWorklist::View view(¬_fully_constructed_worklist_, - kMutatorThreadId); + MarkingWorklists::NotFullyConstructedItem item; + MarkingWorklists::NotFullyConstructedWorklist::View view( + marking_worklists_.not_fully_constructed_worklist(), + MarkingWorklists::kMutatorThreadId); while (view.Pop(&item)) { const HeapObjectHeader& header = BasePage::FromPayload(item)->ObjectHeaderFromInnerAddress( @@ -128,12 +126,12 @@ Marker::~Marker() { DCHECK(header.IsMarked()); } #else - not_fully_constructed_worklist_.Clear(); + marking_worklists_.not_fully_constructed_worklist()->Clear(); #endif } } -void Marker::StartMarking(MarkingConfig config) { +void MarkerBase::StartMarking(MarkingConfig config) { heap().stats_collector()->NotifyMarkingStarted(); config_ = config; @@ -141,66 +139,62 @@ void Marker::StartMarking(MarkingConfig config) { EnterIncrementalMarkingIfNeeded(config, heap()); } -void Marker::EnterAtomicPause(MarkingConfig config) { +void MarkerBase::EnterAtomicPause(MarkingConfig config) { ExitIncrementalMarkingIfNeeded(config_, heap()); config_ = config; // VisitRoots also resets the LABs. VisitRoots(); if (config_.stack_state == MarkingConfig::StackState::kNoHeapPointers) { - FlushNotFullyConstructedObjects(); + marking_worklists_.FlushNotFullyConstructedObjects(); } else { MarkNotFullyConstructedObjects(); } } -void Marker::LeaveAtomicPause() { +void MarkerBase::LeaveAtomicPause() { ResetRememberedSet(heap()); heap().stats_collector()->NotifyMarkingCompleted( - mutator_marking_state_->marked_bytes()); + mutator_marking_state_.marked_bytes()); } -void Marker::FinishMarking(MarkingConfig config) { +void MarkerBase::FinishMarking(MarkingConfig config) { EnterAtomicPause(config); AdvanceMarkingWithDeadline(v8::base::TimeDelta::Max()); LeaveAtomicPause(); } -void Marker::ProcessWeakness() { - heap().GetWeakPersistentRegion().Trace(marking_visitor_.get()); +void MarkerBase::ProcessWeakness() { + heap().GetWeakPersistentRegion().Trace(&visitor()); // Call weak callbacks on objects that may now be pointing to dead objects. - WeakCallbackItem item; + MarkingWorklists::WeakCallbackItem item; LivenessBroker broker = LivenessBrokerFactory::Create(); - WeakCallbackWorklist::View view(&weak_callback_worklist_, kMutatorThreadId); + MarkingWorklists::WeakCallbackWorklist::View view( + marking_worklists_.weak_callback_worklist(), + MarkingWorklists::kMutatorThreadId); while (view.Pop(&item)) { item.callback(broker, item.parameter); } // Weak callbacks should not add any new objects for marking. - DCHECK(marking_worklist_.IsEmpty()); + DCHECK(marking_worklists_.marking_worklist()->IsEmpty()); } -void Marker::VisitRoots() { +void MarkerBase::VisitRoots() { // Reset LABs before scanning roots. LABs are cleared to allow // ObjectStartBitmap handling without considering LABs. heap().object_allocator().ResetLinearAllocationBuffers(); - heap().GetStrongPersistentRegion().Trace(marking_visitor_.get()); + heap().GetStrongPersistentRegion().Trace(&visitor()); if (config_.stack_state != MarkingConfig::StackState::kNoHeapPointers) { - heap().stack()->IteratePointers(conservative_marking_visitor_.get()); + heap().stack()->IteratePointers(&stack_visitor()); } if (config_.collection_type == MarkingConfig::CollectionType::kMinor) { - VisitRememberedSlots(heap(), marking_visitor_.get()); + VisitRememberedSlots(heap(), mutator_marking_state_); } } -std::unique_ptr -Marker::CreateMutatorThreadMarkingVisitor() { - return std::make_unique(this); -} - -bool Marker::AdvanceMarkingWithDeadline(v8::base::TimeDelta duration) { - MutatorThreadMarkingVisitor* visitor = marking_visitor_.get(); +bool MarkerBase::AdvanceMarkingWithDeadline(v8::base::TimeDelta duration) { v8::base::TimeTicks deadline = v8::base::TimeTicks::Now() + duration; do { @@ -208,70 +202,65 @@ bool Marker::AdvanceMarkingWithDeadline(v8::base::TimeDelta duration) { // |marking_worklist_|. This merely re-adds items with the proper // callbacks. if (!DrainWorklistWithDeadline( - deadline, &previously_not_fully_constructed_worklist_, - [this](NotFullyConstructedItem& item) { - mutator_marking_state_->DynamicallyMarkAddress( + deadline, + marking_worklists_.previously_not_fully_constructed_worklist(), + [this](MarkingWorklists::NotFullyConstructedItem& item) { + mutator_marking_state_.DynamicallyMarkAddress( reinterpret_cast(item)); }, - kMutatorThreadId)) + MarkingWorklists::kMutatorThreadId)) return false; if (!DrainWorklistWithDeadline( - deadline, &marking_worklist_, - [this, visitor](const MarkingItem& item) { + deadline, marking_worklists_.marking_worklist(), + [this](const MarkingWorklists::MarkingItem& item) { const HeapObjectHeader& header = HeapObjectHeader::FromPayload(item.base_object_payload); DCHECK(!header.IsInConstruction< HeapObjectHeader::AccessMode::kNonAtomic>()); - item.callback(visitor, item.base_object_payload); - mutator_marking_state_->AccountMarkedBytes(header); + item.callback(&visitor(), item.base_object_payload); + mutator_marking_state_.AccountMarkedBytes(header); }, - kMutatorThreadId)) + MarkingWorklists::kMutatorThreadId)) return false; if (!DrainWorklistWithDeadline( - deadline, &write_barrier_worklist_, - [this, visitor](HeapObjectHeader* header) { + deadline, marking_worklists_.write_barrier_worklist(), + [this](HeapObjectHeader* header) { DCHECK(header); DCHECK(!header->IsInConstruction< HeapObjectHeader::AccessMode::kNonAtomic>()); const GCInfo& gcinfo = GlobalGCInfoTable::GCInfoFromIndex(header->GetGCInfoIndex()); - gcinfo.trace(visitor, header->Payload()); - mutator_marking_state_->AccountMarkedBytes(*header); + gcinfo.trace(&visitor(), header->Payload()); + mutator_marking_state_.AccountMarkedBytes(*header); }, - kMutatorThreadId)) + MarkingWorklists::kMutatorThreadId)) return false; - } while (!marking_worklist_.IsLocalViewEmpty(kMutatorThreadId)); + } while (!marking_worklists_.marking_worklist()->IsLocalViewEmpty( + MarkingWorklists::kMutatorThreadId)); return true; } -void Marker::FlushNotFullyConstructedObjects() { - if (!not_fully_constructed_worklist_.IsLocalViewEmpty(kMutatorThreadId)) { - not_fully_constructed_worklist_.FlushToGlobal(kMutatorThreadId); - previously_not_fully_constructed_worklist_.MergeGlobalPool( - ¬_fully_constructed_worklist_); - } - DCHECK(not_fully_constructed_worklist_.IsLocalViewEmpty(kMutatorThreadId)); -} - -void Marker::MarkNotFullyConstructedObjects() { - NotFullyConstructedItem item; - NotFullyConstructedWorklist::View view(¬_fully_constructed_worklist_, - kMutatorThreadId); +void MarkerBase::MarkNotFullyConstructedObjects() { + MarkingWorklists::NotFullyConstructedItem item; + MarkingWorklists::NotFullyConstructedWorklist::View view( + marking_worklists_.not_fully_constructed_worklist(), + MarkingWorklists::kMutatorThreadId); while (view.Pop(&item)) { - conservative_marking_visitor_->TraceConservativelyIfNeeded(item); + conservative_visitor().TraceConservativelyIfNeeded(item); } } -void Marker::ClearAllWorklistsForTesting() { - marking_worklist_.Clear(); - not_fully_constructed_worklist_.Clear(); - previously_not_fully_constructed_worklist_.Clear(); - write_barrier_worklist_.Clear(); - weak_callback_worklist_.Clear(); +void MarkerBase::ClearAllWorklistsForTesting() { + marking_worklists_.ClearForTesting(); } +Marker::Marker(HeapBase& heap) + : MarkerBase(heap), + marking_visitor_(heap, marking_state()), + conservative_marking_visitor_(heap, marking_state(), marking_visitor_) {} + } // namespace internal } // namespace cppgc diff --git a/src/heap/cppgc/marker.h b/src/heap/cppgc/marker.h index 4db08b6273..fc6f09fc5b 100644 --- a/src/heap/cppgc/marker.h +++ b/src/heap/cppgc/marker.h @@ -10,17 +10,17 @@ #include "include/cppgc/heap.h" #include "include/cppgc/trace-trait.h" #include "include/cppgc/visitor.h" +#include "src/base/macros.h" #include "src/base/platform/time.h" +#include "src/heap/cppgc/marking-state.h" #include "src/heap/cppgc/marking-visitor.h" +#include "src/heap/cppgc/marking-worklists.h" #include "src/heap/cppgc/worklist.h" namespace cppgc { namespace internal { class HeapBase; -class HeapObjectHeader; -class MarkingState; -class MutatorThreadMarkingVisitor; // Marking algorithm. Example for a valid call sequence creating the marking // phase: @@ -31,31 +31,8 @@ class MutatorThreadMarkingVisitor; // 5. LeaveAtomicPause() // // Alternatively, FinishMarking combines steps 3.-5. -class V8_EXPORT_PRIVATE Marker { - static constexpr int kNumConcurrentMarkers = 0; - static constexpr int kNumMarkers = 1 + kNumConcurrentMarkers; - +class V8_EXPORT_PRIVATE MarkerBase { public: - static constexpr int kMutatorThreadId = 0; - - using MarkingItem = cppgc::TraceDescriptor; - using NotFullyConstructedItem = const void*; - struct WeakCallbackItem { - cppgc::WeakCallback callback; - const void* parameter; - }; - - // Segment size of 512 entries necessary to avoid throughput regressions. - // Since the work list is currently a temporary object this is not a problem. - using MarkingWorklist = - Worklist; - using NotFullyConstructedWorklist = - Worklist; - using WeakCallbackWorklist = - Worklist; - using WriteBarrierWorklist = - Worklist; - struct MarkingConfig { enum class CollectionType : uint8_t { kMinor, @@ -75,11 +52,10 @@ class V8_EXPORT_PRIVATE Marker { MarkingType marking_type = MarkingType::kAtomic; }; - explicit Marker(HeapBase& heap); - virtual ~Marker(); + virtual ~MarkerBase(); - Marker(const Marker&) = delete; - Marker& operator=(const Marker&) = delete; + MarkerBase(const MarkerBase&) = delete; + MarkerBase& operator=(const MarkerBase&) = delete; // Initialize marking according to the given config. This method will // trigger incremental/concurrent marking if needed. @@ -107,45 +83,46 @@ class V8_EXPORT_PRIVATE Marker { void ProcessWeakness(); HeapBase& heap() { return heap_; } - MarkingWorklist* marking_worklist() { return &marking_worklist_; } - NotFullyConstructedWorklist* not_fully_constructed_worklist() { - return ¬_fully_constructed_worklist_; - } - WriteBarrierWorklist* write_barrier_worklist() { - return &write_barrier_worklist_; - } - WeakCallbackWorklist* weak_callback_worklist() { - return &weak_callback_worklist_; - } - MarkingState& marking_state() const { return *mutator_marking_state_.get(); } + MarkingState& marking_state() { return mutator_marking_state_; } + MarkingWorklists& marking_worklists() { return marking_worklists_; } + cppgc::Visitor& VisitorForTesting() { return visitor(); } void ClearAllWorklistsForTesting(); - MutatorThreadMarkingVisitor* GetMarkingVisitorForTesting() { - return marking_visitor_.get(); - } - protected: - virtual std::unique_ptr - CreateMutatorThreadMarkingVisitor(); + explicit MarkerBase(HeapBase& heap); + + virtual cppgc::Visitor& visitor() = 0; + virtual ConservativeTracingVisitor& conservative_visitor() = 0; + virtual heap::base::StackVisitor& stack_visitor() = 0; void VisitRoots(); - void FlushNotFullyConstructedObjects(); void MarkNotFullyConstructedObjects(); HeapBase& heap_; MarkingConfig config_ = MarkingConfig::Default(); - std::unique_ptr mutator_marking_state_; - std::unique_ptr marking_visitor_; - std::unique_ptr conservative_marking_visitor_; + MarkingWorklists marking_worklists_; + MarkingState mutator_marking_state_; +}; - MarkingWorklist marking_worklist_; - NotFullyConstructedWorklist not_fully_constructed_worklist_; - NotFullyConstructedWorklist previously_not_fully_constructed_worklist_; - WriteBarrierWorklist write_barrier_worklist_; - WeakCallbackWorklist weak_callback_worklist_; +class V8_EXPORT_PRIVATE Marker final : public MarkerBase { + public: + explicit Marker(HeapBase&); + + protected: + cppgc::Visitor& visitor() final { return marking_visitor_; } + ConservativeTracingVisitor& conservative_visitor() final { + return conservative_marking_visitor_; + } + heap::base::StackVisitor& stack_visitor() final { + return conservative_marking_visitor_; + } + + private: + MarkingVisitor marking_visitor_; + ConservativeMarkingVisitor conservative_marking_visitor_; }; } // namespace internal diff --git a/src/heap/cppgc/marking-state.h b/src/heap/cppgc/marking-state.h index 00e33ddedf..9a4a782cad 100644 --- a/src/heap/cppgc/marking-state.h +++ b/src/heap/cppgc/marking-state.h @@ -10,7 +10,7 @@ #include "src/heap/cppgc/heap-object-header.h" #include "src/heap/cppgc/heap-page.h" #include "src/heap/cppgc/liveness-broker.h" -#include "src/heap/cppgc/marker.h" +#include "src/heap/cppgc/marking-worklists.h" namespace cppgc { namespace internal { @@ -18,9 +18,9 @@ namespace internal { // C++ marking implementation. class MarkingState { public: - inline MarkingState(HeapBase& heap, Marker::MarkingWorklist*, - Marker::NotFullyConstructedWorklist*, - Marker::WeakCallbackWorklist*, int); + inline MarkingState(HeapBase& heap, MarkingWorklists::MarkingWorklist*, + MarkingWorklists::NotFullyConstructedWorklist*, + MarkingWorklists::WeakCallbackWorklist*, int); MarkingState(const MarkingState&) = delete; MarkingState& operator=(const MarkingState&) = delete; @@ -47,17 +47,19 @@ class MarkingState { HeapBase& heap_; #endif // DEBUG - Marker::MarkingWorklist::View marking_worklist_; - Marker::NotFullyConstructedWorklist::View not_fully_constructed_worklist_; - Marker::WeakCallbackWorklist::View weak_callback_worklist_; + MarkingWorklists::MarkingWorklist::View marking_worklist_; + MarkingWorklists::NotFullyConstructedWorklist::View + not_fully_constructed_worklist_; + MarkingWorklists::WeakCallbackWorklist::View weak_callback_worklist_; size_t marked_bytes_ = 0; }; MarkingState::MarkingState( - HeapBase& heap, Marker::MarkingWorklist* marking_worklist, - Marker::NotFullyConstructedWorklist* not_fully_constructed_worklist, - Marker::WeakCallbackWorklist* weak_callback_worklist, int task_id) + HeapBase& heap, MarkingWorklists::MarkingWorklist* marking_worklist, + MarkingWorklists::NotFullyConstructedWorklist* + not_fully_constructed_worklist, + MarkingWorklists::WeakCallbackWorklist* weak_callback_worklist, int task_id) : #ifdef DEBUG heap_(heap), diff --git a/src/heap/cppgc/marking-worklists.cc b/src/heap/cppgc/marking-worklists.cc new file mode 100644 index 0000000000..ecbfe48d82 --- /dev/null +++ b/src/heap/cppgc/marking-worklists.cc @@ -0,0 +1,32 @@ + +// Copyright 2020 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "src/heap/cppgc/marking-worklists.h" + +namespace cppgc { +namespace internal { + +constexpr int MarkingWorklists::kMutatorThreadId; + +void MarkingWorklists::ClearForTesting() { + marking_worklist_.Clear(); + not_fully_constructed_worklist_.Clear(); + previously_not_fully_constructed_worklist_.Clear(); + write_barrier_worklist_.Clear(); + weak_callback_worklist_.Clear(); +} + +void MarkingWorklists::FlushNotFullyConstructedObjects() { + if (!not_fully_constructed_worklist_.IsLocalViewEmpty(kMutatorThreadId)) { + not_fully_constructed_worklist_.FlushToGlobal(kMutatorThreadId); + previously_not_fully_constructed_worklist_.MergeGlobalPool( + ¬_fully_constructed_worklist_); + } + DCHECK(not_fully_constructed_worklist_.IsLocalViewEmpty( + MarkingWorklists::kMutatorThreadId)); +} + +} // namespace internal +} // namespace cppgc diff --git a/src/heap/cppgc/marking-worklists.h b/src/heap/cppgc/marking-worklists.h new file mode 100644 index 0000000000..1404b8dc3e --- /dev/null +++ b/src/heap/cppgc/marking-worklists.h @@ -0,0 +1,72 @@ +// Copyright 2020 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef V8_HEAP_CPPGC_MARKING_WORKLISTS_H_ +#define V8_HEAP_CPPGC_MARKING_WORKLISTS_H_ + +#include "include/cppgc/visitor.h" +#include "src/heap/cppgc/worklist.h" + +namespace cppgc { +namespace internal { + +class HeapObjectHeader; + +class MarkingWorklists { + static constexpr int kNumConcurrentMarkers = 0; + static constexpr int kNumMarkers = 1 + kNumConcurrentMarkers; + + public: + static constexpr int kMutatorThreadId = 0; + + using MarkingItem = cppgc::TraceDescriptor; + using NotFullyConstructedItem = const void*; + struct WeakCallbackItem { + cppgc::WeakCallback callback; + const void* parameter; + }; + + // Segment size of 512 entries necessary to avoid throughput regressions. + // Since the work list is currently a temporary object this is not a problem. + using MarkingWorklist = + Worklist; + using NotFullyConstructedWorklist = + Worklist; + using WeakCallbackWorklist = + Worklist; + using WriteBarrierWorklist = + Worklist; + + MarkingWorklist* marking_worklist() { return &marking_worklist_; } + NotFullyConstructedWorklist* not_fully_constructed_worklist() { + return ¬_fully_constructed_worklist_; + } + NotFullyConstructedWorklist* previously_not_fully_constructed_worklist() { + return &previously_not_fully_constructed_worklist_; + } + WriteBarrierWorklist* write_barrier_worklist() { + return &write_barrier_worklist_; + } + WeakCallbackWorklist* weak_callback_worklist() { + return &weak_callback_worklist_; + } + + // Moves objects in not_fully_constructed_worklist_ to + // previously_not_full_constructed_worklists_. + void FlushNotFullyConstructedObjects(); + + void ClearForTesting(); + + private: + MarkingWorklist marking_worklist_; + NotFullyConstructedWorklist not_fully_constructed_worklist_; + NotFullyConstructedWorklist previously_not_fully_constructed_worklist_; + WriteBarrierWorklist write_barrier_worklist_; + WeakCallbackWorklist weak_callback_worklist_; +}; + +} // namespace internal +} // namespace cppgc + +#endif // V8_HEAP_CPPGC_MARKING_WORKLISTS_H_ diff --git a/src/heap/cppgc/write-barrier.cc b/src/heap/cppgc/write-barrier.cc index 5386c311d9..a15cc07beb 100644 --- a/src/heap/cppgc/write-barrier.cc +++ b/src/heap/cppgc/write-barrier.cc @@ -21,7 +21,7 @@ namespace internal { namespace { -void MarkValue(const BasePage* page, Marker* marker, const void* value) { +void MarkValue(const BasePage* page, MarkerBase* marker, const void* value) { #if defined(CPPGC_CAGED_HEAP) DCHECK(reinterpret_cast( reinterpret_cast(value) & @@ -40,14 +40,17 @@ void MarkValue(const BasePage* page, Marker* marker, const void* value) { // It is assumed that objects on not_fully_constructed_worklist_ are not // marked. header.Unmark(); - Marker::NotFullyConstructedWorklist::View not_fully_constructed_worklist( - marker->not_fully_constructed_worklist(), Marker::kMutatorThreadId); + MarkingWorklists::NotFullyConstructedWorklist::View + not_fully_constructed_worklist( + marker->marking_worklists().not_fully_constructed_worklist(), + MarkingWorklists::kMutatorThreadId); not_fully_constructed_worklist.Push(header.Payload()); return; } - Marker::WriteBarrierWorklist::View write_barrier_worklist( - marker->write_barrier_worklist(), Marker::kMutatorThreadId); + MarkingWorklists::WriteBarrierWorklist::View write_barrier_worklist( + marker->marking_worklists().write_barrier_worklist(), + MarkingWorklists::kMutatorThreadId); write_barrier_worklist.Push(&header); } diff --git a/test/unittests/heap/cppgc/marker-unittest.cc b/test/unittests/heap/cppgc/marker-unittest.cc index afcece579c..a07f3705ce 100644 --- a/test/unittests/heap/cppgc/marker-unittest.cc +++ b/test/unittests/heap/cppgc/marker-unittest.cc @@ -222,7 +222,7 @@ TEST_F(MarkerTest, InConstructionObjectIsEventuallyMarkedEmptyStack) { GCedWithCallback* object = MakeGarbageCollected( GetAllocationHandle(), [&marker](GCedWithCallback* obj) { Member member(obj); - marker.GetMarkingVisitorForTesting()->Trace(member); + marker.VisitorForTesting().Trace(member); }); EXPECT_FALSE(HeapObjectHeader::FromPayload(object).IsMarked()); marker.FinishMarking({MarkingConfig::CollectionType::kMajor, @@ -239,7 +239,7 @@ TEST_F(MarkerTest, InConstructionObjectIsEventuallyMarkedNonEmptyStack) { MakeGarbageCollected( GetAllocationHandle(), [&marker](GCedWithCallback* obj) { Member member(obj); - marker.GetMarkingVisitorForTesting()->Trace(member); + marker.VisitorForTesting().Trace(member); EXPECT_FALSE(HeapObjectHeader::FromPayload(obj).IsMarked()); marker.FinishMarking(config); EXPECT_TRUE(HeapObjectHeader::FromPayload(obj).IsMarked()); diff --git a/test/unittests/heap/cppgc/tests.h b/test/unittests/heap/cppgc/tests.h index d6e501d525..175116d985 100644 --- a/test/unittests/heap/cppgc/tests.h +++ b/test/unittests/heap/cppgc/tests.h @@ -43,7 +43,7 @@ class TestWithHeap : public TestWithPlatform { return allocation_handle_; } - std::unique_ptr& GetMarkerRef() { + std::unique_ptr& GetMarkerRef() { return Heap::From(GetHeap())->marker_; } diff --git a/test/unittests/heap/cppgc/write-barrier-unittest.cc b/test/unittests/heap/cppgc/write-barrier-unittest.cc index 01ebc383ab..c598f6c1b0 100644 --- a/test/unittests/heap/cppgc/write-barrier-unittest.cc +++ b/test/unittests/heap/cppgc/write-barrier-unittest.cc @@ -21,7 +21,7 @@ namespace { class IncrementalMarkingScope { public: - explicit IncrementalMarkingScope(Marker* marker) : marker_(marker) { + explicit IncrementalMarkingScope(MarkerBase* marker) : marker_(marker) { marker_->StartMarking(kIncrementalConfig); } @@ -35,18 +35,21 @@ class IncrementalMarkingScope { Marker::MarkingConfig::StackState::kNoHeapPointers, Marker::MarkingConfig::MarkingType::kIncremental}; - Marker* marker_; + MarkerBase* marker_; }; constexpr Marker::MarkingConfig IncrementalMarkingScope::kIncrementalConfig; class ExpectWriteBarrierFires final : private IncrementalMarkingScope { public: - ExpectWriteBarrierFires(Marker* marker, std::initializer_list objects) + ExpectWriteBarrierFires(MarkerBase* marker, + std::initializer_list objects) : IncrementalMarkingScope(marker), - marking_worklist_(marker->marking_worklist(), Marker::kMutatorThreadId), - write_barrier_worklist_(marker->write_barrier_worklist(), - Marker::kMutatorThreadId), + marking_worklist_(marker->marking_worklists().marking_worklist(), + MarkingWorklists::kMutatorThreadId), + write_barrier_worklist_( + marker->marking_worklists().write_barrier_worklist(), + MarkingWorklists::kMutatorThreadId), objects_(objects) { EXPECT_TRUE(marking_worklist_.IsGlobalPoolEmpty()); EXPECT_TRUE(write_barrier_worklist_.IsGlobalPoolEmpty()); @@ -58,7 +61,7 @@ class ExpectWriteBarrierFires final : private IncrementalMarkingScope { ~ExpectWriteBarrierFires() V8_NOEXCEPT { { - Marker::MarkingItem item; + MarkingWorklists::MarkingItem item; while (marking_worklist_.Pop(&item)) { auto pos = std::find(objects_.begin(), objects_.end(), item.base_object_payload); @@ -82,20 +85,22 @@ class ExpectWriteBarrierFires final : private IncrementalMarkingScope { } private: - Marker::MarkingWorklist::View marking_worklist_; - Marker::WriteBarrierWorklist::View write_barrier_worklist_; + MarkingWorklists::MarkingWorklist::View marking_worklist_; + MarkingWorklists::WriteBarrierWorklist::View write_barrier_worklist_; std::vector objects_; std::vector headers_; }; class ExpectNoWriteBarrierFires final : private IncrementalMarkingScope { public: - ExpectNoWriteBarrierFires(Marker* marker, + ExpectNoWriteBarrierFires(MarkerBase* marker, std::initializer_list objects) : IncrementalMarkingScope(marker), - marking_worklist_(marker->marking_worklist(), Marker::kMutatorThreadId), - write_barrier_worklist_(marker->write_barrier_worklist(), - Marker::kMutatorThreadId) { + marking_worklist_(marker->marking_worklists().marking_worklist(), + MarkingWorklists::kMutatorThreadId), + write_barrier_worklist_( + marker->marking_worklists().write_barrier_worklist(), + MarkingWorklists::kMutatorThreadId) { EXPECT_TRUE(marking_worklist_.IsGlobalPoolEmpty()); EXPECT_TRUE(write_barrier_worklist_.IsGlobalPoolEmpty()); for (void* object : objects) { @@ -113,8 +118,8 @@ class ExpectNoWriteBarrierFires final : private IncrementalMarkingScope { } private: - Marker::MarkingWorklist::View marking_worklist_; - Marker::WriteBarrierWorklist::View write_barrier_worklist_; + MarkingWorklists::MarkingWorklist::View marking_worklist_; + MarkingWorklists::WriteBarrierWorklist::View write_barrier_worklist_; std::vector> headers_; }; @@ -151,11 +156,11 @@ class WriteBarrierTest : public testing::TestWithHeap { GetMarkerRef().reset(); } - Marker* marker() const { return marker_; } + MarkerBase* marker() const { return marker_; } private: Heap* internal_heap_; - Marker* marker_; + MarkerBase* marker_; }; // =============================================================================