unified-young-gen: Support Oilpan tracing from minor MC

The CL adds standalone Oilpan tracing to minor MC. No cross-heap
references are currently processed. In addition, the CL removes
wrapper iteration from Oilpan Minor MC.

Bug: v8:13475
Change-Id: I3a0670e1f3431a3aa723217d5361e4e74f9b0c0f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4027209
Commit-Queue: Anton Bikineev <bikineev@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84306}

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

@@ -98,64 +98,6 @@ class MinorGCHeapGrowing
 
 }  // namespace internal
 
-namespace {
-
-START_ALLOW_USE_DEPRECATED()
-
-class V8ToCppGCReferencesVisitor final
-    : public v8::EmbedderHeapTracer::TracedGlobalHandleVisitor {
- public:
-  V8ToCppGCReferencesVisitor(
-      cppgc::internal::MutatorMarkingState& marking_state,
-      v8::internal::Isolate* isolate,
-      const v8::WrapperDescriptor& wrapper_descriptor)
-      : marking_state_(marking_state),
-        isolate_(isolate),
-        wrapper_descriptor_(wrapper_descriptor) {}
-
-  void VisitTracedReference(const v8::TracedReference<v8::Value>& value) final {
-    VisitHandle(value, value.WrapperClassId());
-  }
-
- private:
-  void VisitHandle(const v8::TracedReference<v8::Value>& value,
-                   uint16_t class_id) {
-    DCHECK(!value.IsEmpty());
-
-    const internal::JSObject js_object =
-        *reinterpret_cast<const internal::JSObject* const&>(value);
-    if (!js_object.ptr() || js_object.IsSmi() ||
-        !js_object.MayHaveEmbedderFields())
-      return;
-
-    internal::LocalEmbedderHeapTracer::WrapperInfo info;
-    if (!internal::LocalEmbedderHeapTracer::ExtractWrappableInfo(
-            isolate_, js_object, wrapper_descriptor_, &info))
-      return;
-
-    marking_state_.MarkAndPush(
-        cppgc::internal::HeapObjectHeader::FromObject(info.second));
-  }
-
-  cppgc::internal::MutatorMarkingState& marking_state_;
-  v8::internal::Isolate* isolate_;
-  const v8::WrapperDescriptor& wrapper_descriptor_;
-};
-
-END_ALLOW_USE_DEPRECATED()
-
-void TraceV8ToCppGCReferences(
-    v8::internal::Isolate* isolate,
-    cppgc::internal::MutatorMarkingState& marking_state,
-    const v8::WrapperDescriptor& wrapper_descriptor) {
-  DCHECK(isolate);
-  V8ToCppGCReferencesVisitor forwarding_visitor(marking_state, isolate,
-                                                wrapper_descriptor);
-  isolate->traced_handles()->Iterate(&forwarding_visitor);
-}
-
-}  // namespace
-
 // static
 constexpr uint16_t WrapperDescriptor::kUnknownEmbedderId;
 
@@ -702,12 +644,6 @@ void CppHeap::InitializeTracing(CollectionType collection_type,
 
   collection_type_ = collection_type;
 
-  if (collection_type == CollectionType::kMinor) {
-    if (!generational_gc_supported()) return;
-    // Notify GC tracer that CppGC started young GC cycle.
-    isolate_->heap()->tracer()->NotifyYoungCppGCRunning();
-  }
-
   CHECK(!sweeper_.IsSweepingInProgress());
 
   // Check that previous cycle metrics for the same collection type have been
@@ -813,11 +749,6 @@ void CppHeap::EnterFinalPause(cppgc::EmbedderStackState stack_state) {
             heap, *heap.mark_compact_collector()->local_marking_worklists()));
   }
   marker.EnterAtomicPause(stack_state);
-  if (isolate_ && *collection_type_ == CollectionType::kMinor) {
-    // Visit V8 -> cppgc references.
-    TraceV8ToCppGCReferences(isolate_, marker.GetMutatorMarkingState(),
-                             wrapper_descriptor_);
-  }
   compactor_.CancelIfShouldNotCompact(MarkingType::kAtomic, stack_state);
 }
 

src/heap/mark-compact.cc

@@ -5644,6 +5644,16 @@ void MinorMarkCompactCollector::SweepArrayBufferExtensions() {
       ArrayBufferSweeper::SweepingType::kYoung);
 }
 
