diff --git a/include/cppgc/visitor.h b/include/cppgc/visitor.h index b70fde537b..99b3c3a5d6 100644 --- a/include/cppgc/visitor.h +++ b/include/cppgc/visitor.h @@ -19,6 +19,7 @@ namespace internal { template class BasicPersistent; +class ConservativeTracingVisitor; class VisitorBase; } // namespace internal @@ -43,6 +44,8 @@ using WeakCallback = void (*)(const LivenessBroker&, const void*); */ class Visitor { public: + virtual ~Visitor() = default; + /** * Trace method for Member. * @@ -194,6 +197,7 @@ class Visitor { template friend class internal::BasicPersistent; + friend class internal::ConservativeTracingVisitor; }; } // namespace cppgc diff --git a/src/heap/cppgc/marker.cc b/src/heap/cppgc/marker.cc index 47ba18261c..c7df664aef 100644 --- a/src/heap/cppgc/marker.cc +++ b/src/heap/cppgc/marker.cc @@ -106,7 +106,10 @@ Marker::Marker(HeapBase& heap) mutator_marking_state_(std::make_unique( heap, &marking_worklist_, ¬_fully_constructed_worklist_, &weak_callback_worklist_, Marker::kMutatorThreadId)), - marking_visitor_(CreateMutatorThreadMarkingVisitor()) {} + marking_visitor_(CreateMutatorThreadMarkingVisitor()), + conservative_marking_visitor_( + std::make_unique( + heap, *mutator_marking_state_.get(), *marking_visitor_.get())) {} Marker::~Marker() { // The fixed point iteration may have found not-fully-constructed objects. @@ -184,7 +187,7 @@ void Marker::VisitRoots() { heap().GetStrongPersistentRegion().Trace(marking_visitor_.get()); if (config_.stack_state != MarkingConfig::StackState::kNoHeapPointers) { - heap().stack()->IteratePointers(marking_visitor_.get()); + heap().stack()->IteratePointers(conservative_marking_visitor_.get()); } if (config_.collection_type == MarkingConfig::CollectionType::kMinor) { VisitRememberedSlots(heap(), marking_visitor_.get()); @@ -258,7 +261,7 @@ void Marker::MarkNotFullyConstructedObjects() { NotFullyConstructedWorklist::View view(¬_fully_constructed_worklist_, kMutatorThreadId); while (view.Pop(&item)) { - marking_visitor_->TraceConservativelyIfNeeded(item); + conservative_marking_visitor_->TraceConservativelyIfNeeded(item); } } diff --git a/src/heap/cppgc/marker.h b/src/heap/cppgc/marker.h index bde4d44792..4db08b6273 100644 --- a/src/heap/cppgc/marker.h +++ b/src/heap/cppgc/marker.h @@ -11,6 +11,7 @@ #include "include/cppgc/trace-trait.h" #include "include/cppgc/visitor.h" #include "src/base/platform/time.h" +#include "src/heap/cppgc/marking-visitor.h" #include "src/heap/cppgc/worklist.h" namespace cppgc { @@ -138,6 +139,7 @@ class V8_EXPORT_PRIVATE Marker { std::unique_ptr mutator_marking_state_; std::unique_ptr marking_visitor_; + std::unique_ptr conservative_marking_visitor_; MarkingWorklist marking_worklist_; NotFullyConstructedWorklist not_fully_constructed_worklist_; diff --git a/src/heap/cppgc/marking-visitor.cc b/src/heap/cppgc/marking-visitor.cc index fd5140a7df..162aa4722e 100644 --- a/src/heap/cppgc/marking-visitor.cc +++ b/src/heap/cppgc/marking-visitor.cc @@ -15,8 +15,7 @@ namespace cppgc { namespace internal { MarkingVisitor::MarkingVisitor(HeapBase& heap, MarkingState& marking_state) - : ConservativeTracingVisitor(heap, *heap.page_backend()), - marking_state_(marking_state) {} + : marking_state_(marking_state) {} void MarkingVisitor::Visit(const void* object, TraceDescriptor desc) { marking_state_.MarkAndPush(object, desc); @@ -40,20 +39,25 @@ void MarkingVisitor::VisitWeakRoot(const void* object, TraceDescriptor desc, weak_root); } -void MarkingVisitor::VisitPointer(const void* address) { - TraceConservativelyIfNeeded(address); +void MarkingVisitor::RegisterWeakCallback(WeakCallback callback, + const void* object) { + marking_state_.RegisterWeakCallback(callback, object); } -void MarkingVisitor::VisitConservatively(HeapObjectHeader& header, - TraceConservativelyCallback callback) { +ConservativeMarkingVisitor::ConservativeMarkingVisitor( + HeapBase& heap, MarkingState& marking_state, cppgc::Visitor& visitor) + : ConservativeTracingVisitor(heap, *heap.page_backend(), visitor), + marking_state_(marking_state) {} + +void ConservativeMarkingVisitor::VisitConservatively( + HeapObjectHeader& header, TraceConservativelyCallback callback) { marking_state_.MarkNoPush(header); callback(this, header); marking_state_.AccountMarkedBytes(header); } -void MarkingVisitor::RegisterWeakCallback(WeakCallback callback, - const void* object) { - marking_state_.RegisterWeakCallback(callback, object); +void ConservativeMarkingVisitor::VisitPointer(const void* address) { + TraceConservativelyIfNeeded(address); } MutatorThreadMarkingVisitor::MutatorThreadMarkingVisitor(Marker* marker) diff --git a/src/heap/cppgc/marking-visitor.h b/src/heap/cppgc/marking-visitor.h index 08968fadc5..3b40c8718a 100644 --- a/src/heap/cppgc/marking-visitor.h +++ b/src/heap/cppgc/marking-visitor.h @@ -18,14 +18,10 @@ class HeapObjectHeader; class Marker; class MarkingState; -class MarkingVisitor : public ConservativeTracingVisitor, - public heap::base::StackVisitor { +class MarkingVisitor : public VisitorBase { public: MarkingVisitor(HeapBase&, MarkingState&); - virtual ~MarkingVisitor() = default; - - MarkingVisitor(const MarkingVisitor&) = delete; - MarkingVisitor& operator=(const MarkingVisitor&) = delete; + ~MarkingVisitor() override = default; private: void Visit(const void*, TraceDescriptor) final; @@ -33,11 +29,20 @@ class MarkingVisitor : public ConservativeTracingVisitor, void VisitRoot(const void*, TraceDescriptor) final; void VisitWeakRoot(const void*, TraceDescriptor, WeakCallback, const void*) final; - void VisitConservatively(HeapObjectHeader&, - TraceConservativelyCallback) final; void RegisterWeakCallback(WeakCallback, const void*) final; - // StackMarker interface. + MarkingState& marking_state_; +}; + +class ConservativeMarkingVisitor : public ConservativeTracingVisitor, + public heap::base::StackVisitor { + public: + ConservativeMarkingVisitor(HeapBase&, MarkingState&, cppgc::Visitor&); + ~ConservativeMarkingVisitor() override = default; + + private: + void VisitConservatively(HeapObjectHeader&, + TraceConservativelyCallback) final; void VisitPointer(const void*) override; MarkingState& marking_state_; diff --git a/src/heap/cppgc/visitor.cc b/src/heap/cppgc/visitor.cc index 2b2d6b535a..3afdd5a804 100644 --- a/src/heap/cppgc/visitor.cc +++ b/src/heap/cppgc/visitor.cc @@ -22,12 +22,12 @@ void Visitor::CheckObjectNotInConstruction(const void* address) { namespace internal { ConservativeTracingVisitor::ConservativeTracingVisitor( - HeapBase& heap, PageBackend& page_backend) - : heap_(heap), page_backend_(page_backend) {} + HeapBase& heap, PageBackend& page_backend, cppgc::Visitor& visitor) + : heap_(heap), page_backend_(page_backend), visitor_(visitor) {} namespace { -void TraceConservatively(ConservativeTracingVisitor* visitor, +void TraceConservatively(ConservativeTracingVisitor* conservative_visitor, const HeapObjectHeader& header) { Address* payload = reinterpret_cast(header.Payload()); const size_t payload_size = header.GetSize(); @@ -40,7 +40,7 @@ void TraceConservatively(ConservativeTracingVisitor* visitor, MSAN_UNPOISON(&maybe_ptr, sizeof(maybe_ptr)); #endif if (maybe_ptr) { - visitor->TraceConservativelyIfNeeded(maybe_ptr); + conservative_visitor->TraceConservativelyIfNeeded(maybe_ptr); } } } @@ -64,9 +64,10 @@ void ConservativeTracingVisitor::TraceConservativelyIfNeeded( if (!header) return; if (!header->IsInConstruction()) { - Visit(header->Payload(), - {header->Payload(), - GlobalGCInfoTable::GCInfoFromIndex(header->GetGCInfoIndex()).trace}); + visitor_.Visit( + header->Payload(), + {header->Payload(), + GlobalGCInfoTable::GCInfoFromIndex(header->GetGCInfoIndex()).trace}); } else { VisitConservatively(*header, TraceConservatively); } diff --git a/src/heap/cppgc/visitor.h b/src/heap/cppgc/visitor.h index 5003e31f8f..35ae1f3725 100644 --- a/src/heap/cppgc/visitor.h +++ b/src/heap/cppgc/visitor.h @@ -21,6 +21,10 @@ class PageBackend; class VisitorBase : public cppgc::Visitor { public: VisitorBase() = default; + ~VisitorBase() override = default; + + VisitorBase(const VisitorBase&) = delete; + VisitorBase& operator=(const VisitorBase&) = delete; template void TraceRootForTesting(const Persistent& p, const SourceLocation& loc) { @@ -35,9 +39,14 @@ class VisitorBase : public cppgc::Visitor { }; // Regular visitor that additionally allows for conservative tracing. -class ConservativeTracingVisitor : public VisitorBase { +class ConservativeTracingVisitor { public: - ConservativeTracingVisitor(HeapBase&, PageBackend&); + ConservativeTracingVisitor(HeapBase&, PageBackend&, cppgc::Visitor&); + virtual ~ConservativeTracingVisitor() = default; + + ConservativeTracingVisitor(const ConservativeTracingVisitor&) = delete; + ConservativeTracingVisitor& operator=(const ConservativeTracingVisitor&) = + delete; void TraceConservativelyIfNeeded(const void*); @@ -49,6 +58,7 @@ class ConservativeTracingVisitor : public VisitorBase { HeapBase& heap_; PageBackend& page_backend_; + cppgc::Visitor& visitor_; }; } // namespace internal