[heap] Simplify incremental marking

In preparation for removing the finalization step, simplify
incremental marking logic.

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

src/heap/heap.cc

@@ -1526,13 +1526,11 @@ void Heap::HandleGCRequest() {
     CollectAllGarbage(NEW_SPACE, GarbageCollectionReason::kTesting);
     stress_scavenge_observer_->RequestedGCDone();
   } else if (HighMemoryPressure()) {
-    incremental_marking()->reset_request_type();
     CheckMemoryPressure();
   } else if (CollectionRequested()) {
     CheckCollectionRequested();
   } else if (incremental_marking()->request_type() ==
              IncrementalMarking::GCRequestType::COMPLETE_MARKING) {
-    incremental_marking()->reset_request_type();
     CollectAllGarbage(current_gc_flags_,
                       GarbageCollectionReason::kFinalizeMarkingViaStackGuard,
                       current_gc_callback_flags_);
@@ -1540,7 +1538,6 @@ void Heap::HandleGCRequest() {
                  IncrementalMarking::GCRequestType::FINALIZATION &&
              incremental_marking()->IsMarking() &&
              !incremental_marking()->finalize_marking_completed()) {
-    incremental_marking()->reset_request_type();
     FinalizeIncrementalMarkingIncrementally(
         GarbageCollectionReason::kFinalizeMarkingViaStackGuard);
   }
@@ -1732,7 +1729,8 @@ void Heap::ReportExternalMemoryPressure() {
     current_gc_callback_flags_ = static_cast<GCCallbackFlags>(
         current_gc_callback_flags_ | kGCCallbackFlagsForExternalMemory);
     incremental_marking()->AdvanceWithDeadline(
-        deadline, IncrementalMarking::GC_VIA_STACK_GUARD, StepOrigin::kV8);
+        deadline, IncrementalMarking::CompletionAction::kGcViaStackGuard,
+        StepOrigin::kV8);
   }
 }
 
