[heap] Move wrappable extraction logic out of LocalEmbedderHeapTracer

Bug: v8:13207
Change-Id: I5d96454c7335e698ff79572706cf0c16640fdd53
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4136711
Reviewed-by: Anton Bikineev <bikineev@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#85100}

BUILD.bazel

@@ -1444,9 +1444,10 @@ 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/cppgc-js/wrappable-info.h",
+        "src/heap/cppgc-js/wrappable-info-inl.h",
         "src/heap/embedder-tracing.cc",
         "src/heap/embedder-tracing.h",
-        "src/heap/embedder-tracing-inl.h",
         "src/heap/evacuation-verifier.cc",
         "src/heap/evacuation-verifier.h",
         "src/heap/evacuation-verifier-inl.h",

BUILD.gn

@@ -3128,7 +3128,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-tracing-inl.h",
+    "src/heap/cppgc-js/wrappable-info-inl.h",
+    "src/heap/cppgc-js/wrappable-info.h",
     "src/heap/embedder-tracing.h",
     "src/heap/evacuation-allocator-inl.h",
     "src/heap/evacuation-allocator.h",

src/heap/cppgc-js/cpp-heap.cc

@@ -43,7 +43,6 @@
 #include "src/heap/cppgc/sweeper.h"
 #include "src/heap/cppgc/unmarker.h"
 #include "src/heap/cppgc/visitor.h"
-#include "src/heap/embedder-tracing-inl.h"
 #include "src/heap/embedder-tracing.h"
 #include "src/heap/gc-tracer.h"
 #include "src/heap/heap.h"
@@ -763,6 +762,21 @@ bool CppHeap::FinishConcurrentMarkingIfNeeded() {
   return marker_->JoinConcurrentMarkingIfNeeded();
 }
 
