diff --git a/BUILD.gn b/BUILD.gn index ba52654401..62007a242f 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -4212,6 +4212,7 @@ v8_source_set("cppgc_base") { "src/heap/cppgc/logging.cc", "src/heap/cppgc/marker.cc", "src/heap/cppgc/marker.h", + "src/heap/cppgc/marking-state.h", "src/heap/cppgc/marking-visitor.cc", "src/heap/cppgc/marking-visitor.h", "src/heap/cppgc/object-allocator.cc", diff --git a/src/heap/cppgc-js/cpp-heap.cc b/src/heap/cppgc-js/cpp-heap.cc index d7d9dedd46..04919c7f62 100644 --- a/src/heap/cppgc-js/cpp-heap.cc +++ b/src/heap/cppgc-js/cpp-heap.cc @@ -14,6 +14,7 @@ #include "src/heap/cppgc/heap-base.h" #include "src/heap/cppgc/heap-object-header.h" #include "src/heap/cppgc/marker.h" +#include "src/heap/cppgc/marking-state.h" #include "src/heap/cppgc/marking-visitor.h" #include "src/heap/cppgc/object-allocator.h" #include "src/heap/cppgc/prefinalizer-handler.h" @@ -72,8 +73,8 @@ UnifiedHeapMarker::UnifiedHeapMarker(cppgc::internal::HeapBase& heap) : cppgc::internal::Marker(heap) {} void UnifiedHeapMarker::AddObject(void* object) { - auto& header = cppgc::internal::HeapObjectHeader::FromPayload(object); - marking_visitor_->MarkObject(header); + mutator_marking_state_->MarkAndPush( + cppgc::internal::HeapObjectHeader::FromPayload(object)); } } // namespace diff --git a/src/heap/cppgc/marker.cc b/src/heap/cppgc/marker.cc index 3bec6fa4d4..47ba18261c 100644 --- a/src/heap/cppgc/marker.cc +++ b/src/heap/cppgc/marker.cc @@ -4,12 +4,15 @@ #include "src/heap/cppgc/marker.h" +#include + #include "include/cppgc/internal/process-heap.h" #include "src/heap/cppgc/heap-object-header.h" #include "src/heap/cppgc/heap-page.h" #include "src/heap/cppgc/heap-visitor.h" #include "src/heap/cppgc/heap.h" #include "src/heap/cppgc/liveness-broker.h" +#include "src/heap/cppgc/marking-state.h" #include "src/heap/cppgc/marking-visitor.h" #include "src/heap/cppgc/stats-collector.h" @@ -99,7 +102,11 @@ bool DrainWorklistWithDeadline(v8::base::TimeTicks deadline, Worklist* worklist, constexpr int Marker::kMutatorThreadId; Marker::Marker(HeapBase& heap) - : heap_(heap), marking_visitor_(CreateMutatorThreadMarkingVisitor()) {} + : heap_(heap), + mutator_marking_state_(std::make_unique( + heap, &marking_worklist_, ¬_fully_constructed_worklist_, + &weak_callback_worklist_, Marker::kMutatorThreadId)), + marking_visitor_(CreateMutatorThreadMarkingVisitor()) {} Marker::~Marker() { // The fixed point iteration may have found not-fully-constructed objects. @@ -147,7 +154,7 @@ void Marker::EnterAtomicPause(MarkingConfig config) { void Marker::LeaveAtomicPause() { ResetRememberedSet(heap()); heap().stats_collector()->NotifyMarkingCompleted( - marking_visitor_->marked_bytes()); + mutator_marking_state_->marked_bytes()); } void Marker::FinishMarking(MarkingConfig config) { @@ -199,8 +206,8 @@ bool Marker::AdvanceMarkingWithDeadline(v8::base::TimeDelta duration) { // callbacks. if (!DrainWorklistWithDeadline( deadline, &previously_not_fully_constructed_worklist_, - [visitor](NotFullyConstructedItem& item) { - visitor->DynamicallyMarkAddress( + [this](NotFullyConstructedItem& item) { + mutator_marking_state_->DynamicallyMarkAddress( reinterpret_cast(item)); }, kMutatorThreadId)) @@ -208,25 +215,27 @@ bool Marker::AdvanceMarkingWithDeadline(v8::base::TimeDelta duration) { if (!DrainWorklistWithDeadline( deadline, &marking_worklist_, - [visitor](const MarkingItem& item) { + [this, visitor](const MarkingItem& item) { const HeapObjectHeader& header = HeapObjectHeader::FromPayload(item.base_object_payload); - DCHECK(!MutatorThreadMarkingVisitor::IsInConstruction(header)); + DCHECK(!header.IsInConstruction< + HeapObjectHeader::AccessMode::kNonAtomic>()); item.callback(visitor, item.base_object_payload); - visitor->AccountMarkedBytes(header); + mutator_marking_state_->AccountMarkedBytes(header); }, kMutatorThreadId)) return false; if (!DrainWorklistWithDeadline( deadline, &write_barrier_worklist_, - [visitor](HeapObjectHeader* header) { + [this, visitor](HeapObjectHeader* header) { DCHECK(header); - DCHECK(!MutatorThreadMarkingVisitor::IsInConstruction(*header)); + DCHECK(!header->IsInConstruction< + HeapObjectHeader::AccessMode::kNonAtomic>()); const GCInfo& gcinfo = GlobalGCInfoTable::GCInfoFromIndex(header->GetGCInfoIndex()); gcinfo.trace(visitor, header->Payload()); - visitor->AccountMarkedBytes(*header); + mutator_marking_state_->AccountMarkedBytes(*header); }, kMutatorThreadId)) return false; diff --git a/src/heap/cppgc/marker.h b/src/heap/cppgc/marker.h index 3edba06c4b..bde4d44792 100644 --- a/src/heap/cppgc/marker.h +++ b/src/heap/cppgc/marker.h @@ -18,6 +18,7 @@ namespace internal { class HeapBase; class HeapObjectHeader; +class MarkingState; class MutatorThreadMarkingVisitor; // Marking algorithm. Example for a valid call sequence creating the marking @@ -115,6 +116,7 @@ class V8_EXPORT_PRIVATE Marker { WeakCallbackWorklist* weak_callback_worklist() { return &weak_callback_worklist_; } + MarkingState& marking_state() const { return *mutator_marking_state_.get(); } void ClearAllWorklistsForTesting(); @@ -134,6 +136,7 @@ class V8_EXPORT_PRIVATE Marker { HeapBase& heap_; MarkingConfig config_ = MarkingConfig::Default(); + std::unique_ptr mutator_marking_state_; std::unique_ptr marking_visitor_; MarkingWorklist marking_worklist_; diff --git a/src/heap/cppgc/marking-state.h b/src/heap/cppgc/marking-state.h new file mode 100644 index 0000000000..00e33ddedf --- /dev/null +++ b/src/heap/cppgc/marking-state.h @@ -0,0 +1,168 @@ +// 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_STATE_H_ +#define V8_HEAP_CPPGC_MARKING_STATE_H_ + +#include "include/cppgc/trace-trait.h" +#include "src/heap/cppgc/globals.h" +#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" + +namespace cppgc { +namespace internal { + +// C++ marking implementation. +class MarkingState { + public: + inline MarkingState(HeapBase& heap, Marker::MarkingWorklist*, + Marker::NotFullyConstructedWorklist*, + Marker::WeakCallbackWorklist*, int); + + MarkingState(const MarkingState&) = delete; + MarkingState& operator=(const MarkingState&) = delete; + + inline void MarkAndPush(const void*, TraceDescriptor); + inline void MarkAndPush(HeapObjectHeader&, TraceDescriptor); + inline void MarkAndPush(HeapObjectHeader&); + + inline bool MarkNoPush(HeapObjectHeader&); + + inline void DynamicallyMarkAddress(ConstAddress); + + inline void RegisterWeakReferenceIfNeeded(const void*, TraceDescriptor, + WeakCallback, const void*); + inline void RegisterWeakCallback(WeakCallback, const void*); + inline void InvokeWeakRootsCallbackIfNeeded(const void*, TraceDescriptor, + WeakCallback, const void*); + + inline void AccountMarkedBytes(const HeapObjectHeader&); + size_t marked_bytes() const { return marked_bytes_; } + + private: +#ifdef DEBUG + HeapBase& heap_; +#endif // DEBUG + + Marker::MarkingWorklist::View marking_worklist_; + Marker::NotFullyConstructedWorklist::View not_fully_constructed_worklist_; + Marker::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) + : +#ifdef DEBUG + heap_(heap), +#endif // DEBUG + marking_worklist_(marking_worklist, task_id), + not_fully_constructed_worklist_(not_fully_constructed_worklist, task_id), + weak_callback_worklist_(weak_callback_worklist, task_id) { +} + +void MarkingState::MarkAndPush(const void* object, TraceDescriptor desc) { + DCHECK_NOT_NULL(object); + if (desc.base_object_payload == + cppgc::GarbageCollectedMixin::kNotFullyConstructedObject) { + // This means that the objects are not-yet-fully-constructed. See comments + // on GarbageCollectedMixin for how those objects are handled. + not_fully_constructed_worklist_.Push(object); + return; + } + MarkAndPush(HeapObjectHeader::FromPayload( + const_cast(desc.base_object_payload)), + desc); +} + +void MarkingState::MarkAndPush(HeapObjectHeader& header, TraceDescriptor desc) { + DCHECK_NOT_NULL(desc.callback); + + if (header.IsInConstruction()) { + not_fully_constructed_worklist_.Push(header.Payload()); + } else if (MarkNoPush(header)) { + marking_worklist_.Push(desc); + } +} + +bool MarkingState::MarkNoPush(HeapObjectHeader& header) { + // A GC should only mark the objects that belong in its heap. + DCHECK_EQ(&heap_, BasePage::FromPayload(&header)->heap()); + // Never mark free space objects. This would e.g. hint to marking a promptly + // freed backing store. + DCHECK(!header.IsFree()); + return header.TryMarkAtomic(); +} + +void MarkingState::DynamicallyMarkAddress(ConstAddress address) { + HeapObjectHeader& header = + BasePage::FromPayload(address)->ObjectHeaderFromInnerAddress( + const_cast
(address)); + DCHECK(!header.IsInConstruction()); + if (MarkNoPush(header)) { + marking_worklist_.Push( + {reinterpret_cast(header.Payload()), + GlobalGCInfoTable::GCInfoFromIndex(header.GetGCInfoIndex()).trace}); + } +} + +void MarkingState::MarkAndPush(HeapObjectHeader& header) { + MarkAndPush( + header, + {header.Payload(), + GlobalGCInfoTable::GCInfoFromIndex(header.GetGCInfoIndex()).trace}); +} + +void MarkingState::RegisterWeakReferenceIfNeeded(const void* object, + TraceDescriptor desc, + WeakCallback weak_callback, + const void* parameter) { + // Filter out already marked values. The write barrier for WeakMember + // ensures that any newly set value after this point is kept alive and does + // not require the callback. + if (desc.base_object_payload != + cppgc::GarbageCollectedMixin::kNotFullyConstructedObject && + HeapObjectHeader::FromPayload(desc.base_object_payload) + .IsMarked()) + return; + RegisterWeakCallback(weak_callback, parameter); +} + +void MarkingState::InvokeWeakRootsCallbackIfNeeded(const void* object, + TraceDescriptor desc, + WeakCallback weak_callback, + const void* parameter) { + if (desc.base_object_payload == + cppgc::GarbageCollectedMixin::kNotFullyConstructedObject) { + // This method is only called at the end of marking. If the object is in + // construction, then it should be reachable from the stack. + return; + } + // Since weak roots are only traced at the end of marking, we can execute + // the callback instead of registering it. + weak_callback(LivenessBrokerFactory::Create(), parameter); +} + +void MarkingState::RegisterWeakCallback(WeakCallback callback, + const void* object) { + weak_callback_worklist_.Push({callback, object}); +} + +void MarkingState::AccountMarkedBytes(const HeapObjectHeader& header) { + marked_bytes_ += + header.IsLargeObject() + ? reinterpret_cast(BasePage::FromPayload(&header)) + ->PayloadSize() + : header.GetSize(); +} + +} // namespace internal +} // namespace cppgc + +#endif // V8_HEAP_CPPGC_MARKING_STATE_H_ diff --git a/src/heap/cppgc/marking-visitor.cc b/src/heap/cppgc/marking-visitor.cc index aa15dce473..fd5140a7df 100644 --- a/src/heap/cppgc/marking-visitor.cc +++ b/src/heap/cppgc/marking-visitor.cc @@ -9,58 +9,24 @@ #include "src/heap/cppgc/heap-page.h" #include "src/heap/cppgc/heap.h" #include "src/heap/cppgc/liveness-broker.h" +#include "src/heap/cppgc/marking-state.h" namespace cppgc { namespace internal { -// static -bool MarkingVisitor::IsInConstruction(const HeapObjectHeader& header) { - return header.IsInConstruction(); -} - -MarkingVisitor::MarkingVisitor( - HeapBase& heap, Marker::MarkingWorklist* marking_worklist, - Marker::NotFullyConstructedWorklist* not_fully_constructed_worklist, - Marker::WeakCallbackWorklist* weak_callback_worklist, int task_id) +MarkingVisitor::MarkingVisitor(HeapBase& heap, MarkingState& marking_state) : ConservativeTracingVisitor(heap, *heap.page_backend()), - marking_worklist_(marking_worklist, task_id), - not_fully_constructed_worklist_(not_fully_constructed_worklist, task_id), - weak_callback_worklist_(weak_callback_worklist, task_id) {} - -void MarkingVisitor::AccountMarkedBytes(const HeapObjectHeader& header) { - marked_bytes_ += - header.IsLargeObject() - ? reinterpret_cast(BasePage::FromPayload(&header)) - ->PayloadSize() - : header.GetSize(); -} + marking_state_(marking_state) {} void MarkingVisitor::Visit(const void* object, TraceDescriptor desc) { - DCHECK_NOT_NULL(object); - if (desc.base_object_payload == - cppgc::GarbageCollectedMixin::kNotFullyConstructedObject) { - // This means that the objects are not-yet-fully-constructed. See comments - // on GarbageCollectedMixin for how those objects are handled. - not_fully_constructed_worklist_.Push(object); - return; - } - MarkHeader(&HeapObjectHeader::FromPayload( - const_cast(desc.base_object_payload)), - desc); + marking_state_.MarkAndPush(object, desc); } void MarkingVisitor::VisitWeak(const void* object, TraceDescriptor desc, WeakCallback weak_callback, const void* weak_member) { - // Filter out already marked values. The write barrier for WeakMember - // ensures that any newly set value after this point is kept alive and does - // not require the callback. - if (desc.base_object_payload != - cppgc::GarbageCollectedMixin::kNotFullyConstructedObject && - HeapObjectHeader::FromPayload(desc.base_object_payload) - .IsMarked()) - return; - RegisterWeakCallback(weak_callback, weak_member); + marking_state_.RegisterWeakReferenceIfNeeded(object, desc, weak_callback, + weak_member); } void MarkingVisitor::VisitRoot(const void* object, TraceDescriptor desc) { @@ -70,15 +36,8 @@ void MarkingVisitor::VisitRoot(const void* object, TraceDescriptor desc) { void MarkingVisitor::VisitWeakRoot(const void* object, TraceDescriptor desc, WeakCallback weak_callback, const void* weak_root) { - if (desc.base_object_payload == - cppgc::GarbageCollectedMixin::kNotFullyConstructedObject) { - // This method is only called at the end of marking. If the object is in - // construction, then it should be reachable from the stack. - return; - } - // Since weak roots are only traced at the end of marking, we can execute - // the callback instead of registering it. - weak_callback(LivenessBrokerFactory::Create(), weak_root); + marking_state_.InvokeWeakRootsCallbackIfNeeded(object, desc, weak_callback, + weak_root); } void MarkingVisitor::VisitPointer(const void* address) { @@ -87,69 +46,18 @@ void MarkingVisitor::VisitPointer(const void* address) { void MarkingVisitor::VisitConservatively(HeapObjectHeader& header, TraceConservativelyCallback callback) { - MarkHeaderNoTracing(&header); + marking_state_.MarkNoPush(header); callback(this, header); - AccountMarkedBytes(header); -} - -void MarkingVisitor::MarkHeader(HeapObjectHeader* header, - TraceDescriptor desc) { - DCHECK(header); - DCHECK_NOT_NULL(desc.callback); - - if (IsInConstruction(*header)) { - not_fully_constructed_worklist_.Push(header->Payload()); - } else if (MarkHeaderNoTracing(header)) { - marking_worklist_.Push(desc); - } -} - -bool MarkingVisitor::MarkHeaderNoTracing(HeapObjectHeader* header) { - DCHECK(header); - // A GC should only mark the objects that belong in its heap. - DCHECK_EQ(&heap_, BasePage::FromPayload(header)->heap()); - // Never mark free space objects. This would e.g. hint to marking a promptly - // freed backing store. - DCHECK(!header->IsFree()); - - return header->TryMarkAtomic(); + marking_state_.AccountMarkedBytes(header); } void MarkingVisitor::RegisterWeakCallback(WeakCallback callback, const void* object) { - weak_callback_worklist_.Push({callback, object}); -} - -void MarkingVisitor::FlushWorklists() { - marking_worklist_.FlushToGlobal(); - not_fully_constructed_worklist_.FlushToGlobal(); - weak_callback_worklist_.FlushToGlobal(); -} - -void MarkingVisitor::DynamicallyMarkAddress(ConstAddress address) { - HeapObjectHeader& header = - BasePage::FromPayload(address)->ObjectHeaderFromInnerAddress( - const_cast
(address)); - DCHECK(!IsInConstruction(header)); - if (MarkHeaderNoTracing(&header)) { - marking_worklist_.Push( - {reinterpret_cast(header.Payload()), - GlobalGCInfoTable::GCInfoFromIndex(header.GetGCInfoIndex()).trace}); - } -} - -void MarkingVisitor::MarkObject(HeapObjectHeader& header) { - MarkHeader( - &header, - {header.Payload(), - GlobalGCInfoTable::GCInfoFromIndex(header.GetGCInfoIndex()).trace}); + marking_state_.RegisterWeakCallback(callback, object); } MutatorThreadMarkingVisitor::MutatorThreadMarkingVisitor(Marker* marker) - : MarkingVisitor(marker->heap(), marker->marking_worklist(), - marker->not_fully_constructed_worklist(), - marker->weak_callback_worklist(), - Marker::kMutatorThreadId) {} + : MarkingVisitor(marker->heap(), marker->marking_state()) {} } // namespace internal } // namespace cppgc diff --git a/src/heap/cppgc/marking-visitor.h b/src/heap/cppgc/marking-visitor.h index 50427162a1..08968fadc5 100644 --- a/src/heap/cppgc/marking-visitor.h +++ b/src/heap/cppgc/marking-visitor.h @@ -5,66 +5,42 @@ #ifndef V8_HEAP_CPPGC_MARKING_VISITOR_H_ #define V8_HEAP_CPPGC_MARKING_VISITOR_H_ -#include "include/cppgc/source-location.h" #include "include/cppgc/trace-trait.h" -#include "include/v8config.h" #include "src/base/macros.h" #include "src/heap/base/stack.h" -#include "src/heap/cppgc/globals.h" -#include "src/heap/cppgc/heap.h" -#include "src/heap/cppgc/marker.h" #include "src/heap/cppgc/visitor.h" namespace cppgc { namespace internal { -class BasePage; +class HeapBase; class HeapObjectHeader; +class Marker; +class MarkingState; class MarkingVisitor : public ConservativeTracingVisitor, public heap::base::StackVisitor { public: - MarkingVisitor(HeapBase&, Marker::MarkingWorklist*, - Marker::NotFullyConstructedWorklist*, - Marker::WeakCallbackWorklist*, int); + MarkingVisitor(HeapBase&, MarkingState&); virtual ~MarkingVisitor() = default; MarkingVisitor(const MarkingVisitor&) = delete; MarkingVisitor& operator=(const MarkingVisitor&) = delete; - void FlushWorklists(); - - void DynamicallyMarkAddress(ConstAddress); - void MarkObject(HeapObjectHeader&); - - void AccountMarkedBytes(const HeapObjectHeader&); - size_t marked_bytes() const { return marked_bytes_; } - - static bool IsInConstruction(const HeapObjectHeader&); - - protected: - void Visit(const void*, TraceDescriptor) override; - void VisitWeak(const void*, TraceDescriptor, WeakCallback, - const void*) override; - void VisitRoot(const void*, TraceDescriptor) override; + private: + void Visit(const void*, TraceDescriptor) final; + void VisitWeak(const void*, TraceDescriptor, WeakCallback, const void*) final; + void VisitRoot(const void*, TraceDescriptor) final; void VisitWeakRoot(const void*, TraceDescriptor, WeakCallback, - const void*) override; + const void*) final; void VisitConservatively(HeapObjectHeader&, - TraceConservativelyCallback) override; + TraceConservativelyCallback) final; + void RegisterWeakCallback(WeakCallback, const void*) final; // StackMarker interface. void VisitPointer(const void*) override; - private: - void MarkHeader(HeapObjectHeader*, TraceDescriptor); - bool MarkHeaderNoTracing(HeapObjectHeader*); - void RegisterWeakCallback(WeakCallback, const void*) override; - - Marker::MarkingWorklist::View marking_worklist_; - Marker::NotFullyConstructedWorklist::View not_fully_constructed_worklist_; - Marker::WeakCallbackWorklist::View weak_callback_worklist_; - - size_t marked_bytes_ = 0; + MarkingState& marking_state_; }; class V8_EXPORT_PRIVATE MutatorThreadMarkingVisitor : public MarkingVisitor { diff --git a/src/heap/cppgc/write-barrier.cc b/src/heap/cppgc/write-barrier.cc index 389a1943e6..5386c311d9 100644 --- a/src/heap/cppgc/write-barrier.cc +++ b/src/heap/cppgc/write-barrier.cc @@ -34,7 +34,9 @@ void MarkValue(const BasePage* page, Marker* marker, const void* value) { DCHECK(marker); - if (V8_UNLIKELY(MutatorThreadMarkingVisitor::IsInConstruction(header))) { + if (V8_UNLIKELY( + header + .IsInConstruction())) { // It is assumed that objects on not_fully_constructed_worklist_ are not // marked. header.Unmark(); diff --git a/test/unittests/heap/cppgc/marking-visitor-unittest.cc b/test/unittests/heap/cppgc/marking-visitor-unittest.cc index 1792ffaab7..3ee20b86d1 100644 --- a/test/unittests/heap/cppgc/marking-visitor-unittest.cc +++ b/test/unittests/heap/cppgc/marking-visitor-unittest.cc @@ -11,6 +11,7 @@ #include "src/heap/cppgc/globals.h" #include "src/heap/cppgc/heap-object-header.h" #include "src/heap/cppgc/marker.h" +#include "src/heap/cppgc/marking-state.h" #include "test/unittests/heap/cppgc/tests.h" #include "testing/gtest/include/gtest/gtest.h" @@ -47,11 +48,10 @@ class GCedWithMixin : public GarbageCollected, public Mixin { } // namespace TEST_F(MarkingVisitorTest, MarkedBytesAreInitiallyZero) { - MutatorThreadMarkingVisitor visitor(GetMarker()); - EXPECT_EQ(0u, visitor.marked_bytes()); + EXPECT_EQ(0u, GetMarker()->marking_state().marked_bytes()); } -// Strong refernces are marked. +// Strong references are marked. TEST_F(MarkingVisitorTest, MarkMember) { Member object(MakeGarbageCollected(GetAllocationHandle()));