[heap] Remove CompletionAction and infer action from StepOrigin

StepOrigin is enough to infer the right completion action: Either
finalization by task (for StepOrigin::kTask) or stack guard
(for StepOrigin::kV8).

Only tests with StepOrigin::kV8 were violating this but they also just
pass when enabling the stack guard.

Bug: v8:12775
Change-Id: I5df50198d8e3612ee97142f84bd497820a5cec78
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3816664Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82294}

src/heap/heap.cc

@@ -1723,9 +1723,7 @@ void Heap::ReportExternalMemoryPressure() {
     // Extend the gc callback flags with external memory flags.
     current_gc_callback_flags_ = static_cast<GCCallbackFlags>(
         current_gc_callback_flags_ | kGCCallbackFlagsForExternalMemory);
-    incremental_marking()->AdvanceWithDeadline(
-        deadline, IncrementalMarking::CompletionAction::kGcViaStackGuard,
-        StepOrigin::kV8);
+    incremental_marking()->AdvanceWithDeadline(deadline, StepOrigin::kV8);
   }
 }
 
@@ -4021,9 +4019,8 @@ bool Heap::PerformIdleTimeAction(GCIdleTimeAction action,
       result = true;
       break;
     case GCIdleTimeAction::kIncrementalStep: {
-      incremental_marking()->AdvanceWithDeadline(
-          deadline_in_ms, IncrementalMarking::CompletionAction::kGCViaTask,
-          StepOrigin::kTask);
+      incremental_marking()->AdvanceWithDeadline(deadline_in_ms,
+                                                 StepOrigin::kTask);
       FinalizeIncrementalMarkingIfComplete(
           GarbageCollectionReason::kFinalizeMarkingViaTask);
       result = incremental_marking()->IsStopped();

src/heap/incremental-marking-job.cc

@@ -86,8 +86,7 @@ StepResult IncrementalMarkingJob::Task::Step(Heap* heap) {
   double deadline =
       heap->MonotonicallyIncreasingTimeInMs() + kIncrementalMarkingDelayMs;
   StepResult result = heap->incremental_marking()->AdvanceWithDeadline(
-      deadline, i::IncrementalMarking::CompletionAction::kGCViaTask,
-      i::StepOrigin::kTask);
+      deadline, i::StepOrigin::kTask);
   heap->FinalizeIncrementalMarkingIfComplete(
       GarbageCollectionReason::kFinalizeMarkingViaTask);
   return result;

src/heap/incremental-marking.cc

@@ -538,15 +538,15 @@ double IncrementalMarking::CurrentTimeToMarkingTask() const {
   return std::max(recorded_time_to_marking_task, current_time_to_marking_task);
 }
 
-void IncrementalMarking::MarkingComplete(CompletionAction action) {
-  // Allowed overshoot percentage of incremental marking walltime.
-  constexpr double kAllowedOvershoot = 0.1;
-  // Minimum overshoot in ms. This is used to allow moving away from stack when
-  // marking was fast.
-  constexpr double kMinOvershootMs = 50;
-
-  if (action == CompletionAction::kGcViaStackGuard) {
+void IncrementalMarking::TryMarkingComplete(StepOrigin step_origin) {
+  if (step_origin == StepOrigin::kV8) {
     if (time_to_force_completion_ == 0.0) {
+      // Allowed overshoot percentage of incremental marking walltime.
+      constexpr double kAllowedOvershoot = 0.1;
+      // Minimum overshoot in ms. This is used to allow moving away from stack
+      // when marking was fast.
+      constexpr double kMinOvershootMs = 50;
+
       const double now = heap_->MonotonicallyIncreasingTimeInMs();
       const double overshoot_ms =
           std::max(kMinOvershootMs, (now - start_time_ms_) * kAllowedOvershoot);
@@ -594,7 +594,7 @@ void IncrementalMarking::MarkingComplete(CompletionAction action) {
         "[IncrementalMarking] Complete (normal).\n");
   }
 
-  if (action == CompletionAction::kGcViaStackGuard) {
+  if (step_origin == StepOrigin::kV8) {
     collection_requested_ = true;
     heap_->isolate()->stack_guard()->RequestGC();
   }
@@ -655,9 +655,8 @@ StepResult CombineStepResults(StepResult a, StepResult b) {
 }
 }  // anonymous namespace
 
-StepResult IncrementalMarking::AdvanceWithDeadline(
-    double deadline_in_ms, CompletionAction completion_action,
-    StepOrigin step_origin) {
+StepResult IncrementalMarking::AdvanceWithDeadline(double deadline_in_ms,
+                                                   StepOrigin step_origin) {
   NestedTimedHistogramScope incremental_marking_scope(
       heap_->isolate()->counters()->gc_incremental_marking());
   TRACE_EVENT1("v8", "V8.GCIncrementalMarking", "epoch",
@@ -668,7 +667,7 @@ StepResult IncrementalMarking::AdvanceWithDeadline(
 
   ScheduleBytesToMarkBasedOnTime(heap()->MonotonicallyIncreasingTimeInMs());
   FastForwardScheduleIfCloseToFinalization();
-  return Step(kStepSizeInMs, completion_action, step_origin);
+  return Step(kStepSizeInMs, step_origin);
 }
 
 size_t IncrementalMarking::StepSizeToKeepUpWithAllocations() {
@@ -770,11 +769,10 @@ void IncrementalMarking::AdvanceOnAllocation() {
   TRACE_GC_EPOCH(heap_->tracer(), GCTracer::Scope::MC_INCREMENTAL,
                  ThreadKind::kMain);
   ScheduleBytesToMarkBasedOnAllocation();
-  Step(kMaxStepSizeInMs, CompletionAction::kGcViaStackGuard, StepOrigin::kV8);
+  Step(kMaxStepSizeInMs, StepOrigin::kV8);
 }
 
 StepResult IncrementalMarking::Step(double max_step_size_in_ms,
-                                    CompletionAction action,
                                     StepOrigin step_origin) {
   double start = heap_->MonotonicallyIncreasingTimeInMs();
 
@@ -842,7 +840,7 @@ StepResult IncrementalMarking::Step(double max_step_size_in_ms,
         // TODO(v8:12775): Try to remove.
         FastForwardSchedule();
       }
-      MarkingComplete(action);
+      TryMarkingComplete(step_origin);
       combined_result = StepResult::kWaitingForFinalization;
     }
     if (FLAG_concurrent_marking) {

src/heap/incremental-marking.h

@@ -21,7 +21,20 @@ class Map;
 class Object;
 class PagedSpace;
 
-enum class StepOrigin { kV8, kTask };
+// Describes in which context IncrementalMarking::Step() is used in. This
+// information is used when marking finishes and for marking progress
+// heuristics.
+enum class StepOrigin {
+  // The caller of Step() is not allowed to complete marking right away. A task
+  // is scheduled to complete the GC. When the task isn't
+  // run soon enough, the stack guard mechanism will be used.
+  kV8,
+
+  // The caller of Step() will complete marking by running the GC right
+  // afterwards.
+  kTask
+};
+
 enum class StepResult {
   kNoImmediateWork,
   kMoreWorkRemaining,
@@ -32,13 +45,6 @@ class V8_EXPORT_PRIVATE IncrementalMarking final {
  public:
   enum State : uint8_t { STOPPED, MARKING, COMPLETE };
 
-  // How to complete a GC when invoking a step.
-  // - kGCViaTask: No action to finish the GC synchronously is performed.
-  //   Instead, a task to finish the GC is scheduled.
-  // - kGcViaStackGuard: Upon determining that there's no more work to do, a GC
-  //   is triggered via stack guard.
-  enum class CompletionAction { kGcViaStackGuard, kGCViaTask };
-
   class V8_NODISCARD PauseBlackAllocationScope {
    public:
     explicit PauseBlackAllocationScope(IncrementalMarking* marking)
@@ -114,12 +120,9 @@ class V8_EXPORT_PRIVATE IncrementalMarking final {
   // Performs incremental marking steps and returns before the deadline_in_ms is
   // reached. It may return earlier if the marker is already ahead of the
   // marking schedule, which is indicated with StepResult::kDone.
-  StepResult AdvanceWithDeadline(double deadline_in_ms,
-                                 CompletionAction completion_action,
-                                 StepOrigin step_origin);
+  StepResult AdvanceWithDeadline(double deadline_in_ms, StepOrigin step_origin);
 
-  StepResult Step(double max_step_size_in_ms, CompletionAction action,
-                  StepOrigin step_origin);
+  StepResult Step(double max_step_size_in_ms, StepOrigin step_origin);
 
   // This function is used to color the object black before it undergoes an
   // unsafe layout change. This is a part of synchronization protocol with
@@ -206,7 +209,7 @@ class V8_EXPORT_PRIVATE IncrementalMarking final {
   // bytes and already marked bytes.
   size_t ComputeStepSizeInBytes(StepOrigin step_origin);
 
-  void MarkingComplete(CompletionAction action);
+  void TryMarkingComplete(StepOrigin step_origin);
   void MarkRoots();
 
   void AdvanceOnAllocation();

src/heap/memory-reducer.cc

@@ -87,9 +87,8 @@ void MemoryReducer::NotifyTimer(const Event& event) {
       const int kIncrementalMarkingDelayMs = 500;
       double deadline = heap()->MonotonicallyIncreasingTimeInMs() +
                         kIncrementalMarkingDelayMs;
-      heap()->incremental_marking()->AdvanceWithDeadline(
-          deadline, IncrementalMarking::CompletionAction::kGCViaTask,
-          StepOrigin::kTask);
+      heap()->incremental_marking()->AdvanceWithDeadline(deadline,
+                                                         StepOrigin::kTask);
       heap()->FinalizeIncrementalMarkingIfComplete(
           GarbageCollectionReason::kFinalizeMarkingViaTask);
     }

test/cctest/heap/heap-utils.cc

@@ -200,9 +200,7 @@ void SimulateIncrementalMarking(i::Heap* heap, bool force_completion) {
   marking->MarkRootsForTesting();
 
   while (!marking->IsComplete()) {
-    marking->Step(kStepSizeInMs,
-                  i::IncrementalMarking::CompletionAction::kGCViaTask,
-                  i::StepOrigin::kV8);
+    marking->Step(kStepSizeInMs, i::StepOrigin::kV8);
   }
   CHECK(marking->IsComplete());
 }

test/cctest/heap/test-heap.cc

@@ -2563,9 +2563,7 @@ TEST(InstanceOfStubWriteBarrier) {
   while (!marking_state->IsBlack(f->code()) && !marking->IsStopped()) {
     // Discard any pending GC requests otherwise we will get GC when we enter
     // code below.
-    marking->Step(kStepSizeInMs,
-                  IncrementalMarking::CompletionAction::kGCViaTask,
-                  StepOrigin::kV8);
+    marking->Step(kStepSizeInMs, StepOrigin::kV8);
   }
 
   CHECK(marking->IsMarking());
@@ -2659,9 +2657,7 @@ TEST(IdleNotificationFinishMarking) {
 
   const double kStepSizeInMs = 100;
   do {
-    marking->Step(kStepSizeInMs,
-                  IncrementalMarking::CompletionAction::kGCViaTask,
-                  StepOrigin::kV8);
+    marking->Step(kStepSizeInMs, StepOrigin::kV8);
   } while (!CcTest::heap()
                 ->mark_compact_collector()
                 ->local_marking_worklists()
@@ -5902,9 +5898,7 @@ TEST(Regress598319) {
   // only partially marked the large object.
   const double kSmallStepSizeInMs = 0.1;
   while (!marking->IsComplete()) {
-    marking->Step(kSmallStepSizeInMs,
-                  i::IncrementalMarking::CompletionAction::kGCViaTask,
-                  StepOrigin::kV8);
+    marking->Step(kSmallStepSizeInMs, StepOrigin::kV8);
     ProgressBar& progress_bar = page->ProgressBar();
     if (progress_bar.IsEnabled() && progress_bar.Value() > 0) {
       CHECK_NE(progress_bar.Value(), arr.get().Size());
@@ -5925,9 +5919,7 @@ TEST(Regress598319) {
 
   // Finish marking with bigger steps to speed up test.
   const double kLargeStepSizeInMs = 1000;
-  while (marking->Step(kLargeStepSizeInMs,
-                       i::IncrementalMarking::CompletionAction::kGCViaTask,
-                       StepOrigin::kV8) !=
+  while (marking->Step(kLargeStepSizeInMs, StepOrigin::kV8) !=
          StepResult::kWaitingForFinalization) {
   }
   CHECK(marking->IsComplete());
@@ -6016,9 +6008,7 @@ TEST(Regress615489) {
   }
   const double kStepSizeInMs = 100;
   while (!marking->IsComplete()) {
-    marking->Step(kStepSizeInMs,
-                  i::IncrementalMarking::CompletionAction::kGCViaTask,
-                  StepOrigin::kV8);
+    marking->Step(kStepSizeInMs, StepOrigin::kV8);
   }
   CHECK(marking->IsComplete());
   intptr_t size_before = heap->SizeOfObjects();
@@ -6076,9 +6066,7 @@ TEST(Regress631969) {
   const double kStepSizeInMs = 100;
   IncrementalMarking* marking = heap->incremental_marking();
   while (!marking->IsComplete()) {
-    marking->Step(kStepSizeInMs,
-                  i::IncrementalMarking::CompletionAction::kGCViaTask,
-                  StepOrigin::kV8);
+    marking->Step(kStepSizeInMs, StepOrigin::kV8);
   }
 
   {
@@ -6687,9 +6675,7 @@ HEAP_TEST(Regress670675) {
     }
     if (marking->IsStopped()) break;
     double deadline = heap->MonotonicallyIncreasingTimeInMs() + 1;
-    marking->AdvanceWithDeadline(
-        deadline, IncrementalMarking::CompletionAction::kGcViaStackGuard,
-        StepOrigin::kV8);
+    marking->AdvanceWithDeadline(deadline, StepOrigin::kV8);
   }
   DCHECK(marking->IsStopped());
 }

test/unittests/heap/heap-utils.cc

@@ -31,9 +31,7 @@ void HeapInternalsBase::SimulateIncrementalMarking(Heap* heap,
   if (!force_completion) return;
 
   while (!marking->IsComplete()) {
-    marking->Step(kStepSizeInMs,
-                  i::IncrementalMarking::CompletionAction::kGCViaTask,
-                  i::StepOrigin::kV8);
+    marking->Step(kStepSizeInMs, i::StepOrigin::kV8);
   }
   CHECK(marking->IsComplete());
 }