@@ -2662,8 +2660,6 @@ void Heap::MarkCompactEpilogue() {
   isolate_->counters()->objs_since_last_full()->Set(0);
 
   incremental_marking()->Epilogue();
-
-  DCHECK(incremental_marking()->IsStopped());
 }
 
 void Heap::MarkCompactPrologue() {
@@ -4093,7 +4089,7 @@ bool Heap::PerformIdleTimeAction(GCIdleTimeAction action,
       break;
     case GCIdleTimeAction::kIncrementalStep: {
       incremental_marking()->AdvanceWithDeadline(
-          deadline_in_ms, IncrementalMarking::NO_GC_VIA_STACK_GUARD,
+          deadline_in_ms, IncrementalMarking::CompletionAction::kGCViaTask,
           StepOrigin::kTask);
       FinalizeIncrementalMarkingIfComplete(
           GarbageCollectionReason::kFinalizeMarkingViaTask);

src/heap/heap.h

@@ -1079,8 +1079,6 @@ class Heap {
     return reinterpret_cast<Address*>(&is_marking_flag_);
   }
 
-  void SetIsMarkingFlag(uint8_t flag) { is_marking_flag_ = flag; }
-
   void ClearRecordedSlot(HeapObject object, ObjectSlot slot);
   void ClearRecordedSlotRange(Address start, Address end);
   static int InsertIntoRememberedSetFromCode(MemoryChunk* chunk, Address slot);
@@ -2146,6 +2144,10 @@ class Heap {
 
   bool IsStressingScavenge();
 
+  void SetIsMarkingFlag(bool value) {
+    is_marking_flag_ = static_cast<uint8_t>(value);
+  }
+
   ExternalMemoryAccounting external_memory_;
 
   // This can be calculated directly from a pointer to the heap; however, it is

src/heap/incremental-marking-job.cc

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

src/heap/incremental-marking.cc

@@ -564,7 +564,7 @@ void IncrementalMarking::FinalizeMarking(CompletionAction action) {
         "marking.\n");
   }
   request_type_ = GCRequestType::FINALIZATION;
-  if (action == GC_VIA_STACK_GUARD) {
+  if (action == CompletionAction::kGcViaStackGuard) {
     heap_->isolate()->stack_guard()->RequestGC();
   }
 }
@@ -585,7 +585,7 @@ void IncrementalMarking::MarkingComplete(CompletionAction action) {
   // marking was fast.
   constexpr double kMinOvershootMs = 50;
 
-  if (action == GC_VIA_STACK_GUARD) {
+  if (action == CompletionAction::kGcViaStackGuard) {
     if (time_to_force_completion_ == 0.0) {
       const double now = heap_->MonotonicallyIncreasingTimeInMs();
       const double overshoot_ms =
@@ -634,14 +634,17 @@ void IncrementalMarking::MarkingComplete(CompletionAction action) {
         "[IncrementalMarking] Complete (normal).\n");
   }
   request_type_ = GCRequestType::COMPLETE_MARKING;
-  if (action == GC_VIA_STACK_GUARD) {
+  if (action == CompletionAction::kGcViaStackGuard) {
     heap_->isolate()->stack_guard()->RequestGC();
   }
 }
 
 void IncrementalMarking::Epilogue() {
+  DCHECK(IsStopped());
+
   was_activated_ = false;
   finalize_marking_completed_ = false;
+  request_type_ = GCRequestType::NONE;
 }
 
 bool IncrementalMarking::ShouldDoEmbedderStep() {
@@ -814,7 +817,7 @@ void IncrementalMarking::AdvanceOnAllocation() {
   TRACE_GC_EPOCH(heap_->tracer(), GCTracer::Scope::MC_INCREMENTAL,
                  ThreadKind::kMain);
   ScheduleBytesToMarkBasedOnAllocation();
-  Step(kMaxStepSizeInMs, GC_VIA_STACK_GUARD, StepOrigin::kV8);
+  Step(kMaxStepSizeInMs, CompletionAction::kGcViaStackGuard, StepOrigin::kV8);
 }
 
 StepResult IncrementalMarking::Step(double max_step_size_in_ms,

src/heap/incremental-marking.h

@@ -31,7 +31,12 @@ class V8_EXPORT_PRIVATE IncrementalMarking final {
  public:
   enum State : uint8_t { STOPPED, MARKING, COMPLETE };
 
-  enum CompletionAction { GC_VIA_STACK_GUARD, NO_GC_VIA_STACK_GUARD };
+  // 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 };
 
   enum class GCRequestType { NONE, COMPLETE_MARKING, FINALIZATION };
 
@@ -97,11 +102,6 @@ class V8_EXPORT_PRIVATE IncrementalMarking final {
 
   V8_INLINE void TransferColor(HeapObject from, HeapObject to);
 
-  State state() const {
-    DCHECK(state_ == STOPPED || FLAG_incremental_marking);
-    return state_;
-  }
-
   bool finalize_marking_completed() const {
     return finalize_marking_completed_;
   }
@@ -110,11 +110,9 @@ class V8_EXPORT_PRIVATE IncrementalMarking final {
     finalize_marking_completed_ = val;
   }
 
-  inline bool IsStopped() const { return state() == STOPPED; }
-
-  inline bool IsMarking() const { return state() >= MARKING; }
-
-  inline bool IsComplete() const { return state() == COMPLETE; }
+  bool IsStopped() const { return state() == STOPPED; }
+  bool IsMarking() const { return state() >= MARKING; }
+  bool IsComplete() const { return state() == COMPLETE; }
 
   inline bool IsReadyToOverApproximateWeakClosure() const {
     return request_type_ == GCRequestType::FINALIZATION &&
@@ -128,10 +126,7 @@ class V8_EXPORT_PRIVATE IncrementalMarking final {
 
   GCRequestType request_type() const { return request_type_; }
 
-  void reset_request_type() { request_type_ = GCRequestType::NONE; }
-
   bool CanBeActivated();
-
   bool WasActivated();
 
   void Start(GarbageCollectionReason gc_reason);
@@ -262,6 +257,11 @@ class V8_EXPORT_PRIVATE IncrementalMarking final {
 
   void AdvanceOnAllocation();
 
+  State state() const {
+    DCHECK_IMPLIES(state_ != STOPPED, FLAG_incremental_marking);
+    return state_;
+  }
+
   void SetState(State s) {
     state_ = s;
     heap_->SetIsMarkingFlag(s >= MARKING);
@@ -285,10 +285,10 @@ class V8_EXPORT_PRIVATE IncrementalMarking final {
   // bytes_marked_ahead_of_schedule_ with contribution of concurrent marking.
   size_t bytes_marked_concurrently_ = 0;
 
-  // Must use SetState() above to update state_
-  // Atomic since main thread can complete marking (= changing state), while a
-  // background thread's slow allocation path will check whether incremental
-  // marking is currently running.
+  // Must use `SetState()` above to update `state_`.
+  // Atomic since main thread can complete marking while a background thread's
+  // slow allocation path will check whether incremental marking is currently
+  // running.
   std::atomic<State> state_;
 
   bool is_compacting_ = false;

src/heap/memory-reducer.cc

@@ -88,7 +88,7 @@ void MemoryReducer::NotifyTimer(const Event& event) {
       double deadline = heap()->MonotonicallyIncreasingTimeInMs() +
                         kIncrementalMarkingDelayMs;
       heap()->incremental_marking()->AdvanceWithDeadline(
-          deadline, IncrementalMarking::NO_GC_VIA_STACK_GUARD,
+          deadline, IncrementalMarking::CompletionAction::kGCViaTask,
           StepOrigin::kTask);
       heap()->FinalizeIncrementalMarkingIfComplete(
           GarbageCollectionReason::kFinalizeMarkingViaTask);

test/cctest/heap/heap-utils.cc

@@ -196,7 +196,8 @@ void SimulateIncrementalMarking(i::Heap* heap, bool force_completion) {
   if (!force_completion) return;
 
   while (!marking->IsComplete()) {
-    marking->Step(kStepSizeInMs, i::IncrementalMarking::NO_GC_VIA_STACK_GUARD,
+    marking->Step(kStepSizeInMs,
+                  i::IncrementalMarking::CompletionAction::kGCViaTask,
                   i::StepOrigin::kV8);
     if (marking->IsReadyToOverApproximateWeakClosure()) {
       SafepointScope scope(heap);

test/cctest/heap/test-heap.cc

@@ -2361,7 +2361,8 @@ 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::NO_GC_VIA_STACK_GUARD,
+    marking->Step(kStepSizeInMs,
+                  IncrementalMarking::CompletionAction::kGCViaTask,
                   StepOrigin::kV8);
   }
 
@@ -2456,7 +2457,8 @@ TEST(IdleNotificationFinishMarking) {
 
   const double kStepSizeInMs = 100;
   do {
-    marking->Step(kStepSizeInMs, IncrementalMarking::NO_GC_VIA_STACK_GUARD,
+    marking->Step(kStepSizeInMs,
+                  IncrementalMarking::CompletionAction::kGCViaTask,
                   StepOrigin::kV8);
   } while (!CcTest::heap()
                 ->mark_compact_collector()
@@ -5702,7 +5704,7 @@ TEST(Regress598319) {
   const double kSmallStepSizeInMs = 0.1;
   while (!marking->IsComplete()) {
     marking->Step(kSmallStepSizeInMs,
-                  i::IncrementalMarking::NO_GC_VIA_STACK_GUARD,
+                  i::IncrementalMarking::CompletionAction::kGCViaTask,
                   StepOrigin::kV8);
     ProgressBar& progress_bar = page->ProgressBar();
     if (progress_bar.IsEnabled() && progress_bar.Value() > 0) {
@@ -5723,7 +5725,7 @@ TEST(Regress598319) {
   const double kLargeStepSizeInMs = 1000;
   while (!marking->IsComplete()) {
     marking->Step(kLargeStepSizeInMs,
-                  i::IncrementalMarking::NO_GC_VIA_STACK_GUARD,
+                  i::IncrementalMarking::CompletionAction::kGCViaTask,
                   StepOrigin::kV8);
     if (marking->IsReadyToOverApproximateWeakClosure()) {
       SafepointScope safepoint_scope(heap);
@@ -5817,7 +5819,8 @@ TEST(Regress615489) {
   }
   const double kStepSizeInMs = 100;
   while (!marking->IsComplete()) {
-    marking->Step(kStepSizeInMs, i::IncrementalMarking::NO_GC_VIA_STACK_GUARD,
+    marking->Step(kStepSizeInMs,
+                  i::IncrementalMarking::CompletionAction::kGCViaTask,
                   StepOrigin::kV8);
     if (marking->IsReadyToOverApproximateWeakClosure()) {
       SafepointScope safepoint_scope(heap);
@@ -5880,7 +5883,8 @@ TEST(Regress631969) {
   const double kStepSizeInMs = 100;
   IncrementalMarking* marking = heap->incremental_marking();
   while (!marking->IsComplete()) {
-    marking->Step(kStepSizeInMs, i::IncrementalMarking::NO_GC_VIA_STACK_GUARD,
+    marking->Step(kStepSizeInMs,
+                  i::IncrementalMarking::CompletionAction::kGCViaTask,
                   StepOrigin::kV8);
     if (marking->IsReadyToOverApproximateWeakClosure()) {
       SafepointScope safepoint_scope(heap);
@@ -6497,7 +6501,8 @@ HEAP_TEST(Regress670675) {
     if (marking->IsStopped()) break;
     double deadline = heap->MonotonicallyIncreasingTimeInMs() + 1;
     marking->AdvanceWithDeadline(
-        deadline, IncrementalMarking::GC_VIA_STACK_GUARD, StepOrigin::kV8);
+        deadline, IncrementalMarking::CompletionAction::kGcViaStackGuard,
+        StepOrigin::kV8);
   }
   DCHECK(marking->IsStopped());
 }

test/unittests/heap/heap-utils.cc

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