Commit 822e1bc9 authored by Michael Lippautz's avatar Michael Lippautz Committed by Commit Bot

cppgc: Refactor visitation 1/3

Split off MarkingState from MarkingVisitor.

With this CL the marking implementation is moved to "MarkingState"
which is the new bottleneck for marking a single object.
MarkingVisitor merely forwards to MarkingState, which knows how to set
the markbit and add the object to the worklist accordingly. This
allows to have a "UnifiedHeapMarkingVisitor" in future which can
easily reuse Marking to provide C++ marking.

Change-Id: I87ebbe37e8e8cd841e872cae9dc3490e2b55c4dd
Bug: chromium:1056170
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2270172Reviewed-by: 's avatarOmer Katz <omerkatz@chromium.org>
Reviewed-by: 's avatarAnton Bikineev <bikineev@chromium.org>
Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68660}
parent 7c6ff8b1
......@@ -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",
......
......@@ -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
......
......@@ -4,12 +4,15 @@
#include "src/heap/cppgc/marker.h"
#include <memory>
#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<MarkingState>(
heap, &marking_worklist_, &not_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<ConstAddress>(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;
......
......@@ -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<MarkingState> mutator_marking_state_;
std::unique_ptr<MutatorThreadMarkingVisitor> marking_visitor_;
MarkingWorklist marking_worklist_;
......
// 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<void*>(desc.base_object_payload)),
desc);
}
void MarkingState::MarkAndPush(HeapObjectHeader& header, TraceDescriptor desc) {
DCHECK_NOT_NULL(desc.callback);
if (header.IsInConstruction<HeapObjectHeader::AccessMode::kNonAtomic>()) {
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>(address));
DCHECK(!header.IsInConstruction<HeapObjectHeader::AccessMode::kNonAtomic>());
if (MarkNoPush(header)) {
marking_worklist_.Push(
{reinterpret_cast<void*>(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<HeapObjectHeader::AccessMode::kAtomic>())
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<const LargePage*>(BasePage::FromPayload(&header))
->PayloadSize()
: header.GetSize();
}
} // namespace internal
} // namespace cppgc
#endif // V8_HEAP_CPPGC_MARKING_STATE_H_
......@@ -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<HeapObjectHeader::AccessMode::kNonAtomic>();
}
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<const LargePage*>(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<void*>(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<HeapObjectHeader::AccessMode::kAtomic>())
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>(address));
DCHECK(!IsInConstruction(header));
if (MarkHeaderNoTracing(&header)) {
marking_worklist_.Push(
{reinterpret_cast<void*>(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
......@@ -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 {
......
......@@ -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<HeapObjectHeader::AccessMode::kNonAtomic>())) {
// It is assumed that objects on not_fully_constructed_worklist_ are not
// marked.
header.Unmark();
......
......@@ -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<GCedWithMixin>, 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<GCed> object(MakeGarbageCollected<GCed>(GetAllocationHandle()));
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment