cppgc-js: Avoid using marked bytes deadline during the atomic pause

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

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

@@ -4,6 +4,8 @@
 
 #include "src/heap/cppgc-js/cpp-heap.h"
 
+#include <cstdint>
+
 #include "include/cppgc/heap-consistency.h"
 #include "include/cppgc/platform.h"
 #include "include/v8-platform.h"
@@ -280,12 +282,14 @@ bool CppHeap::AdvanceTracing(double deadline_in_ms) {
       AsBase(), in_atomic_pause_
                     ? cppgc::internal::StatsCollector::kAtomicMark
                     : cppgc::internal::StatsCollector::kIncrementalMark);
-  v8::base::TimeDelta deadline =
+  const v8::base::TimeDelta deadline =
       in_atomic_pause_ ? v8::base::TimeDelta::Max()
                        : v8::base::TimeDelta::FromMillisecondsD(deadline_in_ms);
+  const size_t marked_bytes_limit = in_atomic_pause_ ? SIZE_MAX : 0;
   // TODO(chromium:1056170): Replace when unified heap transitions to
   // bytes-based deadline.
-  marking_done_ = marker_->AdvanceMarkingWithMaxDuration(deadline);
+  marking_done_ =
+      marker_->AdvanceMarkingWithLimits(deadline, marked_bytes_limit);
   DCHECK_IMPLIES(in_atomic_pause_, marking_done_);
   return marking_done_;
 }

src/heap/cppgc/marker.cc

@@ -4,11 +4,13 @@
 
 #include "src/heap/cppgc/marker.h"
 
+#include <cstdint>
 #include <memory>
 
 #include "include/cppgc/heap-consistency.h"
 #include "include/cppgc/internal/process-heap.h"
 #include "include/cppgc/platform.h"
+#include "src/base/platform/time.h"
 #include "src/heap/cppgc/heap-object-header.h"
 #include "src/heap/cppgc/heap-page.h"
 #include "src/heap/cppgc/heap-visitor.h"
@@ -278,8 +280,7 @@ void MarkerBase::FinishMarking(MarkingConfig::StackState stack_state) {
   DCHECK(is_marking_started_);
   StatsCollector::EnabledScope stats_scope(heap(), StatsCollector::kAtomicMark);
   EnterAtomicPause(stack_state);
-  CHECK(ProcessWorklistsWithDeadline(std::numeric_limits<size_t>::max(),
-                                     v8::base::TimeTicks::Max()));
+  CHECK(AdvanceMarkingWithLimits(v8::base::TimeDelta::Max(), SIZE_MAX));
   mutator_marking_state_.Publish();
   LeaveAtomicPause();
 }
@@ -358,34 +359,34 @@ bool MarkerBase::IncrementalMarkingStep(MarkingConfig::StackState stack_state) {
   }
   config_.stack_state = stack_state;
 