+void CppHeap::WriteBarrier(JSObject js_object) {
+  DCHECK(js_object.MayHaveEmbedderFields());
+  DCHECK_NOT_NULL(isolate()->heap()->mark_compact_collector());
+  auto descriptor = wrapper_descriptor();
+  const EmbedderDataSlot type_slot(js_object, descriptor.wrappable_type_index);
+  const EmbedderDataSlot instance_slot(js_object,
+                                       descriptor.wrappable_instance_index);
+  isolate()
+      ->heap()
+      ->mark_compact_collector()
+      ->local_marking_worklists()
+      ->cpp_marking_state()
+      ->MarkAndPush(type_slot, instance_slot);
+}
+
 namespace {
 
 void RecordEmbedderSpeed(GCTracer* tracer, base::TimeDelta marking_time,

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

@@ -144,6 +144,7 @@ class V8_EXPORT_PRIVATE CppHeap final
   void TraceEpilogue();
   void EnterFinalPause(cppgc::EmbedderStackState stack_state);
   bool FinishConcurrentMarkingIfNeeded();
+  void WriteBarrier(JSObject);
 
   // StatsCollector::AllocationObserver interface.
   void AllocatedObjectSizeIncreased(size_t) final;

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

@@ -6,7 +6,8 @@
 #define V8_HEAP_CPPGC_JS_CPP_MARKING_STATE_INL_H_
 
 #include "src/heap/cppgc-js/cpp-marking-state.h"
-#include "src/heap/embedder-tracing-inl.h"
+#include "src/heap/cppgc-js/wrappable-info-inl.h"
+#include "src/heap/cppgc-js/wrappable-info.h"
 #include "src/objects/embedder-data-slot.h"
 #include "src/objects/js-objects.h"
 
@@ -33,11 +34,11 @@ void CppMarkingState::MarkAndPush(const EmbedderDataSnapshot& snapshot) {
 
 void CppMarkingState::MarkAndPush(const EmbedderDataSlot type_slot,
                                   const EmbedderDataSlot instance_slot) {
-  LocalEmbedderHeapTracer::WrapperInfo info;
+  const auto maybe_info = WrappableInfo::From(
-  if (LocalEmbedderHeapTracer::ExtractWrappableInfo(
+      isolate_, type_slot, instance_slot, wrapper_descriptor_);
-          isolate_, wrapper_descriptor_, type_slot, instance_slot, &info)) {
+  if (maybe_info.has_value()) {
     marking_state_.MarkAndPush(
-        cppgc::internal::HeapObjectHeader::FromObject(info.second));
+        cppgc::internal::HeapObjectHeader::FromObject(maybe_info->instance));
   }
 }
 

src/heap/cppgc-js/cpp-snapshot.cc

@@ -14,6 +14,8 @@
 #include "src/base/logging.h"
 #include "src/execution/isolate.h"
 #include "src/heap/cppgc-js/cpp-heap.h"
+#include "src/heap/cppgc-js/wrappable-info-inl.h"
+#include "src/heap/cppgc-js/wrappable-info.h"
 #include "src/heap/cppgc/heap-object-header.h"
 #include "src/heap/cppgc/heap-visitor.h"
 #include "src/heap/cppgc/visitor.h"
@@ -352,7 +354,7 @@ class StateStorage final {
   size_t state_count_ = 0;
 };
 
-void* ExtractEmbedderDataBackref(Isolate* isolate,
+void* ExtractEmbedderDataBackref(Isolate* isolate, CppHeap& cpp_heap,
                                  v8::Local<v8::Value> v8_value) {
   // See LocalEmbedderHeapTracer::VerboseWrapperTypeInfo for details on how
   // wrapper objects are set up.
@@ -364,10 +366,10 @@ void* ExtractEmbedderDataBackref(Isolate* isolate,
     return nullptr;
 
   JSObject js_object = JSObject::cast(*v8_object);
-  return LocalEmbedderHeapTracer::VerboseWrapperInfo(
+
-             isolate->heap()->local_embedder_heap_tracer()->ExtractWrapperInfo(
+  const auto maybe_info =
-                 isolate, js_object))
+      WrappableInfo::From(isolate, js_object, cpp_heap.wrapper_descriptor());
-      .instance();
+  return maybe_info.has_value() ? maybe_info->instance : nullptr;
 }
 
 // The following implements a snapshotting algorithm for C++ objects that also
@@ -488,7 +490,7 @@ class CppGraphBuilderImpl final {
 
       void* back_reference_object = ExtractEmbedderDataBackref(
           reinterpret_cast<v8::internal::Isolate*>(cpp_heap_.isolate()),
-          v8_value);
+          cpp_heap_, v8_value);
       if (back_reference_object) {
         auto& back_header = HeapObjectHeader::FromObject(back_reference_object);
         auto& back_state = states_.GetExistingState(back_header);

src/heap/cppgc-js/wrappable-info-inl.h

@@ -0,0 +1,50 @@
+// Copyright 2023 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_JS_WRAPPABLE_INFO_INL_H_
+#define V8_HEAP_CPPGC_JS_WRAPPABLE_INFO_INL_H_
+
+#include "src/base/optional.h"
+#include "src/heap/cppgc-js/wrappable-info.h"
+#include "src/objects/embedder-data-slot.h"
+#include "src/objects/js-objects-inl.h"
+
+namespace v8::internal {
+
+// static
+base::Optional<WrappableInfo> WrappableInfo::From(
+    Isolate* isolate, JSObject wrapper,
+    const WrapperDescriptor& wrapper_descriptor) {
+  DCHECK(wrapper.MayHaveEmbedderFields());
+  return wrapper.GetEmbedderFieldCount() < 2
+             ? base::Optional<WrappableInfo>()
+             : From(isolate,
+                    EmbedderDataSlot(wrapper,
+                                     wrapper_descriptor.wrappable_type_index),
+                    EmbedderDataSlot(
+                        wrapper, wrapper_descriptor.wrappable_instance_index),
+                    wrapper_descriptor);
+}
+
+// static
+base::Optional<WrappableInfo> WrappableInfo::From(
+    Isolate* isolate, const EmbedderDataSlot& type_slot,
+    const EmbedderDataSlot& instance_slot,
+    const WrapperDescriptor& wrapper_descriptor) {
+  void* type;
+  void* instance;
+  if (type_slot.ToAlignedPointer(isolate, &type) && type &&
+      instance_slot.ToAlignedPointer(isolate, &instance) && instance &&
+      (wrapper_descriptor.embedder_id_for_garbage_collected ==
+           WrapperDescriptor::kUnknownEmbedderId ||
+       (*static_cast<uint16_t*>(type) ==
+        wrapper_descriptor.embedder_id_for_garbage_collected))) {
+    return base::Optional<WrappableInfo>(base::in_place, type, instance);
+  }
+  return {};
+}
+
+}  // namespace v8::internal
+
+#endif  // V8_HEAP_CPPGC_JS_WRAPPABLE_INFO_INL_H_

src/heap/cppgc-js/wrappable-info.h

@@ -0,0 +1,34 @@
+// Copyright 2023 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_JS_WRAPPABLE_INFO_H_
+#define V8_HEAP_CPPGC_JS_WRAPPABLE_INFO_H_
+
+#include "include/v8-cppgc.h"
+#include "src/base/optional.h"
+#include "src/objects/embedder-data-slot.h"
+#include "src/objects/js-objects.h"
+
+namespace v8::internal {
+
+class Isolate;
+
+struct WrappableInfo final {
+ public:
+  static V8_INLINE base::Optional<WrappableInfo> From(Isolate*, JSObject,
+                                                      const WrapperDescriptor&);
+  static V8_INLINE base::Optional<WrappableInfo> From(
+      Isolate*, const EmbedderDataSlot& type_slot,
+      const EmbedderDataSlot& instance_slot, const WrapperDescriptor&);
+
+  constexpr WrappableInfo(void* type, void* instance)
+      : type(type), instance(instance) {}
+
+  void* type = nullptr;
+  void* instance = nullptr;
+};
+
+}  // namespace v8::internal
+
+#endif  // V8_HEAP_CPPGC_JS_WRAPPABLE_INFO_H_

src/heap/embedder-tracing-inl.h

@@ -1,46 +0,0 @@
-// Copyright 2021 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_TRACING_INL_H_
-#define V8_HEAP_EMBEDDER_TRACING_INL_H_
-
-#include "src/heap/embedder-tracing.h"
-#include "src/objects/embedder-data-slot.h"
-#include "src/objects/js-objects-inl.h"
-
-namespace v8 {
-namespace internal {
-
-// static
-bool LocalEmbedderHeapTracer::ExtractWrappableInfo(
-    Isolate* isolate, JSObject js_object,
-    const WrapperDescriptor& wrapper_descriptor, WrapperInfo* info) {
-  DCHECK(js_object.MayHaveEmbedderFields());
-  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.ToAlignedPointer(isolate, &info->first) && info->first &&
-      instance_slot.ToAlignedPointer(isolate, &info->second) && info->second) {
-    return (wrapper_descriptor.embedder_id_for_garbage_collected ==
-            WrapperDescriptor::kUnknownEmbedderId) ||
-           (*static_cast<uint16_t*>(info->first) ==
-            wrapper_descriptor.embedder_id_for_garbage_collected);
-  }
-  return false;
-}
-
-}  // namespace internal
-}  // namespace v8
-
-#endif  // V8_HEAP_EMBEDDER_TRACING_INL_H_

src/heap/embedder-tracing.cc

@@ -7,15 +7,10 @@
 #include "include/cppgc/common.h"
 #include "include/v8-cppgc.h"
 #include "src/base/logging.h"
-#include "src/handles/global-handles.h"
-#include "src/heap/embedder-tracing-inl.h"
-#include "src/heap/gc-tracer.h"
 #include "src/heap/marking-worklist-inl.h"
 
 namespace v8::internal {
 
-START_ALLOW_USE_DEPRECATED()
-
 void LocalEmbedderHeapTracer::SetCppHeap(CppHeap* cpp_heap) {
   cpp_heap_ = cpp_heap;
 }
@@ -70,33 +65,4 @@ bool LocalEmbedderHeapTracer::IsRemoteTracingDone() {
   return !InUse() || cpp_heap()->IsTracingDone();
 }
 
-LocalEmbedderHeapTracer::WrapperInfo
-LocalEmbedderHeapTracer::ExtractWrapperInfo(Isolate* isolate,
-                                            JSObject js_object) {
-  DCHECK(InUse());
-  WrapperInfo info;
-  if (ExtractWrappableInfo(isolate, js_object, wrapper_descriptor(), &info)) {
-    return info;
-  }
-  return {nullptr, nullptr};
-}
-
-void LocalEmbedderHeapTracer::EmbedderWriteBarrier(Heap* heap,
-                                                   JSObject js_object) {
-  DCHECK(InUse());
-  DCHECK(js_object.MayHaveEmbedderFields());
-  DCHECK_NOT_NULL(heap->mark_compact_collector());
-  auto descriptor = wrapper_descriptor();
-  const EmbedderDataSlot type_slot(js_object, descriptor.wrappable_type_index);
-  const EmbedderDataSlot instance_slot(js_object,
-                                       descriptor.wrappable_instance_index);
-  heap->mark_compact_collector()
-      ->local_marking_worklists()
-      ->cpp_marking_state()
-      ->MarkAndPush(type_slot, instance_slot);
-  return;
-}
-
-END_ALLOW_USE_DEPRECATED()
-
 }  // namespace v8::internal

src/heap/embedder-tracing.h

@@ -25,32 +25,6 @@ class V8_EXPORT_PRIVATE LocalEmbedderHeapTracer final {
     kMinor,
     kMajor,
   };
-  using WrapperInfo = std::pair<void*, void*>;
-
-  // WrapperInfo is passed over the API. Use VerboseWrapperInfo to access pair
-  // internals in a named way. See ProcessingScope::TracePossibleJSWrapper()
-  // below on how a V8 object is parsed to gather the information.
-  struct VerboseWrapperInfo {
-    constexpr explicit VerboseWrapperInfo(const WrapperInfo& raw_info)
-        : raw_info(raw_info) {}
-
-    // Information describing the type pointed to via instance().
-    void* type_info() const { return raw_info.first; }
-    // Direct pointer to an instance described by type_info().
-    void* instance() const { return raw_info.second; }
-    // Returns whether the info is empty and thus does not keep a C++ object
-    // alive.
-    bool is_empty() const { return !type_info() || !instance(); }
-
-    const WrapperInfo& raw_info;
-  };
-
-  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) {}
 
@@ -87,14 +61,10 @@ class V8_EXPORT_PRIVATE LocalEmbedderHeapTracer final {
     embedder_worklist_empty_ = is_empty;
   }
 
-  WrapperInfo ExtractWrapperInfo(Isolate* isolate, JSObject js_object);
-
   cppgc::EmbedderStackState embedder_stack_state() const {
     return embedder_stack_state_;
   }
 
-  void EmbedderWriteBarrier(Heap*, JSObject);
-
  private:
   CppHeap* cpp_heap() {
     DCHECK_NOT_NULL(cpp_heap_);
@@ -102,10 +72,6 @@ class V8_EXPORT_PRIVATE LocalEmbedderHeapTracer final {
     return cpp_heap_;
   }
 
-  WrapperDescriptor wrapper_descriptor() {
-    return cpp_heap()->wrapper_descriptor();
-  }
-
   Isolate* const isolate_;
   CppHeap* cpp_heap_ = nullptr;
 

src/heap/heap-write-barrier.cc

@@ -58,10 +58,9 @@ void WriteBarrier::MarkingSlowFromGlobalHandle(HeapObject value) {
 
 // static
 void WriteBarrier::MarkingSlowFromInternalFields(Heap* heap, JSObject host) {
-  auto* local_embedder_heap_tracer = heap->local_embedder_heap_tracer();
+  if (auto* cpp_heap = heap->cpp_heap()) {
-  if (!local_embedder_heap_tracer->InUse()) return;
+    CppHeap::From(cpp_heap)->WriteBarrier(host);
-
+  }
-  local_embedder_heap_tracer->EmbedderWriteBarrier(heap, host);
 }
 
 void WriteBarrier::MarkingSlow(Code host, RelocInfo* reloc_info,