Revert "cppgc-js,heap: Implement snapshots for embedder fields"

This reverts commit 142dd775.

Reason for revert: TSAN breaks: https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20TSAN%20-%20stress-incremental-marking/6113/overview

Original change's description:
> 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/+/3380983
> Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
> Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#78604}

Bug: chromium:1285706
Change-Id: If1976c0356f450fc068aa4dcc39fb9a0d5417a40
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3386598
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Owners-Override: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#78605}

BUILD.bazel

@@ -1325,8 +1325,6 @@ 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,8 +2954,6 @@ 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,36 +6032,18 @@ 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);
-
-  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::DisallowGarbageCollection no_gc;
   i::JSObject js_obj = i::JSObject::cast(*obj);
   int nof_embedder_fields = js_obj.GetEmbedderFieldCount();
   for (int i = 0; i < argc; i++) {
@@ -6077,10 +6059,6 @@ 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,7 +10,6 @@
 #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"
@@ -87,16 +86,15 @@ class ConcurrentMarkingVisitor final
  public:
   ConcurrentMarkingVisitor(int task_id,
                            MarkingWorklists::Local* local_marking_worklists,
-                           WeakObjects::Local* local_weak_objects,
-                           EmbedderDataSnapshot* embedder_data_snapshot,
-                           Heap* heap, unsigned mark_compact_epoch,
+                           WeakObjects::Local* local_weak_objects, 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,
-                           embedder_data_snapshot, heap, mark_compact_epoch,
-                           code_flush_mode, embedder_tracing_enabled,
+      : MarkingVisitorBase(local_marking_worklists, local_weak_objects, 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) {}
@@ -459,16 +457,11 @@ 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,
-      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);
+      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);
   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/embedder-data-slot.h"
