cppgc-js,heap: Implement snapshots for embedder fields

https://crrev.com/c/3293410 added concurrent processing of C++ objects
found through V8 embedder fields. The CL missed that those embedder
fields are not read atomically from JS objects. The problem is that
embedder fields are only aligned to kTaggedSize on builds with pointer
compression and are as such mis-aligned for atomic ops. This is not a
problem for on-heap values as the upper 32bits are anyways computed
from the cage. Is is a problem for generic C++ values though, as they
are used with Oilpan.

This CL adds the standard marker snapshot protocol for embedder fields.

Marker:
1. Snapshot embedder fields
2. Try to mark host object
3. On success: process snapshot

Main thread:
1. On setting embedder fields mark the object black first
2. Emit a write barrier for the embedder fields

This will get simpler with the heap sandbox that uses a separate table
for embedder fields. Once the sandbox is the default configuration, we
	can use it as dependency for the concurrent fast path.

Bug: chromium:1285706
Change-Id: I6b975ea561be08cda840ef0dd27a11627de93900
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3380983Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78604}

BUILD.bazel

@@ -1325,6 +1325,8 @@ filegroup(
         "src/heap/cppgc-js/unified-heap-marking-verifier.h",
         "src/heap/cppgc-js/unified-heap-marking-visitor.cc",
         "src/heap/cppgc-js/unified-heap-marking-visitor.h",
+        "src/heap/embedder-data-snapshot.h",
+        "src/heap/embedder-data-snapshot-inl.h",
         "src/heap/embedder-tracing.cc",
         "src/heap/embedder-tracing.h",
         "src/heap/embedder-tracing-inl.h",

BUILD.gn

@@ -2954,6 +2954,8 @@ v8_header_set("v8_internal_headers") {
     "src/heap/cppgc-js/unified-heap-marking-state.h",
     "src/heap/cppgc-js/unified-heap-marking-verifier.h",
     "src/heap/cppgc-js/unified-heap-marking-visitor.h",
+    "src/heap/embedder-data-snapshot-inl.h",
+    "src/heap/embedder-data-snapshot.h",
     "src/heap/embedder-tracing-inl.h",
     "src/heap/embedder-tracing.h",
     "src/heap/factory-base-inl.h",

src/api/api.cc

@@ -6032,18 +6032,36 @@ void v8::Object::SetAlignedPointerInInternalField(int index, void* value) {
   i::Handle<i::JSReceiver> obj = Utils::OpenHandle(this);
   const char* location = "v8::Object::SetAlignedPointerInInternalField()";
   if (!InternalFieldOK(obj, index, location)) return;
+
+  i::DisallowGarbageCollection no_gc;
+
+  // There's no need to invalidate slots as embedder fields are always
+  // tagged.
+  obj->GetHeap()->NotifyObjectLayoutChange(*obj, no_gc,
+                                           i::InvalidateRecordedSlots::kNo);
+
   Utils::ApiCheck(i::EmbedderDataSlot(i::JSObject::cast(*obj), index)
                       .store_aligned_pointer(obj->GetIsolate(), value),
                   location, "Unaligned pointer");
   DCHECK_EQ(value, GetAlignedPointerFromInternalField(index));
   internal::WriteBarrier::MarkingFromInternalFields(i::JSObject::cast(*obj));
+
+#ifdef VERIFY_HEAP
+  obj->GetHeap()->VerifyObjectLayoutChange(*obj, obj->map());
+#endif  // VERIFY_HEAP
 }
 
 void v8::Object::SetAlignedPointerInInternalFields(int argc, int indices[],
                                                    void* values[]) {
   i::Handle<i::JSReceiver> obj = Utils::OpenHandle(this);
-  const char* location = "v8::Object::SetAlignedPointerInInternalFields()";
+
   i::DisallowGarbageCollection no_gc;
+  // There's no need to invalidate slots as embedder fields are always
+  // tagged.
+  obj->GetHeap()->NotifyObjectLayoutChange(*obj, no_gc,
+                                           i::InvalidateRecordedSlots::kNo);
+
+  const char* location = "v8::Object::SetAlignedPointerInInternalFields()";
   i::JSObject js_obj = i::JSObject::cast(*obj);
   int nof_embedder_fields = js_obj.GetEmbedderFieldCount();
   for (int i = 0; i < argc; i++) {
@@ -6059,6 +6077,10 @@ void v8::Object::SetAlignedPointerInInternalFields(int argc, int indices[],
     DCHECK_EQ(value, GetAlignedPointerFromInternalField(index));
   }
   internal::WriteBarrier::MarkingFromInternalFields(js_obj);
+
+#ifdef VERIFY_HEAP
+  obj->GetHeap()->VerifyObjectLayoutChange(*obj, obj->map());
+#endif  // VERIFY_HEAP
 }
 
 static void* ExternalValue(i::Object obj) {

src/heap/concurrent-marking.cc

@@ -10,6 +10,7 @@
 #include "include/v8config.h"
 #include "src/common/globals.h"
 #include "src/execution/isolate.h"
+#include "src/heap/embedder-data-snapshot.h"
 #include "src/heap/gc-tracer.h"
 #include "src/heap/heap-inl.h"
 #include "src/heap/heap.h"
@@ -86,15 +87,16 @@ class ConcurrentMarkingVisitor final
  public:
   ConcurrentMarkingVisitor(int task_id,
                            MarkingWorklists::Local* local_marking_worklists,
-                           WeakObjects::Local* local_weak_objects, Heap* heap,
-                           unsigned mark_compact_epoch,
+                           WeakObjects::Local* local_weak_objects,
+                           EmbedderDataSnapshot* embedder_data_snapshot,
+                           Heap* heap, unsigned mark_compact_epoch,
                            base::EnumSet<CodeFlushMode> code_flush_mode,
                            bool embedder_tracing_enabled,
                            bool should_keep_ages_unchanged,
                            MemoryChunkDataMap* memory_chunk_data)
-      : MarkingVisitorBase(local_marking_worklists, local_weak_objects, heap,
-                           mark_compact_epoch, code_flush_mode,
-                           embedder_tracing_enabled,
+      : MarkingVisitorBase(local_marking_worklists, local_weak_objects,
+                           embedder_data_snapshot, heap, mark_compact_epoch,
+                           code_flush_mode, embedder_tracing_enabled,
                            should_keep_ages_unchanged),
         marking_state_(heap->isolate(), memory_chunk_data),
         memory_chunk_data_(memory_chunk_data) {}
@@ -457,11 +459,16 @@ void ConcurrentMarking::Run(JobDelegate* delegate,
                               ? cpp_heap->CreateCppMarkingState()
                               : MarkingWorklists::Local::kNoCppMarkingState);
   WeakObjects::Local local_weak_objects(weak_objects_);
+  EmbedderDataSnapshot embedder_data_snapshot(
+      cpp_heap ? cpp_heap->wrapper_descriptor()
+               // Without CppHeap the snapshot will not be used and any
+               // descriptor will do.
+               : v8::WrapperDescriptor(0, 0, 0));
   ConcurrentMarkingVisitor visitor(
-      task_id, &local_marking_worklists, &local_weak_objects, heap_,
-      mark_compact_epoch, code_flush_mode,
-      heap_->local_embedder_heap_tracer()->InUse(), should_keep_ages_unchanged,
-      &task_state->memory_chunk_data);
+      task_id, &local_marking_worklists, &local_weak_objects,
+      cpp_heap ? &embedder_data_snapshot : nullptr, heap_, mark_compact_epoch,
+      code_flush_mode, heap_->local_embedder_heap_tracer()->InUse(),
+      should_keep_ages_unchanged, &task_state->memory_chunk_data);
   NativeContextInferrer& native_context_inferrer =
       task_state->native_context_inferrer;
   NativeContextStats& native_context_stats = task_state->native_context_stats;

src/heap/cppgc-js/cpp-marking-state-inl.h

@@ -7,16 +7,16 @@
 
 #include "src/heap/cppgc-js/cpp-marking-state.h"
 #include "src/heap/embedder-tracing-inl.h"
-#include "src/objects/js-objects.h"
+#include "src/objects/embedder-data-slot.h"
 
 namespace v8 {
 namespace internal {
 
-void CppMarkingState::MarkAndPush(const JSObject& js_object) {
-  DCHECK(js_object.IsApiWrapper());
+void CppMarkingState::MarkAndPush(const EmbedderDataSlot& type_slot,
+                                  const EmbedderDataSlot& instance_slot) {
   LocalEmbedderHeapTracer::WrapperInfo info;
   if (LocalEmbedderHeapTracer::ExtractWrappableInfo(
-          isolate_, js_object, wrapper_descriptor_, &info)) {
+          isolate_, wrapper_descriptor_, type_slot, instance_slot, &info)) {
     marking_state_.MarkAndPush(
         cppgc::internal::HeapObjectHeader::FromObject(info.second));
   }

src/heap/cppgc-js/cpp-marking-state.h

@@ -15,6 +15,7 @@ namespace v8 {
 namespace internal {
 
 class JSObject;
+class EmbedderDataSlot;
 
 class CppMarkingState {
  public:
@@ -37,7 +38,8 @@ class CppMarkingState {
 
   void Publish() { marking_state_.Publish(); }
 
-  inline void MarkAndPush(const JSObject& js_object);
+  inline void MarkAndPush(const EmbedderDataSlot& type_slot,
+                          const EmbedderDataSlot& instance_slot);
 
   bool IsLocalEmpty() {
     return marking_state_.marking_worklist().IsLocalEmpty();

src/heap/embedder-data-snapshot-inl.h

+// Copyright 2022 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_EMBEDDER_DATA_SNAPSHOT_INL_H_
+#define V8_HEAP_EMBEDDER_DATA_SNAPSHOT_INL_H_
+
+#include "src/common/globals.h"
+#include "src/heap/embedder-data-snapshot.h"
+#include "src/objects/embedder-data-slot-inl.h"
+#include "src/objects/js-objects-inl.h"
+
+// Has to be the last include (doesn't have include guards):
+#include "src/objects/object-macros.h"
+
+namespace v8 {
+namespace internal {
+
+bool EmbedderDataSnapshot::Populate(Map map, JSObject js_object) {
+  if (JSObject::GetEmbedderFieldCount(map) < 2) {
+#ifdef DEBUG
+    has_valid_snapshot_ = false;
+#endif  // DEBUG
+    return false;
+  }
+
+  // Tracing only requires the first two embedder fields. Avoid taking a
+  // snapshot of the other data.
+  Address start_address =
+      FIELD_ADDR(js_object, JSObject::GetEmbedderFieldsStartOffset(map));
+  DCHECK_LE(last_index_, kMaxNumTaggedEmbedderSlots);
+  int end_offset = (last_index_ + 1) * kEmbedderDataSlotSize;
+  DCHECK_EQ(0, start_address % kTaggedSize);
+  DCHECK_EQ(0, end_offset % kTaggedSize);
+  for (int i = 0; i < end_offset / kTaggedSize; i++) {
+    snapshot_[i] = AsAtomicTagged::Relaxed_Load(
+        reinterpret_cast<AtomicTagged_t*>(start_address) + i);
+  }
+
+#ifdef DEBUG
+  has_valid_snapshot_ = true;
+#endif  // DEBUG
+  return true;
+}
+
+V8_INLINE std::pair<EmbedderDataSlot, EmbedderDataSlot>
+EmbedderDataSnapshot::ExtractWrapperSlots() const {
+  DCHECK(has_valid_snapshot_);
+
+  static constexpr size_t kTaggedSlotsPerEmbedderSlot =
+      kEmbedderDataSlotSize / kTaggedSize;
+
+  return std::make_pair(
+      EmbedderDataSlot(reinterpret_cast<Address>(
+          &snapshot_[kTaggedSlotsPerEmbedderSlot *
+                     wrapper_descriptor_.wrappable_type_index])),
+      EmbedderDataSlot(reinterpret_cast<Address>(
+          &snapshot_[kTaggedSlotsPerEmbedderSlot *
+                     wrapper_descriptor_.wrappable_instance_index])));
+}
+
+}  // namespace internal
+}  // namespace v8
+
+#include "src/objects/object-macros-undef.h"
+
+#endif  // V8_HEAP_EMBEDDER_DATA_SNAPSHOT_INL_H_

src/heap/embedder-data-snapshot.h

+// Copyright 2022 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_EMBEDDER_DATA_SNAPSHOT_H_
+#define V8_HEAP_EMBEDDER_DATA_SNAPSHOT_H_
+
+#include "include/v8-cppgc.h"
+#include "src/common/globals.h"
+#include "src/heap/embedder-tracing.h"
+#include "src/objects/js-objects.h"
+#include "src/objects/map.h"
+
+namespace v8 {
+namespace internal {
+
+// Snapshot for embedder data that is used for concurrently processing embedder
+// fields.
+//
+// The snapshot is used together with a notification for object layout change
+// which locks out the concurrent marker from processing embedder fields. This
+// is necessary as embedder fields are only aligned for tagged values which
+// violates atomicity of a single pointer read and prevents us from using atomic
+// operations.
+class EmbedderDataSnapshot final {
+ public:
+  explicit EmbedderDataSnapshot(const WrapperDescriptor wrapper_descriptor)
+      : wrapper_descriptor_(wrapper_descriptor),
+        last_index_(std::max(wrapper_descriptor_.wrappable_type_index,
+                             wrapper_descriptor_.wrappable_instance_index)) {}
+
+  V8_INLINE bool Populate(Map map, JSObject js_object);
+
+  V8_INLINE std::pair<EmbedderDataSlot, EmbedderDataSlot> ExtractWrapperSlots()
+      const;
+
+ private:
+  static constexpr size_t kTypeIndex = 0;
+  static constexpr size_t kInstanceIndex = 1;
+  static constexpr size_t kMaxNumTaggedEmbedderSlots =
+      JSObject::kMaxEmbedderFields * kEmbedderDataSlotSize / kTaggedSize;
+
+  static_assert(
+      kMaxNumTaggedEmbedderSlots < 32,
+      "EmbedderDataSnapshot is allocated on stack and should stay small.");
+
+  const WrapperDescriptor wrapper_descriptor_;
+  Tagged_t snapshot_[kMaxNumTaggedEmbedderSlots];
+  int last_index_;
+#ifdef DEBUG
+  bool has_valid_snapshot_{false};
+#endif  // DEBUG
+};
+
+}  // namespace internal
+}  // namespace v8
+
+#endif  // V8_HEAP_EMBEDDER_DATA_SNAPSHOT_H_

src/heap/embedder-tracing-inl.h

@@ -18,11 +18,20 @@ bool LocalEmbedderHeapTracer::ExtractWrappableInfo(
   DCHECK(js_object.IsApiWrapper());
   if (js_object.GetEmbedderFieldCount() < 2) return false;
 
-  if (EmbedderDataSlot(js_object, wrapper_descriptor.wrappable_type_index)
-          .ToAlignedPointerSafe(isolate, &info->first) &&
-      info->first &&
-      EmbedderDataSlot(js_object, wrapper_descriptor.wrappable_instance_index)
-          .ToAlignedPointerSafe(isolate, &info->second) &&
+  return ExtractWrappableInfo(
+      isolate, wrapper_descriptor,
+      EmbedderDataSlot(js_object, wrapper_descriptor.wrappable_type_index),
+      EmbedderDataSlot(js_object, wrapper_descriptor.wrappable_instance_index),
+      info);
+}
+
+// static
+bool LocalEmbedderHeapTracer::ExtractWrappableInfo(
+    Isolate* isolate, const WrapperDescriptor& wrapper_descriptor,
+    const EmbedderDataSlot& type_slot, const EmbedderDataSlot& instance_slot,
+    WrapperInfo* info) {
+  if (type_slot.ToAlignedPointerSafe(isolate, &info->first) && info->first &&
+      instance_slot.ToAlignedPointerSafe(isolate, &info->second) &&
       info->second) {
     return (wrapper_descriptor.embedder_id_for_garbage_collected ==
             WrapperDescriptor::kUnknownEmbedderId) ||

src/heap/embedder-tracing.cc

@@ -197,8 +197,13 @@ void LocalEmbedderHeapTracer::EmbedderWriteBarrier(Heap* heap,
   DCHECK(js_object.IsApiWrapper());
   if (cpp_heap_) {
     DCHECK_NOT_NULL(heap->mark_compact_collector());
-    heap->mark_compact_collector()->local_marking_worklists()->PushWrapper(
-        js_object);
+    const EmbedderDataSlot type_slot(js_object,
+                                     wrapper_descriptor_.wrappable_type_index);
+    const EmbedderDataSlot instance_slot(
+        js_object, wrapper_descriptor_.wrappable_instance_index);
+    heap->mark_compact_collector()
+        ->local_marking_worklists()
+        ->PushExtractedWrapper(type_slot, instance_slot);
     return;
   }
   LocalEmbedderHeapTracer::ProcessingScope scope(this);

src/heap/embedder-tracing.h

@@ -77,6 +77,9 @@ class V8_EXPORT_PRIVATE LocalEmbedderHeapTracer final {
   static V8_INLINE bool ExtractWrappableInfo(Isolate*, JSObject,
                                              const WrapperDescriptor&,
                                              WrapperInfo*);
+  static V8_INLINE bool ExtractWrappableInfo(
+      Isolate*, const WrapperDescriptor&, const EmbedderDataSlot& type_slot,
+      const EmbedderDataSlot& instance_slot, WrapperInfo*);
 
   explicit LocalEmbedderHeapTracer(Isolate* isolate) : isolate_(isolate) {}
 

src/heap/heap-write-barrier.cc

@@ -49,11 +49,9 @@ void WriteBarrier::MarkingSlowFromGlobalHandle(Heap* heap, HeapObject value) {
 
 // static
 void WriteBarrier::MarkingSlowFromInternalFields(Heap* heap, JSObject host) {
-  // We are not checking the mark bits of host here as (a) there's no
-  // synchronization with the marker and (b) we are writing into a live object
-  // (independent of the mark bits).
   auto* local_embedder_heap_tracer = heap->local_embedder_heap_tracer();
   if (!local_embedder_heap_tracer->InUse()) return;
+
   local_embedder_heap_tracer->EmbedderWriteBarrier(heap, host);
 }
 

src/heap/mark-compact.cc

@@ -22,6 +22,8 @@
 #include "src/heap/array-buffer-sweeper.h"
 #include "src/heap/basic-memory-chunk.h"
 #include "src/heap/code-object-registry.h"
+#include "src/heap/embedder-data-snapshot-inl.h"
+#include "src/heap/embedder-data-snapshot.h"
 #include "src/heap/gc-tracer.h"
 #include "src/heap/heap.h"
 #include "src/heap/incremental-marking-inl.h"
@@ -578,9 +580,12 @@ void MarkCompactCollector::StartMarking() {
       cpp_heap ? cpp_heap->CreateCppMarkingStateForMutatorThread()
                : MarkingWorklists::Local::kNoCppMarkingState);
   local_weak_objects_ = std::make_unique<WeakObjects::Local>(weak_objects());
+  embedder_data_snapshot_ = cpp_heap ? std::make_unique<EmbedderDataSnapshot>(
+                                           cpp_heap->wrapper_descriptor())
+                                     : nullptr;
   marking_visitor_ = std::make_unique<MarkingVisitor>(
       marking_state(), local_marking_worklists(), local_weak_objects_.get(),
-      heap_, epoch(), code_flush_mode(),
+      embedder_data_snapshot_.get(), heap_, epoch(), code_flush_mode(),
       heap_->local_embedder_heap_tracer()->InUse(),
       heap_->ShouldCurrentGCKeepAgesUnchanged());
 // Marking bits are cleared by the sweeper.
@@ -998,6 +1003,7 @@ void MarkCompactCollector::Finish() {
 
   marking_visitor_.reset();
   local_marking_worklists_.reset();
+  embedder_data_snapshot_.reset();
   marking_worklists_.ReleaseContextWorklists();
   native_context_stats_.Clear();
 

src/heap/mark-compact.h

@@ -11,6 +11,7 @@
 #include "include/v8-internal.h"
 #include "src/heap/base/worklist.h"
 #include "src/heap/concurrent-marking.h"
+#include "src/heap/embedder-data-snapshot.h"
 #include "src/heap/marking-visitor.h"
 #include "src/heap/marking-worklist.h"
 #include "src/heap/marking.h"
@@ -23,6 +24,7 @@ namespace v8 {
 namespace internal {
 
 // Forward declarations.
+class EmbedderDataSnapshot;
 class EvacuationJobTraits;
 class HeapObjectVisitor;
 class ItemParallelJob;
@@ -390,14 +392,15 @@ class MainMarkingVisitor final
 
   MainMarkingVisitor(MarkingState* marking_state,
                      MarkingWorklists::Local* local_marking_worklists,
-                     WeakObjects::Local* local_weak_objects, Heap* heap,
+                     WeakObjects::Local* local_weak_objects,
+                     EmbedderDataSnapshot* embedder_data_snapshot, Heap* heap,
                      unsigned mark_compact_epoch,
                      base::EnumSet<CodeFlushMode> code_flush_mode,
                      bool embedder_tracing_enabled,
                      bool should_keep_ages_unchanged)
       : MarkingVisitorBase<MainMarkingVisitor<MarkingState>, MarkingState>(
-            local_marking_worklists, local_weak_objects, heap,
-            mark_compact_epoch, code_flush_mode, embedder_tracing_enabled,
+            local_marking_worklists, local_weak_objects, embedder_data_snapshot,
+            heap, mark_compact_epoch, code_flush_mode, embedder_tracing_enabled,
             should_keep_ages_unchanged),
         marking_state_(marking_state),
         revisiting_object_(false) {}
@@ -788,6 +791,7 @@ class MarkCompactCollector final : public MarkCompactCollectorBase {
   std::unique_ptr<MarkingVisitor> marking_visitor_;
   std::unique_ptr<MarkingWorklists::Local> local_marking_worklists_;
   std::unique_ptr<WeakObjects::Local> local_weak_objects_;
+  std::unique_ptr<EmbedderDataSnapshot> embedder_data_snapshot_;
   NativeContextInferrer native_context_inferrer_;
   NativeContextStats native_context_stats_;
 

src/heap/marking-visitor-inl.h

@@ -5,6 +5,7 @@
 #ifndef V8_HEAP_MARKING_VISITOR_INL_H_
 #define V8_HEAP_MARKING_VISITOR_INL_H_
 
+#include "src/heap/embedder-data-snapshot-inl.h"
 #include "src/heap/marking-visitor.h"
 #include "src/heap/objects-visiting-inl.h"
 #include "src/heap/objects-visiting.h"
@@ -264,11 +265,20 @@ int MarkingVisitorBase<ConcreteVisitor,
                        MarkingState>::VisitEmbedderTracingSubclass(Map map,
                                                                    T object) {
   DCHECK(object.IsApiWrapper());
+  const bool valid_snapshot_data =
+      is_embedder_tracing_enabled_ && embedder_data_snapshot_ &&
+      embedder_data_snapshot_->Populate(map, object);
   int size = concrete_visitor()->VisitJSObjectSubclass(map, object);
   if (size && is_embedder_tracing_enabled_) {
-    // Success: The object needs to be processed for embedder references on
-    // the main thread.
-    local_marking_worklists_->PushWrapper(object);
+    // Success: The object needs to be processed for embedder references.
+    if (valid_snapshot_data) {
+      // Push extracted snapshot.
+      const auto pair = embedder_data_snapshot_->ExtractWrapperSlots();
+      local_marking_worklists_->PushExtractedWrapper(pair.first, pair.second);
+    } else if (!embedder_data_snapshot_) {
+      // Fallback for main thread processing.
+      local_marking_worklists_->PushWrapper(object);
+    }
   }
   return size;
 }

src/heap/marking-visitor.h

@@ -6,6 +6,7 @@
 #define V8_HEAP_MARKING_VISITOR_H_
 
 #include "src/common/globals.h"
+#include "src/heap/embedder-tracing.h"
 #include "src/heap/marking-worklist.h"
 #include "src/heap/marking.h"
 #include "src/heap/memory-chunk.h"
@@ -16,6 +17,8 @@
 namespace v8 {
 namespace internal {
 
+class EmbedderDataSnapshot;
+
 struct EphemeronMarking {
   std::vector<HeapObject> newly_discovered;
   bool newly_discovered_overflowed;
@@ -128,14 +131,15 @@ class MarkingVisitorBase : public HeapVisitor<int, ConcreteVisitor> {
  public:
   MarkingVisitorBase(MarkingWorklists::Local* local_marking_worklists,
                      WeakObjects::Local* local_weak_objects,
-                     //  WeakObjects* weak_objects,
-                     Heap* heap, unsigned mark_compact_epoch,
+                     EmbedderDataSnapshot* embedder_data_snapshot, Heap* heap,
+                     unsigned mark_compact_epoch,
                      base::EnumSet<CodeFlushMode> code_flush_mode,
                      bool is_embedder_tracing_enabled,
                      bool should_keep_ages_unchanged)
       : HeapVisitor<int, ConcreteVisitor>(heap),
         local_marking_worklists_(local_marking_worklists),
         local_weak_objects_(local_weak_objects),
+        embedder_data_snapshot_(embedder_data_snapshot),
         heap_(heap),
         mark_compact_epoch_(mark_compact_epoch),
         code_flush_mode_(code_flush_mode),
@@ -230,6 +234,7 @@ class MarkingVisitorBase : public HeapVisitor<int, ConcreteVisitor> {
 
   MarkingWorklists::Local* const local_marking_worklists_;
   WeakObjects::Local* const local_weak_objects_;
+  EmbedderDataSnapshot* const embedder_data_snapshot_;
   Heap* const heap_;
   const unsigned mark_compact_epoch_;
   const base::EnumSet<CodeFlushMode> code_flush_mode_;

src/heap/marking-worklist-inl.h

@@ -10,6 +10,7 @@
 #include "src/heap/cppgc-js/cpp-marking-state-inl.h"
 #include "src/heap/marking-worklist.h"
 #include "src/objects/js-objects-inl.h"
+#include "src/objects/property-details.h"
 
 namespace v8 {
 namespace internal {
@@ -47,16 +48,19 @@ bool MarkingWorklists::Local::PopOnHold(HeapObject* object) {
   return on_hold_.Pop(object);
 }
 
+void MarkingWorklists::Local::PushExtractedWrapper(
+    const EmbedderDataSlot& type_slot, const EmbedderDataSlot& instance_slot) {
+  DCHECK_NOT_NULL(cpp_marking_state_);
+  cpp_marking_state_->MarkAndPush(type_slot, instance_slot);
+}
+
 void MarkingWorklists::Local::PushWrapper(HeapObject object) {
-  if (cpp_marking_state_) {
-    cpp_marking_state_->MarkAndPush(JSObject::cast(object));
-  } else {
-    wrapper_.Push(object);
-  }
+  DCHECK_NULL(cpp_marking_state_);
+  wrapper_.Push(object);
 }
 
 bool MarkingWorklists::Local::PopWrapper(HeapObject* object) {
-  DCHECK(!cpp_marking_state_);
+  DCHECK_NULL(cpp_marking_state_);
   return wrapper_.Pop(object);
 }
 

src/heap/marking-worklist.h

@@ -19,6 +19,7 @@ namespace internal {
 
 class CppMarkingState;
 class JSObject;
+class EmbedderDataSlot;
 
 // The index of the main thread task used by concurrent/parallel GC.
 const int kMainThreadTask = 0;
@@ -157,6 +158,8 @@ class V8_EXPORT_PRIVATE MarkingWorklists::Local {
   inline void PushOnHold(HeapObject object);
   inline bool PopOnHold(HeapObject* object);
 
+  inline void PushExtractedWrapper(const EmbedderDataSlot& type_slot,
+                                   const EmbedderDataSlot& instance_slot);
   inline void PushWrapper(HeapObject object);
   inline bool PopWrapper(HeapObject* object);
 

src/objects/embedder-data-slot-inl.h

@@ -5,11 +5,10 @@
 #ifndef V8_OBJECTS_EMBEDDER_DATA_SLOT_INL_H_
 #define V8_OBJECTS_EMBEDDER_DATA_SLOT_INL_H_
 
-#include "src/objects/embedder-data-slot.h"
-
 #include "src/base/memory.h"
 #include "src/heap/heap-write-barrier-inl.h"
 #include "src/objects/embedder-data-array.h"
+#include "src/objects/embedder-data-slot.h"
 #include "src/objects/js-objects-inl.h"
 #include "src/objects/objects-inl.h"
 
@@ -27,6 +26,9 @@ EmbedderDataSlot::EmbedderDataSlot(JSObject object, int embedder_field_index)
     : SlotBase(FIELD_ADDR(
           object, object.GetEmbedderFieldOffset(embedder_field_index))) {}
 
+EmbedderDataSlot::EmbedderDataSlot(Address raw_address)
+    : SlotBase(raw_address) {}
+
 void EmbedderDataSlot::AllocateExternalPointerEntry(Isolate* isolate) {
 #ifdef V8_SANDBOXED_EXTERNAL_POINTERS
   // TODO(v8:10391, saelo): Use InitExternalPointerField() once

src/objects/embedder-data-slot.h

@@ -35,6 +35,7 @@ class EmbedderDataSlot
   EmbedderDataSlot() : SlotBase(kNullAddress) {}
   V8_INLINE EmbedderDataSlot(EmbedderDataArray array, int entry_index);
   V8_INLINE EmbedderDataSlot(JSObject object, int embedder_field_index);
+  V8_INLINE explicit EmbedderDataSlot(Address raw_address);
 
 #if defined(V8_TARGET_BIG_ENDIAN) && defined(V8_COMPRESS_POINTERS)
   static constexpr int kTaggedPayloadOffset = kTaggedSize;

test/cctest/test-api.cc

@@ -7841,6 +7841,12 @@ void CheckInternalFields(
 }
 
 void InternalFieldCallback(bool global_gc) {
+  // Manual GC scope as --stress-incremental-marking starts marking early and
+  // setting internal pointer fields mark the object for a heap layout change,
+  // which prevents it from being reclaimed and the callbacks from being
+  // executed.
+  ManualGCScope manual_gc_scope;
+
   LocalContext env;
   v8::Isolate* isolate = env->GetIsolate();
   v8::HandleScope scope(isolate);

test/unittests/heap/cppgc-js/unified-heap-unittest.cc

@@ -63,9 +63,10 @@ TEST_F(UnifiedHeapTest, FindingV8ToBlinkReference) {
   v8::Local<v8::Context> context = v8::Context::New(v8_isolate());
   v8::Context::Scope context_scope(context);
   uint16_t wrappable_type = WrapperHelper::kTracedEmbedderId;
-  v8::Local<v8::Object> api_object = WrapperHelper::CreateWrapper(
-      context, &wrappable_type,
-      cppgc::MakeGarbageCollected<Wrappable>(allocation_handle()));
+  auto* wrappable_object =
+      cppgc::MakeGarbageCollected<Wrappable>(allocation_handle());
+  v8::Local<v8::Object> api_object =
+      WrapperHelper::CreateWrapper(context, &wrappable_type, wrappable_object);
   Wrappable::destructor_callcount = 0;
   EXPECT_FALSE(api_object.IsEmpty());
   EXPECT_EQ(0u, Wrappable::destructor_callcount);