+void MinorMarkCompactCollector::PerformWrapperTracing() {
+  if (!heap_->local_embedder_heap_tracer()->InUse()) return;
+  TRACE_GC(heap()->tracer(), GCTracer::Scope::MINOR_MC_MARK_EMBEDDER_TRACING);
+  const bool published = local_marking_worklists()->PublishWrapper();
+  DCHECK(published);
+  USE(published);
+  heap_->local_embedder_heap_tracer()->Trace(
+      std::numeric_limits<double>::infinity());
+}
+
 class YoungGenerationMigrationObserver final : public MigrationObserver {
  public:
   YoungGenerationMigrationObserver(Heap* heap,
@@ -6091,6 +6101,8 @@ class YoungGenerationMarkingTask {
            marking_worklists_local_->PopOnHold(&object)) {
       visitor_.Visit(object);
     }
+    // Publish wrapper objects to the cppgc marking state, if registered.
+    marking_worklists_local_->PublishWrapper();
   }
 
   void PublishMarkingWorklist() { marking_worklists_local_->Publish(); }
@@ -6314,6 +6326,10 @@ void MinorMarkCompactCollector::MarkLiveObjects() {
 
   MarkRootSetInParallel(&root_visitor, was_marked_incrementally);
 
+  if (auto* cpp_heap = CppHeap::From(heap_->cpp_heap())) {
+    cpp_heap->FinishConcurrentMarkingIfNeeded();
+  }
+
   // Mark rest on the main thread.
   {
     TRACE_GC(heap()->tracer(), GCTracer::Scope::MINOR_MC_MARK_CLOSURE);
@@ -6340,14 +6356,19 @@ void MinorMarkCompactCollector::MarkLiveObjects() {
 
 void MinorMarkCompactCollector::DrainMarkingWorklist() {
   PtrComprCageBase cage_base(isolate());
-  HeapObject object;
+  do {
-  while (local_marking_worklists_->Pop(&object)) {
+    PerformWrapperTracing();
-    DCHECK(!object.IsFreeSpaceOrFiller(cage_base));
+
-    DCHECK(object.IsHeapObject());
+    HeapObject object;
-    DCHECK(heap()->Contains(object));
+    while (local_marking_worklists_->Pop(&object)) {
-    DCHECK(!non_atomic_marking_state()->IsWhite(object));
+      DCHECK(!object.IsFreeSpaceOrFiller(cage_base));
-    main_marking_visitor_->Visit(object);
+      DCHECK(object.IsHeapObject());
-  }
+      DCHECK(heap()->Contains(object));
+      DCHECK(!non_atomic_marking_state()->IsWhite(object));
+      main_marking_visitor_->Visit(object);
+    }
+  } while (!local_marking_worklists_->IsWrapperEmpty() ||
+           !heap()->local_embedder_heap_tracer()->IsRemoteTracingDone());
   DCHECK(local_marking_worklists_->IsEmpty());
 }
 

src/heap/mark-compact.h

@@ -707,6 +707,9 @@ class MinorMarkCompactCollector final : public CollectorBase {
 
   void VisitObject(HeapObject obj) final;
 
+  // Perform Wrapper Tracing if in use.
+  void PerformWrapperTracing();
+
  private:
   class RootMarkingVisitor;
 

test/unittests/BUILD.gn

@@ -398,6 +398,7 @@ v8_source_set("unittests_sources") {
     "heap/cppgc-js/unified-heap-unittest.cc",
     "heap/cppgc-js/unified-heap-utils.cc",
     "heap/cppgc-js/unified-heap-utils.h",
+    "heap/cppgc-js/young-unified-heap-unittest.cc",
     "heap/embedder-tracing-unittest.cc",
     "heap/gc-idle-time-handler-unittest.cc",
     "heap/gc-tracer-unittest.cc",

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

@@ -49,6 +49,27 @@ void UnifiedHeapTest::CollectGarbageWithoutEmbedderStack(
   }
 }
 
+void UnifiedHeapTest::CollectYoungGarbageWithEmbedderStack(
+    cppgc::Heap::SweepingType sweeping_type) {
+  EmbedderStackStateScope stack_scope(
+      heap(), EmbedderStackStateScope::kExplicitInvocation,
+      StackState::kMayContainHeapPointers);
+  CollectGarbage(NEW_SPACE);
+  if (sweeping_type == cppgc::Heap::SweepingType::kAtomic) {
+    cpp_heap().AsBase().sweeper().FinishIfRunning();
+  }
+}
+void UnifiedHeapTest::CollectYoungGarbageWithoutEmbedderStack(
+    cppgc::Heap::SweepingType sweeping_type) {
+  EmbedderStackStateScope stack_scope(
+      heap(), EmbedderStackStateScope::kExplicitInvocation,
+      StackState::kNoHeapPointers);
+  CollectGarbage(NEW_SPACE);
+  if (sweeping_type == cppgc::Heap::SweepingType::kAtomic) {
+    cpp_heap().AsBase().sweeper().FinishIfRunning();
+  }
+}
+
 CppHeap& UnifiedHeapTest::cpp_heap() const {
   return *CppHeap::From(isolate()->heap()->cpp_heap());
 }

test/unittests/heap/cppgc-js/unified-heap-utils.h

@@ -31,6 +31,13 @@ class UnifiedHeapTest : public TestWithHeapInternals {
       cppgc::Heap::SweepingType sweeping_type =
           cppgc::Heap::SweepingType::kAtomic);
 
+  void CollectYoungGarbageWithEmbedderStack(
+      cppgc::Heap::SweepingType sweeping_type =
+          cppgc::Heap::SweepingType::kAtomic);
+  void CollectYoungGarbageWithoutEmbedderStack(
+      cppgc::Heap::SweepingType sweeping_type =
+          cppgc::Heap::SweepingType::kAtomic);
+
   CppHeap& cpp_heap() const;
   cppgc::AllocationHandle& allocation_handle();
 

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

@@ -0,0 +1,93 @@
+// 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.
+
+#if defined(CPPGC_YOUNG_GENERATION)
+
+#include <memory>
+
+#include "include/cppgc/allocation.h"
+#include "include/cppgc/garbage-collected.h"
+#include "include/cppgc/testing.h"
+#include "include/v8-context.h"
+#include "include/v8-cppgc.h"
+#include "include/v8-local-handle.h"
+#include "include/v8-object.h"
+#include "include/v8-traced-handle.h"
+#include "src/api/api-inl.h"
+#include "src/common/globals.h"
+#include "src/heap/cppgc-js/cpp-heap.h"
+#include "src/heap/cppgc/heap-object-header.h"
+#include "src/objects/objects-inl.h"
+#include "test/common/flag-utils.h"
+#include "test/unittests/heap/cppgc-js/unified-heap-utils.h"
+#include "test/unittests/heap/heap-utils.h"
+
+namespace v8 {
+namespace internal {
+
+namespace {
+
+class Wrappable final : public cppgc::GarbageCollected<Wrappable> {
+ public:
+  static size_t destructor_callcount;
+
+  ~Wrappable() { destructor_callcount++; }
+
+  void Trace(cppgc::Visitor* visitor) const { visitor->Trace(wrapper_); }
+
+  void SetWrapper(v8::Isolate* isolate, v8::Local<v8::Object> wrapper) {
+    wrapper_.Reset(isolate, wrapper);
+  }
+
+  TracedReference<v8::Object>& wrapper() { return wrapper_; }
+
+ private:
+  TracedReference<v8::Object> wrapper_;
+};
+
+size_t Wrappable::destructor_callcount = 0;
+
+class MinorMCEnabler {
+ public:
+  MinorMCEnabler()
+      : minor_mc_(&v8_flags.minor_mc, true),
+        cppgc_young_generation_(&v8_flags.cppgc_young_generation, true) {}
+
+ private:
+  FlagScope<bool> minor_mc_;
+  FlagScope<bool> cppgc_young_generation_;
+};
+
+}  // namespace
+
+class YoungUnifiedHeapTest : public MinorMCEnabler, public UnifiedHeapTest {
+ public:
+  YoungUnifiedHeapTest() {
+    // Enable young generation flag and run GC. After the first run the heap
+    // will enable minor GC.
+    CollectGarbageWithoutEmbedderStack();
+  }
+};
+
+TEST_F(YoungUnifiedHeapTest, OnlyGC) { CollectYoungGarbageWithEmbedderStack(); }
+
+TEST_F(YoungUnifiedHeapTest, CollectUnreachableCppGCObject) {
+  v8::HandleScope scope(v8_isolate());
+  v8::Local<v8::Context> context = v8::Context::New(v8_isolate());
+  v8::Context::Scope context_scope(context);
+
+  cppgc::MakeGarbageCollected<Wrappable>(allocation_handle());
+  v8::Local<v8::Object> api_object =
+      WrapperHelper::CreateWrapper(context, nullptr, nullptr);
+  EXPECT_FALSE(api_object.IsEmpty());
+
+  Wrappable::destructor_callcount = 0;
+  CollectYoungGarbageWithoutEmbedderStack(cppgc::Heap::SweepingType::kAtomic);
+  EXPECT_EQ(1u, Wrappable::destructor_callcount);
+}
+
+}  // namespace internal
+}  // namespace v8
+
+#endif  // defined(CPPGC_YOUNG_GENERATION)