cppgc: Join concurrent marking instead of cancelling

Join instead of cancel to make use of the the main thread.

Also make the Join() call explicit instead of implicitly finishing
concurrency on advancing tracing form the main thread.

Bug: v8:12600
Change-Id: I60d3e82bfc2e8a3ccc2dda761a5d3eb3ac7694d1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3578855Reviewed-by: Omer Katz <omerkatz@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79910}

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

@@ -607,6 +607,10 @@ void CppHeap::EnterFinalPause(cppgc::EmbedderStackState stack_state) {
   compactor_.CancelIfShouldNotCompact(MarkingType::kAtomic, stack_state);
 }
 
+bool CppHeap::FinishConcurrentMarkingIfNeeded() {
+  return marker_->JoinConcurrentMarkingIfNeeded();
+}
+
 void CppHeap::TraceEpilogue() {
   CHECK(in_atomic_pause_);
   CHECK(marking_done_);
@@ -738,7 +742,10 @@ void CppHeap::CollectGarbageForTesting(CollectionType collection_type,
       StartTracing();
     }
     EnterFinalPause(stack_state);
-    AdvanceTracing(std::numeric_limits<double>::infinity());
+    CHECK(AdvanceTracing(std::numeric_limits<double>::infinity()));
+    if (FinishConcurrentMarkingIfNeeded()) {
+      CHECK(AdvanceTracing(std::numeric_limits<double>::infinity()));
+    }
     TraceEpilogue();
   }
 }

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

@@ -134,6 +134,7 @@ class V8_EXPORT_PRIVATE CppHeap final
   bool IsTracingDone();
   void TraceEpilogue();
   void EnterFinalPause(cppgc::EmbedderStackState stack_state);
+  bool FinishConcurrentMarkingIfNeeded();
 
   void RunMinorGC(StackState);
 

src/heap/cppgc/concurrent-marker.cc

@@ -203,17 +203,12 @@ void ConcurrentMarkerBase::Start() {
                          std::make_unique<ConcurrentMarkingTask>(*this));
 }
 
-bool ConcurrentMarkerBase::Cancel() {
+bool ConcurrentMarkerBase::Join() {
   if (!concurrent_marking_handle_ || !concurrent_marking_handle_->IsValid())
     return false;
 
-  concurrent_marking_handle_->Cancel();
-  return true;
-}
-
-void ConcurrentMarkerBase::JoinForTesting() {
-  if (concurrent_marking_handle_ && concurrent_marking_handle_->IsValid())
   concurrent_marking_handle_->Join();
+  return true;
 }
 
 bool ConcurrentMarkerBase::IsActive() const {

src/heap/cppgc/concurrent-marker.h

@@ -24,10 +24,8 @@ class V8_EXPORT_PRIVATE ConcurrentMarkerBase {
   ConcurrentMarkerBase& operator=(const ConcurrentMarkerBase&) = delete;
 
   void Start();
-  // Returns whether the job has been cancelled.
-  bool Cancel();
-
-  void JoinForTesting();
+  // Returns whether the job has been joined.
+  bool Join();
 
   void NotifyIncrementalMutatorStepCompleted();
 

src/heap/cppgc/marker.cc

@@ -317,6 +317,9 @@ void MarkerBase::FinishMarking(MarkingConfig::StackState stack_state) {
     StatsCollector::EnabledScope stats_scope(heap().stats_collector(),
                                              StatsCollector::kAtomicMark);
     CHECK(AdvanceMarkingWithLimits(v8::base::TimeDelta::Max(), SIZE_MAX));
+    if (JoinConcurrentMarkingIfNeeded()) {
+      CHECK(AdvanceMarkingWithLimits(v8::base::TimeDelta::Max(), SIZE_MAX));
+    }
     mutator_marking_state_.Publish();
   }
   LeaveAtomicPause();
@@ -447,9 +450,9 @@ void MarkerBase::AdvanceMarkingOnAllocation() {
   }
 }
 
-bool MarkerBase::CancelConcurrentMarkingIfNeeded() {
+bool MarkerBase::JoinConcurrentMarkingIfNeeded() {
   if (config_.marking_type != MarkingConfig::MarkingType::kAtomic ||
-      !concurrent_marker_->Cancel())
+      !concurrent_marker_->Join())
     return false;
 
   // Concurrent markers may have pushed some "leftover" in-construction objects
@@ -478,9 +481,6 @@ bool MarkerBase::AdvanceMarkingWithLimits(v8::base::TimeDelta max_duration,
       // adjustment.
       is_done = ProcessWorklistsWithDeadline(marked_bytes_limit, deadline);
     }
-    if (is_done && CancelConcurrentMarkingIfNeeded()) {
-      is_done = ProcessWorklistsWithDeadline(marked_bytes_limit, deadline);
-    }
     schedule_.UpdateMutatorThreadMarkedBytes(
         mutator_marking_state_.marked_bytes());
   }
@@ -637,7 +637,7 @@ void MarkerBase::SetMainThreadMarkingDisabledForTesting(bool value) {
 }
 
 void MarkerBase::WaitForConcurrentMarkingForTesting() {
-  concurrent_marker_->JoinForTesting();
+  concurrent_marker_->Join();
 }
 
 Marker::Marker(HeapBase& heap, cppgc::Platform* platform, MarkingConfig config)

src/heap/cppgc/marker.h

@@ -100,6 +100,8 @@ class V8_EXPORT_PRIVATE MarkerBase {
 
   void ProcessWeakness();
 
+  bool JoinConcurrentMarkingIfNeeded();
+
   inline void WriteBarrierForInConstructionObject(HeapObjectHeader&);
 
   template <WriteBarrierType type>
@@ -149,8 +151,6 @@ class V8_EXPORT_PRIVATE MarkerBase {
 
   void AdvanceMarkingOnAllocation();
 
-  bool CancelConcurrentMarkingIfNeeded();
-
   void HandleNotFullyConstructedObjects();
 
   HeapBase& heap_;

src/heap/mark-compact.cc

@@ -1021,6 +1021,9 @@ void MarkCompactCollector::FinishConcurrentMarking() {
         non_atomic_marking_state());
     heap()->concurrent_marking()->FlushNativeContexts(&native_context_stats_);
   }
+  if (auto* cpp_heap = CppHeap::From(heap_->cpp_heap())) {
+    cpp_heap->FinishConcurrentMarkingIfNeeded();
+  }
 }
 
 void MarkCompactCollector::VerifyMarking() {