cppgc: Add marked bytes deadline

This CL adds a bytes based deadline to draining of worklist.
The time based deadline is also kept because:
1) Unified heap can't transition to bytes-based deadlines yet.
2) Unified heap with concurrent marking needs to flush v8 references
   which don't count as marked_bytes and can cause very long incremental
   pauses.

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

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

@@ -135,7 +135,10 @@ void CppHeap::TracePrologue(TraceFlags flags) {
 }
 
 bool CppHeap::AdvanceTracing(double deadline_in_ms) {
+  // TODO(chromium:1056170): Replace std::numeric_limits<size_t>::max() with a
+  // proper deadline when unified heap transitions to bytes-based deadline.
   marking_done_ = marker_->AdvanceMarkingWithDeadline(
+      std::numeric_limits<size_t>::max(),
       v8::base::TimeDelta::FromMillisecondsD(deadline_in_ms));
   return marking_done_;
 }

src/heap/cppgc/marker.cc

@@ -83,26 +83,42 @@ void ResetRememberedSet(HeapBase& heap) {
 #endif
 }
 
-template <typename Worklist, typename Callback>
-bool DrainWorklistWithDeadline(v8::base::TimeTicks deadline, Worklist* worklist,
-                               Callback callback, int task_id) {
-  const size_t kDeadlineCheckInterval = 1250;
+static constexpr size_t kDefaultDeadlineCheckInterval = 150u;
 
+template <size_t kDeadlineCheckInterval = kDefaultDeadlineCheckInterval,
+          typename Worklist, typename Callback, typename Predicate>
+bool DrainWorklistWithDeadline(Predicate should_yield, Worklist* worklist,
+                               Callback callback, int task_id) {
   size_t processed_callback_count = 0;
   typename Worklist::View view(worklist, task_id);
   typename Worklist::EntryType item;
   while (view.Pop(&item)) {
     callback(item);
-    if (++processed_callback_count == kDeadlineCheckInterval) {
-      if (deadline <= v8::base::TimeTicks::Now()) {
+    if (processed_callback_count-- == 0) {
+      if (should_yield()) {
         return false;
       }
-      processed_callback_count = 0;
+      processed_callback_count = kDeadlineCheckInterval;
     }
   }
   return true;
 }
 
+template <size_t kDeadlineCheckInterval = kDefaultDeadlineCheckInterval,
+          typename Worklist, typename Callback>
+bool DrainWorklistWithBytesAndTimeDeadline(MarkingState& marking_state,
+                                           size_t marked_bytes_deadline,
+                                           v8::base::TimeTicks time_deadline,
+                                           Worklist* worklist,
+                                           Callback callback, int task_id) {
+  return DrainWorklistWithDeadline(
+      [&marking_state, marked_bytes_deadline, time_deadline]() {
+        return (marked_bytes_deadline <= marking_state.marked_bytes()) ||
+               (time_deadline <= v8::base::TimeTicks::Now());
+      },
+      worklist, callback, task_id);
+}
+
 void TraceMarkedObject(Visitor* visitor, const HeapObjectHeader* header) {
   DCHECK(header);
   DCHECK(!header->IsInConstruction<HeapObjectHeader::AccessMode::kNonAtomic>());
@@ -114,6 +130,8 @@ void TraceMarkedObject(Visitor* visitor, const HeapObjectHeader* header) {
 
 }  // namespace
 
+constexpr v8::base::TimeDelta MarkerBase::kMaximumIncrementalStepDuration;
+
 MarkerBase::IncrementalMarkingTask::IncrementalMarkingTask(MarkerBase* marker)
     : marker_(marker), handle_(Handle::NonEmptyTag{}) {}
 
@@ -130,10 +148,11 @@ MarkerBase::IncrementalMarkingTask::Post(v8::TaskRunner* runner,
 void MarkerBase::IncrementalMarkingTask::Run() {
   if (handle_.IsCanceled()) return;
 
-  // TODO(chromium:1056170): Replace hardcoded duration with schedule.
+  // TODO(chromium:1056170): Replace hardcoded expected marked bytes with
+  // schedule.
   if (marker_->IncrementalMarkingStep(
           MarkingConfig::StackState::kNoHeapPointers,
-          v8::base::TimeDelta::FromMillisecondsD(2))) {
+          kMinimumMarkedBytesPerIncrementalStep)) {
     // Incremental marking is done so should finalize GC.
     marker_->heap().FinalizeIncrementalGarbageCollectionIfNeeded(
         MarkingConfig::StackState::kNoHeapPointers);
@@ -211,7 +230,8 @@ void MarkerBase::LeaveAtomicPause() {
 void MarkerBase::FinishMarking(MarkingConfig::StackState stack_state) {
   DCHECK(is_marking_started_);
   EnterAtomicPause(stack_state);
-  ProcessWorklistsWithDeadline(v8::base::TimeDelta::Max());
+  ProcessWorklistsWithDeadline(std::numeric_limits<size_t>::max(),
+                               v8::base::TimeDelta::Max());
   LeaveAtomicPause();
   is_marking_started_ = false;
 }
@@ -247,29 +267,31 @@ void MarkerBase::VisitRoots(MarkingConfig::StackState stack_state) {
 }
 
 void MarkerBase::ScheduleIncrementalMarkingTask() {
-  if (!platform_ || !foreground_task_runner_) return;
-  DCHECK(!incremental_marking_handle_);
+  if (!platform_ || !foreground_task_runner_ || incremental_marking_handle_)
+    return;
   incremental_marking_handle_ =
       IncrementalMarkingTask::Post(foreground_task_runner_.get(), this);
 }
 
 bool MarkerBase::IncrementalMarkingStepForTesting(
-    MarkingConfig::StackState stack_state, v8::base::TimeDelta deadline) {
-  return IncrementalMarkingStep(stack_state, deadline);
+    MarkingConfig::StackState stack_state, size_t expected_marked_bytes) {
+  return IncrementalMarkingStep(stack_state, expected_marked_bytes);
 }
 
 bool MarkerBase::IncrementalMarkingStep(MarkingConfig::StackState stack_state,
-                                        v8::base::TimeDelta duration) {
+                                        size_t expected_marked_bytes) {
   if (stack_state == MarkingConfig::StackState::kNoHeapPointers) {
     marking_worklists_.FlushNotFullyConstructedObjects();
   }
   config_.stack_state = stack_state;
 
-  return AdvanceMarkingWithDeadline(duration);
+  return AdvanceMarkingWithDeadline(expected_marked_bytes);
 }
 
-bool MarkerBase::AdvanceMarkingWithDeadline(v8::base::TimeDelta duration) {
-  bool is_done = ProcessWorklistsWithDeadline(duration);
+bool MarkerBase::AdvanceMarkingWithDeadline(size_t expected_marked_bytes,
+                                            v8::base::TimeDelta max_duration) {
+  bool is_done =
+      ProcessWorklistsWithDeadline(expected_marked_bytes, max_duration);
   if (!is_done) {
     // If marking is atomic, |is_done| should always be true.
     DCHECK_NE(MarkingConfig::MarkingType::kAtomic, config_.marking_type);
@@ -278,15 +300,18 @@ bool MarkerBase::AdvanceMarkingWithDeadline(v8::base::TimeDelta duration) {
   return is_done;
 }
 
-bool MarkerBase::ProcessWorklistsWithDeadline(v8::base::TimeDelta duration) {
-  v8::base::TimeTicks deadline = v8::base::TimeTicks::Now() + duration;
+bool MarkerBase::ProcessWorklistsWithDeadline(
+    size_t expected_marked_bytes, v8::base::TimeDelta max_duration) {
+  size_t marked_bytes_deadline =
+      mutator_marking_state_.marked_bytes() + expected_marked_bytes;
+  v8::base::TimeTicks time_deadline = v8::base::TimeTicks::Now() + max_duration;
 
   do {
     // Convert |previously_not_fully_constructed_worklist_| to
     // |marking_worklist_|. This merely re-adds items with the proper
     // callbacks.
-    if (!DrainWorklistWithDeadline(
-            deadline,
+    if (!DrainWorklistWithBytesAndTimeDeadline(
+            mutator_marking_state_, marked_bytes_deadline, time_deadline,
             marking_worklists_.previously_not_fully_constructed_worklist(),
             [this](HeapObjectHeader* header) {
               TraceMarkedObject(&visitor(), header);
@@ -295,8 +320,9 @@ bool MarkerBase::ProcessWorklistsWithDeadline(v8::base::TimeDelta duration) {
             MarkingWorklists::kMutatorThreadId))
       return false;
 
-    if (!DrainWorklistWithDeadline(
-            deadline, marking_worklists_.marking_worklist(),
+    if (!DrainWorklistWithBytesAndTimeDeadline(
+            mutator_marking_state_, marked_bytes_deadline, time_deadline,
+            marking_worklists_.marking_worklist(),
             [this](const MarkingWorklists::MarkingItem& item) {
               const HeapObjectHeader& header =
                   HeapObjectHeader::FromPayload(item.base_object_payload);
@@ -310,8 +336,9 @@ bool MarkerBase::ProcessWorklistsWithDeadline(v8::base::TimeDelta duration) {
             MarkingWorklists::kMutatorThreadId))
       return false;
 
-    if (!DrainWorklistWithDeadline(
-            deadline, marking_worklists_.write_barrier_worklist(),
+    if (!DrainWorklistWithBytesAndTimeDeadline(
+            mutator_marking_state_, marked_bytes_deadline, time_deadline,
+            marking_worklists_.write_barrier_worklist(),
             [this](HeapObjectHeader* header) {
               TraceMarkedObject(&visitor(), header);
               mutator_marking_state_.AccountMarkedBytes(*header);

src/heap/cppgc/marker.h

@@ -69,7 +69,10 @@ class V8_EXPORT_PRIVATE MarkerBase {
   void EnterAtomicPause(MarkingConfig::StackState);
 
   // Makes marking progress.
-  bool AdvanceMarkingWithDeadline(v8::base::TimeDelta);
+  // TODO(chromium:1056170): Remove TimeDelta argument when unified heap no
+  // longer uses it.
+  bool AdvanceMarkingWithDeadline(
+      size_t, v8::base::TimeDelta = kMaximumIncrementalStepDuration);
 
   // Signals leaving the atomic marking pause. This method expects no more
   // objects to be marked and merely updates marking states if needed.
@@ -93,8 +96,7 @@ class V8_EXPORT_PRIVATE MarkerBase {
   cppgc::Visitor& VisitorForTesting() { return visitor(); }
   void ClearAllWorklistsForTesting();
 
-  bool IncrementalMarkingStepForTesting(MarkingConfig::StackState,
-                                        v8::base::TimeDelta);
+  bool IncrementalMarkingStepForTesting(MarkingConfig::StackState, size_t);
 
   class IncrementalMarkingTask final : public v8::Task {
    public:
@@ -113,13 +115,17 @@ class V8_EXPORT_PRIVATE MarkerBase {
   };
 
  protected:
+  static constexpr v8::base::TimeDelta kMaximumIncrementalStepDuration =
+      v8::base::TimeDelta::FromMilliseconds(2);
+  static constexpr size_t kMinimumMarkedBytesPerIncrementalStep = 64 * kKB;
+
   MarkerBase(HeapBase&, cppgc::Platform*, MarkingConfig);
 
   virtual cppgc::Visitor& visitor() = 0;
   virtual ConservativeTracingVisitor& conservative_visitor() = 0;
   virtual heap::base::StackVisitor& stack_visitor() = 0;
 
-  bool ProcessWorklistsWithDeadline(v8::base::TimeDelta);
+  bool ProcessWorklistsWithDeadline(size_t, v8::base::TimeDelta);
 
   void VisitRoots(MarkingConfig::StackState);
 
@@ -127,7 +133,7 @@ class V8_EXPORT_PRIVATE MarkerBase {
 
   void ScheduleIncrementalMarkingTask();
 
-  bool IncrementalMarkingStep(MarkingConfig::StackState, v8::base::TimeDelta);
+  bool IncrementalMarkingStep(MarkingConfig::StackState, size_t);
 
   HeapBase& heap_;
   MarkingConfig config_ = MarkingConfig::Default();

src/heap/incremental-marking.cc

@@ -1082,6 +1082,9 @@ StepResult IncrementalMarking::Step(double max_step_size_in_ms,
       embedder_deadline =
           Min(max_step_size_in_ms,
               static_cast<double>(bytes_to_process) / marking_speed);
+      // TODO(chromium:1056170): Replace embedder_deadline with bytes_to_process
+      // after migrating blink to the cppgc library and after v8 can directly
+      // push objects to Oilpan.
       embedder_result = EmbedderStep(embedder_deadline, &embedder_duration);
     }
     bytes_marked_ += v8_bytes_processed;

test/unittests/heap/cppgc/marker-unittest.cc

@@ -279,7 +279,7 @@ class IncrementalMarkingTest : public testing::TestWithHeap {
       MarkingConfig::MarkingType::kIncremental};
 
   void FinishSteps(Marker& marker, MarkingConfig::StackState stack_state) {
-    SingleStep(marker, stack_state, v8::base::TimeDelta::Max());
+    SingleStep(marker, stack_state, std::numeric_limits<size_t>::max());
   }
 
   void FinishMarking(Marker& marker) {
@@ -292,8 +292,8 @@ class IncrementalMarkingTest : public testing::TestWithHeap {
 
  private:
   bool SingleStep(Marker& marker, MarkingConfig::StackState stack_state,
-                  v8::base::TimeDelta deadline) {
-    return marker.IncrementalMarkingStepForTesting(stack_state, deadline);
+                  size_t bytes_to_mark) {
+    return marker.IncrementalMarkingStepForTesting(stack_state, bytes_to_mark);
   }
 };
 constexpr IncrementalMarkingTest::MarkingConfig