-  return AdvanceMarkingWithDeadline();
+  return AdvanceMarkingWithLimits();
 }
 
 void MarkerBase::AdvanceMarkingOnAllocation() {
-  if (AdvanceMarkingWithDeadline()) {
+  if (AdvanceMarkingWithLimits()) {
     // Schedule another incremental task for finalizing without a stack.
     ScheduleIncrementalMarkingTask();
   }
 }
 
-bool MarkerBase::AdvanceMarkingWithMaxDuration(
-    v8::base::TimeDelta max_duration) {
-  return AdvanceMarkingWithDeadline(max_duration);
-}
-
-bool MarkerBase::AdvanceMarkingWithDeadline(v8::base::TimeDelta max_duration) {
+bool MarkerBase::AdvanceMarkingWithLimits(v8::base::TimeDelta max_duration,
+                                          size_t marked_bytes_limit) {
   bool is_done = false;
-  if (!incremental_marking_disabled_for_testing_) {
-    size_t step_size_in_bytes =
-        GetNextIncrementalStepDuration(schedule_, heap_);
+  if (!main_marking_disabled_for_testing_) {
+    const bool with_schedule = marked_bytes_limit == 0;
+    if (with_schedule) {
+      marked_bytes_limit = mutator_marking_state_.marked_bytes() +
+                           GetNextIncrementalStepDuration(schedule_, heap_);
+    }
     StatsCollector::EnabledScope deadline_scope(
         heap(), StatsCollector::kMarkTransitiveClosureWithDeadline,
         "deadline_ms", max_duration.InMillisecondsF());
     is_done = ProcessWorklistsWithDeadline(
-        mutator_marking_state_.marked_bytes() + step_size_in_bytes,
-        v8::base::TimeTicks::Now() + max_duration);
-    schedule_.UpdateIncrementalMarkedBytes(
-        mutator_marking_state_.marked_bytes());
+        marked_bytes_limit, v8::base::TimeTicks::Now() + max_duration);
+    if (with_schedule) {
+      schedule_.UpdateIncrementalMarkedBytes(
+          mutator_marking_state_.marked_bytes());
+    }
   }
   mutator_marking_state_.Publish();
   if (!is_done) {
@@ -515,8 +516,8 @@ void MarkerBase::ClearAllWorklistsForTesting() {
   if (compaction_worklists) compaction_worklists->ClearForTesting();
 }
 
-void MarkerBase::DisableIncrementalMarkingForTesting() {
-  incremental_marking_disabled_for_testing_ = true;
+void MarkerBase::SetMainThreadMarkingDisabledForTesting(bool value) {
+  main_marking_disabled_for_testing_ = value;
 }
 
 void MarkerBase::WaitForConcurrentMarkingForTesting() {

src/heap/cppgc/marker.h

@@ -30,9 +30,9 @@ class MarkerFactory;
 // phase:
 // 1. StartMarking() [Called implicitly when creating a Marker using
 //                    MarkerFactory]
-// 2. AdvanceMarkingWithDeadline() [Optional, depending on environment.]
+// 2. AdvanceMarkingWithLimits() [Optional, depending on environment.]
 // 3. EnterAtomicPause()
-// 4. AdvanceMarkingWithDeadline()
+// 4. AdvanceMarkingWithLimits()
 // 5. LeaveAtomicPause()
 //
 // Alternatively, FinishMarking combines steps 3.-5.
@@ -69,10 +69,14 @@ class V8_EXPORT_PRIVATE MarkerBase {
   // - Updates the MarkingConfig if the stack state has changed;
   void EnterAtomicPause(MarkingConfig::StackState);
 
-  // Makes marking progress.
+  // Makes marking progress.  A `marked_bytes_limit` of 0 means that the limit
+  // is determined by the internal marking scheduler.
+  //
   // TODO(chromium:1056170): Remove TimeDelta argument when unified heap no
   // longer uses it.
-  bool AdvanceMarkingWithMaxDuration(v8::base::TimeDelta);
+  bool AdvanceMarkingWithLimits(
+      v8::base::TimeDelta = kMaximumIncrementalStepDuration,
+      size_t marked_bytes_limit = 0);
 
   // Makes marking progress when allocation a new lab.
   void AdvanceMarkingOnAllocation();
@@ -83,7 +87,7 @@ class V8_EXPORT_PRIVATE MarkerBase {
 
   // Combines:
   // - EnterAtomicPause()
-  // - AdvanceMarkingWithDeadline()
+  // - AdvanceMarkingWithLimits()
   // - ProcessWeakness()
   // - LeaveAtomicPause()
   void FinishMarking(MarkingConfig::StackState);
@@ -121,7 +125,7 @@ class V8_EXPORT_PRIVATE MarkerBase {
     Handle handle_;
   };
 
-  void DisableIncrementalMarkingForTesting();
+  void SetMainThreadMarkingDisabledForTesting(bool);
 
   void WaitForConcurrentMarkingForTesting();
 
@@ -147,12 +151,6 @@ class V8_EXPORT_PRIVATE MarkerBase {
   virtual ConservativeTracingVisitor& conservative_visitor() = 0;
   virtual heap::base::StackVisitor& stack_visitor() = 0;
 
-  // Makes marking progress.
-  // TODO(chromium:1056170): Remove TimeDelta argument when unified heap no
-  // longer uses it.
-  bool AdvanceMarkingWithDeadline(
-      v8::base::TimeDelta = kMaximumIncrementalStepDuration);
-
   bool ProcessWorklistsWithDeadline(size_t, v8::base::TimeTicks);
 
   void VisitRoots(MarkingConfig::StackState);
@@ -178,7 +176,7 @@ class V8_EXPORT_PRIVATE MarkerBase {
 
   std::unique_ptr<ConcurrentMarkerBase> concurrent_marker_{nullptr};
 
-  bool incremental_marking_disabled_for_testing_{false};
+  bool main_marking_disabled_for_testing_{false};
 
   friend class MarkerFactory;
 };

test/unittests/heap/cppgc/concurrent-marking-unittest.cc

@@ -37,7 +37,7 @@ class ConcurrentMarkingTest : public testing::TestWithHeap {
     Heap* heap = Heap::From(GetHeap());
     heap->DisableHeapGrowingForTesting();
     heap->StartIncrementalGarbageCollection(ConcurrentPreciseConfig);
-    heap->marker()->DisableIncrementalMarkingForTesting();
+    heap->marker()->SetMainThreadMarkingDisabledForTesting(true);
   }
 
   bool SingleStep(Config::StackState stack_state) {
@@ -52,7 +52,9 @@ class ConcurrentMarkingTest : public testing::TestWithHeap {
   }
 
   void FinishGC() {
-    Heap::From(GetHeap())->FinalizeIncrementalGarbageCollectionIfRunning(
+    Heap* heap = Heap::From(GetHeap());
+    heap->marker()->SetMainThreadMarkingDisabledForTesting(false);
+    heap->FinalizeIncrementalGarbageCollectionIfRunning(
         ConcurrentPreciseConfig);
   }
 };