+#include "src/objects/js-objects.h"
 
 namespace v8 {
 namespace internal {
 
-void CppMarkingState::MarkAndPush(const EmbedderDataSlot& type_slot,
-                                  const EmbedderDataSlot& instance_slot) {
+void CppMarkingState::MarkAndPush(const JSObject& js_object) {
+  DCHECK(js_object.IsApiWrapper());
   LocalEmbedderHeapTracer::WrapperInfo info;
   if (LocalEmbedderHeapTracer::ExtractWrappableInfo(
-          isolate_, wrapper_descriptor_, type_slot, instance_slot, &info)) {
+          isolate_, js_object, wrapper_descriptor_, &info)) {
     marking_state_.MarkAndPush(
         cppgc::internal::HeapObjectHeader::FromObject(info.second));
   }

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

@@ -15,7 +15,6 @@ namespace v8 {
 namespace internal {
 
 class JSObject;
-class EmbedderDataSlot;
 
 class CppMarkingState {
  public:
@@ -38,8 +37,7 @@ class CppMarkingState {
 
   void Publish() { marking_state_.Publish(); }
 
-  inline void MarkAndPush(const EmbedderDataSlot& type_slot,
-                          const EmbedderDataSlot& instance_slot);
+  inline void MarkAndPush(const JSObject& js_object);
 
   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,20 +18,11 @@ bool LocalEmbedderHeapTracer::ExtractWrappableInfo(
   DCHECK(js_object.IsApiWrapper());
   if (js_object.GetEmbedderFieldCount() < 2) return false;
 
-  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) &&
+  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) &&
       info->second) {
     return (wrapper_descriptor.embedder_id_for_garbage_collected ==
             WrapperDescriptor::kUnknownEmbedderId) ||

src/heap/embedder-tracing.cc

@@ -197,13 +197,8 @@ void LocalEmbedderHeapTracer::EmbedderWriteBarrier(Heap* heap,
   DCHECK(js_object.IsApiWrapper());
   if (cpp_heap_) {
     DCHECK_NOT_NULL(heap->mark_compact_collector());
-    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);
+    heap->mark_compact_collector()->local_marking_worklists()->PushWrapper(
+        js_object);
     return;
   }
   LocalEmbedderHeapTracer::ProcessingScope scope(this);

src/heap/embedder-tracing.h

@@ -77,9 +77,6 @@ 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,9 +49,11 @@ 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,8 +22,6 @@
 #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"
@@ -580,12 +578,9 @@ 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(),
-      embedder_data_snapshot_.get(), heap_, epoch(), code_flush_mode(),
+      heap_, epoch(), code_flush_mode(),
       heap_->local_embedder_heap_tracer()->InUse(),
       heap_->ShouldCurrentGCKeepAgesUnchanged());
 // Marking bits are cleared by the sweeper.
@@ -1003,7 +998,6 @@ 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,7 +11,6 @@
 #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"
@@ -24,7 +23,6 @@ namespace v8 {
 namespace internal {
 
 // Forward declarations.
-class EmbedderDataSnapshot;
 class EvacuationJobTraits;
 class HeapObjectVisitor;
 class ItemParallelJob;
@@ -392,15 +390,14 @@ class MainMarkingVisitor final
 
   MainMarkingVisitor(MarkingState* marking_state,
                      MarkingWorklists::Local* local_marking_worklists,
-                     WeakObjects::Local* local_weak_objects,
-                     EmbedderDataSnapshot* embedder_data_snapshot, Heap* heap,
+                     WeakObjects::Local* local_weak_objects, 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, embedder_data_snapshot,
-            heap, mark_compact_epoch, code_flush_mode, embedder_tracing_enabled,
+            local_marking_worklists, local_weak_objects, heap,
+            mark_compact_epoch, code_flush_mode, embedder_tracing_enabled,
             should_keep_ages_unchanged),
         marking_state_(marking_state),
         revisiting_object_(false) {}
@@ -791,7 +788,6 @@ 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,7 +5,6 @@
 #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"
@@ -265,20 +264,11 @@ 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.
-    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);
-    }
+    // Success: The object needs to be processed for embedder references on
+    // the main thread.
+    local_marking_worklists_->PushWrapper(object);
   }
   return size;
 }

src/heap/marking-visitor.h

@@ -6,7 +6,6 @@
 #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"
@@ -17,8 +16,6 @@
 namespace v8 {
 namespace internal {
 
-class EmbedderDataSnapshot;
-
 struct EphemeronMarking {
   std::vector<HeapObject> newly_discovered;
   bool newly_discovered_overflowed;
@@ -131,15 +128,14 @@ class MarkingVisitorBase : public HeapVisitor<int, ConcreteVisitor> {
  public:
   MarkingVisitorBase(MarkingWorklists::Local* local_marking_worklists,
                      WeakObjects::Local* local_weak_objects,
-                     EmbedderDataSnapshot* embedder_data_snapshot, Heap* heap,
-                     unsigned mark_compact_epoch,
+                     //  WeakObjects* weak_objects,
+                     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),
@@ -234,7 +230,6 @@ 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,7 +10,6 @@
 #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 {
@@ -48,19 +47,16 @@ 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) {
-  DCHECK_NULL(cpp_marking_state_);
-  wrapper_.Push(object);
+  if (cpp_marking_state_) {
+    cpp_marking_state_->MarkAndPush(JSObject::cast(object));
+  } else {
+    wrapper_.Push(object);
+  }
 }
 
 bool MarkingWorklists::Local::PopWrapper(HeapObject* object) {
-  DCHECK_NULL(cpp_marking_state_);
+  DCHECK(!cpp_marking_state_);
   return wrapper_.Pop(object);
 }
 

src/heap/marking-worklist.h

@@ -19,7 +19,6 @@ namespace internal {
 
 class CppMarkingState;
 class JSObject;
-class EmbedderDataSlot;
 
 // The index of the main thread task used by concurrent/parallel GC.
 const int kMainThreadTask = 0;
@@ -158,8 +157,6 @@ 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,10 +5,11 @@
 #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"
 
@@ -26,9 +27,6 @@ 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,7 +35,6 @@ 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,12 +7841,6 @@ 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,10 +63,9 @@ 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;
-  auto* wrappable_object =
-      cppgc::MakeGarbageCollected<Wrappable>(allocation_handle());
-  v8::Local<v8::Object> api_object =
-      WrapperHelper::CreateWrapper(context, &wrappable_type, wrappable_object);
+  v8::Local<v8::Object> api_object = WrapperHelper::CreateWrapper(
+      context, &wrappable_type,
+      cppgc::MakeGarbageCollected<Wrappable>(allocation_handle()));
   Wrappable::destructor_callcount = 0;
   EXPECT_FALSE(api_object.IsEmpty());
   EXPECT_EQ(0u, Wrappable::destructor_callcount);