Commit 75131637 authored by Omer Katz's avatar Omer Katz Committed by Commit Bot

cppgc: Handle non-gced traceable ephemeron values

On-heap hash maps in blink are limited to Member types and non-traceable
types. The only exception to that is TraceWrapperV8Reference. Thus
ephemerons can have non-gced traceable values. This values should not be
pushed to the marking worklist since we expect everything in the
worklist to be marked and not in construction (but these values don't
have an object header).
Instead, when getting a non-gced value we should immediately trace it.

This is only relevant to ephemerons. Any other case would go through
Trace(const T&) that dispatches to the TraceTrait.

Blink has 1 use case of HeahHashMap from WeakMember<ScriptWrappable> to
TraceWrapperV8Reference.

Bug: chromium:1056170
Change-Id: Ia8f341d6bb1fc8fd3655b2be66b7814896549d1e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2696648Reviewed-by: 's avatarMichael Lippautz <mlippautz@chromium.org>
Commit-Queue: Omer Katz <omerkatz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72763}
parent 983462da
...@@ -170,7 +170,7 @@ class V8_EXPORT Visitor { ...@@ -170,7 +170,7 @@ class V8_EXPORT Visitor {
if (!k) return; if (!k) return;
TraceDescriptor value_desc = TraceTrait<V>::GetTraceDescriptor(value); TraceDescriptor value_desc = TraceTrait<V>::GetTraceDescriptor(value);
if (!value_desc.base_object_payload) return; if (!value_desc.base_object_payload) return;
VisitEphemeron(key, value_desc); VisitEphemeron(key, value, value_desc);
} }
/** /**
...@@ -251,7 +251,8 @@ class V8_EXPORT Visitor { ...@@ -251,7 +251,8 @@ class V8_EXPORT Visitor {
virtual void VisitRoot(const void*, TraceDescriptor, const SourceLocation&) {} virtual void VisitRoot(const void*, TraceDescriptor, const SourceLocation&) {}
virtual void VisitWeakRoot(const void* self, TraceDescriptor, WeakCallback, virtual void VisitWeakRoot(const void* self, TraceDescriptor, WeakCallback,
const void* weak_root, const SourceLocation&) {} const void* weak_root, const SourceLocation&) {}
virtual void VisitEphemeron(const void* key, TraceDescriptor value_desc) {} virtual void VisitEphemeron(const void* key, const void* value,
TraceDescriptor value_desc) {}
virtual void VisitWeakContainer(const void* self, TraceDescriptor strong_desc, virtual void VisitWeakContainer(const void* self, TraceDescriptor strong_desc,
TraceDescriptor weak_desc, TraceDescriptor weak_desc,
WeakCallback callback, const void* data) {} WeakCallback callback, const void* data) {}
......
...@@ -34,8 +34,9 @@ void UnifiedHeapMarkingVisitorBase::VisitWeak(const void* object, ...@@ -34,8 +34,9 @@ void UnifiedHeapMarkingVisitorBase::VisitWeak(const void* object,
} }
void UnifiedHeapMarkingVisitorBase::VisitEphemeron(const void* key, void UnifiedHeapMarkingVisitorBase::VisitEphemeron(const void* key,
const void* value,
TraceDescriptor value_desc) { TraceDescriptor value_desc) {
marking_state_.ProcessEphemeron(key, value_desc); marking_state_.ProcessEphemeron(key, value, value_desc, *this);
} }
void UnifiedHeapMarkingVisitorBase::VisitWeakContainer( void UnifiedHeapMarkingVisitorBase::VisitWeakContainer(
......
...@@ -41,7 +41,7 @@ class V8_EXPORT_PRIVATE UnifiedHeapMarkingVisitorBase : public JSVisitor { ...@@ -41,7 +41,7 @@ class V8_EXPORT_PRIVATE UnifiedHeapMarkingVisitorBase : public JSVisitor {
// C++ handling. // C++ handling.
void Visit(const void*, TraceDescriptor) final; void Visit(const void*, TraceDescriptor) final;
void VisitWeak(const void*, TraceDescriptor, WeakCallback, const void*) final; void VisitWeak(const void*, TraceDescriptor, WeakCallback, const void*) final;
void VisitEphemeron(const void*, TraceDescriptor) final; void VisitEphemeron(const void*, const void*, TraceDescriptor) final;
void VisitWeakContainer(const void* self, TraceDescriptor strong_desc, void VisitWeakContainer(const void* self, TraceDescriptor strong_desc,
TraceDescriptor weak_desc, WeakCallback callback, TraceDescriptor weak_desc, WeakCallback callback,
const void* data) final; const void* data) final;
......
...@@ -158,10 +158,11 @@ void ConcurrentMarkingTask::ProcessWorklists( ...@@ -158,10 +158,11 @@ void ConcurrentMarkingTask::ProcessWorklists(
concurrent_marker_.incremental_marking_schedule(), concurrent_marker_.incremental_marking_schedule(),
concurrent_marking_state concurrent_marking_state
.ephemeron_pairs_for_processing_worklist(), .ephemeron_pairs_for_processing_worklist(),
[&concurrent_marking_state]( [&concurrent_marking_state, &concurrent_marking_visitor](
const MarkingWorklists::EphemeronPairItem& item) { const MarkingWorklists::EphemeronPairItem& item) {
concurrent_marking_state.ProcessEphemeron(item.key, concurrent_marking_state.ProcessEphemeron(
item.value_desc); item.key, item.value, item.value_desc,
concurrent_marking_visitor);
})) { })) {
return; return;
} }
......
...@@ -493,8 +493,8 @@ bool MarkerBase::ProcessWorklistsWithDeadline( ...@@ -493,8 +493,8 @@ bool MarkerBase::ProcessWorklistsWithDeadline(
mutator_marking_state_, marked_bytes_deadline, time_deadline, mutator_marking_state_, marked_bytes_deadline, time_deadline,
mutator_marking_state_.ephemeron_pairs_for_processing_worklist(), mutator_marking_state_.ephemeron_pairs_for_processing_worklist(),
[this](const MarkingWorklists::EphemeronPairItem& item) { [this](const MarkingWorklists::EphemeronPairItem& item) {
mutator_marking_state_.ProcessEphemeron(item.key, mutator_marking_state_.ProcessEphemeron(
item.value_desc); item.key, item.value, item.value_desc, visitor());
})) { })) {
return false; return false;
} }
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include <algorithm> #include <algorithm>
#include "include/cppgc/trace-trait.h" #include "include/cppgc/trace-trait.h"
#include "include/cppgc/visitor.h"
#include "src/heap/cppgc/compaction-worklists.h" #include "src/heap/cppgc/compaction-worklists.h"
#include "src/heap/cppgc/globals.h" #include "src/heap/cppgc/globals.h"
#include "src/heap/cppgc/heap-object-header.h" #include "src/heap/cppgc/heap-object-header.h"
...@@ -48,7 +49,8 @@ class MarkingStateBase { ...@@ -48,7 +49,8 @@ class MarkingStateBase {
inline void ProcessWeakContainer(const void*, TraceDescriptor, WeakCallback, inline void ProcessWeakContainer(const void*, TraceDescriptor, WeakCallback,
const void*); const void*);
inline void ProcessEphemeron(const void*, TraceDescriptor); inline void ProcessEphemeron(const void*, const void*, TraceDescriptor,
Visitor&);
inline void AccountMarkedBytes(const HeapObjectHeader&); inline void AccountMarkedBytes(const HeapObjectHeader&);
inline void AccountMarkedBytes(size_t); inline void AccountMarkedBytes(size_t);
...@@ -265,16 +267,23 @@ void MarkingStateBase::ProcessWeakContainer(const void* object, ...@@ -265,16 +267,23 @@ void MarkingStateBase::ProcessWeakContainer(const void* object,
if (desc.callback) PushMarked(header, desc); if (desc.callback) PushMarked(header, desc);
} }
void MarkingStateBase::ProcessEphemeron(const void* key, void MarkingStateBase::ProcessEphemeron(const void* key, const void* value,
TraceDescriptor value_desc) { TraceDescriptor value_desc,
Visitor& visitor) {
// Filter out already marked keys. The write barrier for WeakMember // Filter out already marked keys. The write barrier for WeakMember
// ensures that any newly set value after this point is kept alive and does // ensures that any newly set value after this point is kept alive and does
// not require the callback. // not require the callback.
if (HeapObjectHeader::FromPayload(key).IsMarked<AccessMode::kAtomic>()) { if (HeapObjectHeader::FromPayload(key).IsMarked<AccessMode::kAtomic>()) {
if (value_desc.base_object_payload) {
MarkAndPush(value_desc.base_object_payload, value_desc); MarkAndPush(value_desc.base_object_payload, value_desc);
} else {
// If value_desc.base_object_payload is nullptr, the value is not GCed and
// should be immediately traced.
value_desc.callback(&visitor, value);
}
return; return;
} }
discovered_ephemeron_pairs_worklist_.Push({key, value_desc}); discovered_ephemeron_pairs_worklist_.Push({key, value, value_desc});
} }
void MarkingStateBase::AccountMarkedBytes(const HeapObjectHeader& header) { void MarkingStateBase::AccountMarkedBytes(const HeapObjectHeader& header) {
......
...@@ -25,9 +25,9 @@ void MarkingVisitorBase::VisitWeak(const void* object, TraceDescriptor desc, ...@@ -25,9 +25,9 @@ void MarkingVisitorBase::VisitWeak(const void* object, TraceDescriptor desc,
weak_member); weak_member);
} }
void MarkingVisitorBase::VisitEphemeron(const void* key, void MarkingVisitorBase::VisitEphemeron(const void* key, const void* value,
TraceDescriptor value_desc) { TraceDescriptor value_desc) {
marking_state_.ProcessEphemeron(key, value_desc); marking_state_.ProcessEphemeron(key, value, value_desc, *this);
} }
void MarkingVisitorBase::VisitWeakContainer(const void* object, void MarkingVisitorBase::VisitWeakContainer(const void* object,
......
...@@ -28,7 +28,7 @@ class V8_EXPORT_PRIVATE MarkingVisitorBase : public VisitorBase { ...@@ -28,7 +28,7 @@ class V8_EXPORT_PRIVATE MarkingVisitorBase : public VisitorBase {
protected: protected:
void Visit(const void*, TraceDescriptor) final; void Visit(const void*, TraceDescriptor) final;
void VisitWeak(const void*, TraceDescriptor, WeakCallback, const void*) final; void VisitWeak(const void*, TraceDescriptor, WeakCallback, const void*) final;
void VisitEphemeron(const void*, TraceDescriptor) final; void VisitEphemeron(const void*, const void*, TraceDescriptor) final;
void VisitWeakContainer(const void* self, TraceDescriptor strong_desc, void VisitWeakContainer(const void* self, TraceDescriptor strong_desc,
TraceDescriptor weak_desc, WeakCallback callback, TraceDescriptor weak_desc, WeakCallback callback,
const void* data) final; const void* data) final;
......
...@@ -63,6 +63,7 @@ class MarkingWorklists { ...@@ -63,6 +63,7 @@ class MarkingWorklists {
struct EphemeronPairItem { struct EphemeronPairItem {
const void* key; const void* key;
const void* value;
TraceDescriptor value_desc; TraceDescriptor value_desc;
}; };
......
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