api, cppgc-js: Allow creating a v8::CppHeap in detached state

The detached CppHeap allows for allocation without invoking garbage
collections.  Allocated bytes are reported on the first allocation
after the CppHeap has been attached to an Isolate.

States:
- Detached: Allow only allocation;
- Attached: Unified heap GCs;
- Termination GC: Require detached state;

Destruction:
- Heap::TearDown: Detach if attached;
- ~CppHeap: Detach if attached;

Bug: chromium:1056170
Change-Id: I95ce029f36a7f10392257080b6e23e13cc0fc7b8
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2672940
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72579}

include/v8-cppgc.h

@@ -35,6 +35,9 @@ struct V8_EXPORT CppHeapCreateParams {
  */
 class V8_EXPORT CppHeap {
  public:
+  static std::unique_ptr<CppHeap> Create(v8::Platform* platform,
+                                         const CppHeapCreateParams& params);
+
   virtual ~CppHeap() = default;
 
   /**

include/v8.h

@@ -93,7 +93,6 @@ class Utils;
 class Value;
 class WasmMemoryObject;
 class WasmModuleObject;
-struct CppHeapCreateParams;
 template <class K, class V, class T>
 class GlobalValueMap;
 template <class K, class V, class T>
@@ -8469,16 +8468,6 @@ class V8_EXPORT Isolate {
     int embedder_wrapper_type_index = -1;
     int embedder_wrapper_object_index = -1;
 
-    /**
-     * If parameters are set, V8 creates a managed C++ heap as extension to its
-     * JavaScript heap.
-     *
-     * See v8::Isolate::GetCppHeap() for working with the heap.
-     *
-     * This is an experimental feature and may still change significantly.
-     */
-    std::shared_ptr<CppHeapCreateParams> cpp_heap_params;
-
     V8_DEPRECATED(
         "Setting this has no effect. Embedders should ignore import assertions "
         "that they do not use.")
@@ -9100,8 +9089,26 @@ class V8_EXPORT Isolate {
   EmbedderHeapTracer* GetEmbedderHeapTracer();
 
   /**
-   * \returns the C++ heap managed by V8. Only available if the Isolate was
-   *   created with proper CreatePrams::cpp_heap_params option.
+   * Attaches a managed C++ heap as an extension to the JavaScript heap. The
+   * embedder maintains ownership of the CppHeap. At most one C++ heap can be
+   * attached to V8.
+   *
+   * This is an experimental feature and may still change significantly.
+   */
+  void AttachCppHeap(CppHeap*);
+
+  /**
+   * Detaches a managed C++ heap if one was attached using `AttachCppHeap()`.
+   *
+   * This is an experimental feature and may still change significantly.
+   */
+  void DetachCppHeap();
+
+  /**
+   * This is an experimental feature and may still change significantly.
+
+   * \returns the C++ heap managed by V8. Only available if such a heap has been
+   *   attached using `AttachCppHeap()`.
    */
   CppHeap* GetCppHeap() const;
 

src/api/api.cc

@@ -8223,6 +8223,16 @@ EmbedderHeapTracer* Isolate::GetEmbedderHeapTracer() {
   return isolate->heap()->GetEmbedderHeapTracer();
 }
 
+void Isolate::AttachCppHeap(CppHeap* cpp_heap) {
+  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(this);
+  isolate->heap()->AttachCppHeap(cpp_heap);
+}
+
+void Isolate::DetachCppHeap() {
+  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(this);
+  isolate->heap()->DetachCppHeap();
+}
+
 CppHeap* Isolate::GetCppHeap() const {
   const i::Isolate* isolate = reinterpret_cast<const i::Isolate*>(this);
   return isolate->heap()->cpp_heap();
@@ -8335,9 +8345,6 @@ void Isolate::Initialize(Isolate* isolate,
   i_isolate->set_allow_atomics_wait(params.allow_atomics_wait);
 
   i_isolate->heap()->ConfigureHeap(params.constraints);
-  if (params.cpp_heap_params) {
-    i_isolate->heap()->ConfigureCppHeap(params.cpp_heap_params);
-  }
   if (params.constraints.stack_limit() != nullptr) {
     uintptr_t limit =
         reinterpret_cast<uintptr_t>(params.constraints.stack_limit());

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

@@ -35,6 +35,12 @@
 
 namespace v8 {
 
+// static
+std::unique_ptr<CppHeap> CppHeap::Create(v8::Platform* platform,
+                                         const CppHeapCreateParams& params) {
+  return std::make_unique<internal::CppHeap>(platform, params.custom_spaces);
+}
+
 cppgc::AllocationHandle& CppHeap::GetAllocationHandle() {
   return internal::CppHeap::From(this)->object_allocator();
 }
@@ -63,8 +69,7 @@ namespace {
 
 class CppgcPlatformAdapter final : public cppgc::Platform {
  public:
-  explicit CppgcPlatformAdapter(v8::Isolate* isolate)
-      : platform_(V8::GetCurrentPlatform()), isolate_(isolate) {}
+  explicit CppgcPlatformAdapter(v8::Platform* platform) : platform_(platform) {}
 
   CppgcPlatformAdapter(const CppgcPlatformAdapter&) = delete;
   CppgcPlatformAdapter& operator=(const CppgcPlatformAdapter&) = delete;
@@ -90,9 +95,11 @@ class CppgcPlatformAdapter final : public cppgc::Platform {
     return platform_->GetTracingController();
   }
 
+  void SetIsolate(v8::Isolate* isolate) { isolate_ = isolate; }
+
  private:
   v8::Platform* platform_;
-  v8::Isolate* isolate_;
+  v8::Isolate* isolate_ = nullptr;
 };
 
 class UnifiedHeapConcurrentMarker
@@ -169,38 +176,67 @@ void UnifiedHeapMarker::AddObject(void* object) {
 }  // namespace
 
 CppHeap::CppHeap(
-    v8::Isolate* isolate,
+    v8::Platform* platform,
     const std::vector<std::unique_ptr<cppgc::CustomSpaceBase>>& custom_spaces,
     std::unique_ptr<cppgc::internal::MetricRecorder> metric_recorder)
-    : cppgc::internal::HeapBase(std::make_shared<CppgcPlatformAdapter>(isolate),
-                                custom_spaces,
+    : cppgc::internal::HeapBase(
+          std::make_shared<CppgcPlatformAdapter>(platform), custom_spaces,
           cppgc::internal::HeapBase::StackSupport::
               kSupportsConservativeStackScan,
-                                std::move(metric_recorder)),
-      isolate_(*reinterpret_cast<Isolate*>(isolate)) {
-  if (isolate_.heap_profiler()) {
-    isolate_.heap_profiler()->AddBuildEmbedderGraphCallback(
-        &CppGraphBuilder::Run, this);
-  }
+          std::move(metric_recorder)) {
+  // Enter no GC scope. `AttachIsolate()` removes this and allows triggering
+  // garbage collections.
+  no_gc_scope_++;
   stats_collector()->RegisterObserver(this);
 }
 
 CppHeap::~CppHeap() {
-  if (isolate_.heap_profiler()) {
-    isolate_.heap_profiler()->RemoveBuildEmbedderGraphCallback(
-        &CppGraphBuilder::Run, this);
+  if (isolate_) {
+    isolate_->heap()->DetachCppHeap();
   }
 }
 
 void CppHeap::Terminate() {
-  FinalizeIncrementalGarbageCollectionIfNeeded(
-      cppgc::Heap::StackState::kNoHeapPointers);
-  // Any future garbage collections will ignore the V8->C++ references.
-  isolate()->SetEmbedderHeapTracer(nullptr);
+  // Must not be attached to a heap when invoking termination GCs.
+  CHECK(!isolate_);
   // Gracefully terminate the C++ heap invoking destructors.
   HeapBase::Terminate();
 }
 
+void CppHeap::AttachIsolate(Isolate* isolate) {
+  CHECK_NULL(isolate_);
+  isolate_ = isolate;
+  static_cast<CppgcPlatformAdapter*>(platform())
+      ->SetIsolate(reinterpret_cast<v8::Isolate*>(isolate_));
+  if (isolate_->heap_profiler()) {
+    isolate_->heap_profiler()->AddBuildEmbedderGraphCallback(
+        &CppGraphBuilder::Run, this);
+  }
+  isolate_->heap()->SetEmbedderHeapTracer(this);
+  no_gc_scope_--;
+}
+
+void CppHeap::DetachIsolate() {
+  // TODO(chromium:1056170): Investigate whether this can be enforced with a
+  // CHECK across all relevant embedders and setups.
+  if (!isolate_) return;
+
+  // Delegate to existing EmbedderHeapTracer API to finish any ongoing garbage
+  // collection.
+  FinalizeTracing();
+  sweeper_.FinishIfRunning();
+
+  if (isolate_->heap_profiler()) {
+    isolate_->heap_profiler()->RemoveBuildEmbedderGraphCallback(
+        &CppGraphBuilder::Run, this);
+  }
+  isolate_ = nullptr;
+  // Any future garbage collections will ignore the V8->C++ references.
+  isolate()->SetEmbedderHeapTracer(nullptr);
+  // Enter no GC scope.
+  no_gc_scope_++;
+}
+
 void CppHeap::RegisterV8References(
     const std::vector<std::pair<void*, void*> >& embedder_fields) {
   DCHECK(marker_);
@@ -231,7 +267,7 @@ void CppHeap::TracePrologue(TraceFlags flags) {
   }
   marker_ =
       cppgc::internal::MarkerFactory::CreateAndStartMarking<UnifiedHeapMarker>(
-          *isolate_.heap(), AsBase(), platform_.get(), marking_config);
+          *isolate_->heap(), AsBase(), platform_.get(), marking_config);
   marking_done_ = false;
 }
 

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

@@ -32,7 +32,7 @@ class V8_EXPORT_PRIVATE CppHeap final
   }
 
   CppHeap(
-      v8::Isolate* isolate,
+      v8::Platform* platform,
       const std::vector<std::unique_ptr<cppgc::CustomSpaceBase>>& custom_spaces,
       std::unique_ptr<cppgc::internal::MetricRecorder> metric_recorder =
           nullptr);
@@ -44,6 +44,9 @@ class V8_EXPORT_PRIVATE CppHeap final
   HeapBase& AsBase() { return *this; }
   const HeapBase& AsBase() const { return *this; }
 
+  void AttachIsolate(Isolate* isolate);
+  void DetachIsolate();
+
   void Terminate();
 
   // v8::EmbedderHeapTracer interface.
@@ -69,7 +72,7 @@ class V8_EXPORT_PRIVATE CppHeap final
 
   void ReportBufferedAllocationSizeIfPossible();
 
-  Isolate& isolate_;
+  Isolate* isolate_ = nullptr;
   bool marking_done_ = false;
 
   // Buffered allocated bytes. Reporting allocated bytes to V8 can trigger a GC

src/heap/cppgc/heap-base.cc

@@ -105,7 +105,6 @@ void HeapBase::AdvanceIncrementalGarbageCollectionOnAllocationIfNeeded() {
 
 void HeapBase::Terminate() {
   DCHECK(!IsMarking());
-  DCHECK(!in_no_gc_scope());
   CHECK(!in_disallow_gc_scope());
 
   sweeper().FinishIfRunning();

src/heap/heap.cc

@@ -4732,12 +4732,6 @@ void Heap::ConfigureHeap(const v8::ResourceConstraints& constraints) {
   configured_ = true;
 }
 
-void Heap::ConfigureCppHeap(std::shared_ptr<CppHeapCreateParams> params) {
-  cpp_heap_ = std::make_unique<CppHeap>(
-      reinterpret_cast<v8::Isolate*>(isolate()), params->custom_spaces);
-  SetEmbedderHeapTracer(CppHeap::From(cpp_heap_.get()));
-}
-
 void Heap::AddToRingBuffer(const char* string) {
   size_t first_part =
       std::min(strlen(string), kTraceRingBufferSize - ring_buffer_end_);
@@ -5379,6 +5373,8 @@ void Heap::NotifyOldGenerationExpansion(AllocationSpace space,
 
 void Heap::SetEmbedderHeapTracer(EmbedderHeapTracer* tracer) {
   DCHECK_EQ(gc_state(), HeapState::NOT_IN_GC);
+  // Setting a tracer is only supported when CppHeap is not used.
+  DCHECK_IMPLIES(tracer, !cpp_heap_);
   local_embedder_heap_tracer()->SetRemoteTracer(tracer);
 }
 
@@ -5386,6 +5382,16 @@ EmbedderHeapTracer* Heap::GetEmbedderHeapTracer() const {
   return local_embedder_heap_tracer()->remote_tracer();
 }
 
+void Heap::AttachCppHeap(v8::CppHeap* cpp_heap) {
+  CppHeap::From(cpp_heap)->AttachIsolate(isolate());
+  cpp_heap_ = cpp_heap;
+}
+
+void Heap::DetachCppHeap() {
+  CppHeap::From(cpp_heap_)->DetachIsolate();
+  cpp_heap_ = nullptr;
+}
+
 EmbedderHeapTracer::TraceFlags Heap::flags_for_embedder_tracer() const {
   if (is_current_gc_forced()) {
     return EmbedderHeapTracer::TraceFlags::kForced;
@@ -5512,7 +5518,10 @@ void Heap::TearDown() {
   dead_object_stats_.reset();
 
   local_embedder_heap_tracer_.reset();
-  cpp_heap_.reset();
+  if (cpp_heap_) {
+    CppHeap::From(cpp_heap_)->DetachIsolate();
+    cpp_heap_ = nullptr;
+  }
 
   external_string_table_.TearDown();
 

src/heap/heap.h

@@ -1137,10 +1137,10 @@ class Heap {
   // Unified heap (C++) support. ===============================================
   // ===========================================================================
 
-  V8_EXPORT_PRIVATE void ConfigureCppHeap(
-      std::shared_ptr<CppHeapCreateParams> params);
+  V8_EXPORT_PRIVATE void AttachCppHeap(v8::CppHeap* cpp_heap);
+  V8_EXPORT_PRIVATE void DetachCppHeap();
 
-  v8::CppHeap* cpp_heap() const { return cpp_heap_.get(); }
+  v8::CppHeap* cpp_heap() const { return cpp_heap_; }
 
   // ===========================================================================
   // External string table API. ================================================
@@ -2236,7 +2236,9 @@ class Heap {
   std::unique_ptr<AllocationObserver> stress_concurrent_allocation_observer_;
   std::unique_ptr<LocalEmbedderHeapTracer> local_embedder_heap_tracer_;
   std::unique_ptr<MarkingBarrier> marking_barrier_;
-  std::unique_ptr<v8::CppHeap> cpp_heap_;
+
+  // The embedder owns the C++ heap.
+  v8::CppHeap* cpp_heap_ = nullptr;
 
   StrongRootsEntry* strong_roots_head_ = nullptr;
   base::Mutex strong_roots_mutex_;

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

@@ -10,6 +10,7 @@
 #include "include/v8.h"
 #include "src/api/api-inl.h"
 #include "src/heap/cppgc-js/cpp-heap.h"
+#include "src/heap/cppgc/sweeper.h"
 #include "src/objects/objects-inl.h"
 #include "test/unittests/heap/heap-utils.h"
 #include "test/unittests/heap/unified-heap-utils.h"
@@ -39,6 +40,8 @@ class Wrappable final : public cppgc::GarbageCollected<Wrappable> {
 
 size_t Wrappable::destructor_callcount = 0;
 
+using UnifiedHeapDetachedTest = TestWithHeapInternals;
+
 }  // namespace
 
 TEST_F(UnifiedHeapTest, OnlyGC) { CollectGarbageWithEmbedderStack(); }
@@ -119,5 +122,29 @@ TEST_F(UnifiedHeapTest, WriteBarrierCppToV8Reference) {
             wrappable->wrapper()->GetAlignedPointerFromInternalField(1));
 }
 
+TEST_F(UnifiedHeapDetachedTest, AllocationBeforeConfigureHeap) {
+  auto heap =
+      v8::CppHeap::Create(V8::GetCurrentPlatform(), CppHeapCreateParams{});
+  auto* object =
+      cppgc::MakeGarbageCollected<Wrappable>(heap->GetAllocationHandle());
+  cppgc::WeakPersistent<Wrappable> weak_holder{object};
+
+  auto& js_heap = *isolate()->heap();
+  js_heap.AttachCppHeap(heap.get());
+  auto& cpp_heap = *CppHeap::From(isolate()->heap()->cpp_heap());
+  {
+    CollectGarbage(OLD_SPACE);
+    cpp_heap.AsBase().sweeper().FinishIfRunning();
+    EXPECT_TRUE(weak_holder);
+  }
+  {
+    js_heap.SetEmbedderStackStateForNextFinalization(
+        EmbedderHeapTracer::EmbedderStackState::kNoHeapPointers);
+    CollectGarbage(OLD_SPACE);
+    cpp_heap.AsBase().sweeper().FinishIfRunning();
+    EXPECT_FALSE(weak_holder);
+  }
+}
+
 }  // namespace internal
 }  // namespace v8

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

@@ -14,8 +14,10 @@
 namespace v8 {
 namespace internal {
 
-UnifiedHeapTest::UnifiedHeapTest() {
-  isolate()->heap()->ConfigureCppHeap(std::make_unique<CppHeapCreateParams>());
+UnifiedHeapTest::UnifiedHeapTest()
+    : cpp_heap_(v8::CppHeap::Create(V8::GetCurrentPlatform(),
+                                    CppHeapCreateParams{})) {
+  isolate()->heap()->AttachCppHeap(cpp_heap_.get());
 }
 
 void UnifiedHeapTest::CollectGarbageWithEmbedderStack(

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

@@ -10,6 +10,9 @@
 #include "test/unittests/heap/heap-utils.h"
 
 namespace v8 {
+
+class CppHeap;
+
 namespace internal {
 
 class CppHeap;
@@ -27,6 +30,9 @@ class UnifiedHeapTest : public TestWithHeapInternals {
 
   CppHeap& cpp_heap() const;
   cppgc::AllocationHandle& allocation_handle();
+
+ private:
+  std::unique_ptr<v8::CppHeap> cpp_heap_;
 };
 
 class WrapperHelper {