[heap] Possibly finalize marking in Heap::ReportExternalMemoryPressure

We used to treat Heap::ReportExternalMemoryPressure just like
allocation observer marking steps. Which means that we advance
incremental marking but never finalize here immediately. This is
now problematic without a separate COMPLETE phase when we don't reach
the stack guard because we are stuck in C++ for awhile. In such cases
we might perform way more marking work than we used to.

We can fix this by finalizing marking immediately at this point when
the stack guard was already armed. Otherwise we prefer to finalize
marking in a task where we don't have a stack at all.

For this we add a new method
IncrementalMarking::AdvanceAndFinalizeIfNecessary. AdvanceFromTask
is renamed to AdvanceAndFinalizeIfComplete to make the difference
between those methods more clear.

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

src/heap/heap.cc

@@ -1725,7 +1725,7 @@ void Heap::ReportExternalMemoryPressure() {
     // Incremental marking is turned on and has already been started.
     current_gc_callback_flags_ = static_cast<GCCallbackFlags>(
         current_gc_callback_flags_ | kGCCallbackFlagsForExternalMemory);
-    incremental_marking()->AdvanceOnAllocation();
+    incremental_marking()->AdvanceAndFinalizeIfNecessary();
   }
 }
 
@@ -3996,7 +3996,7 @@ bool Heap::PerformIdleTimeAction(GCIdleTimeAction action,
       result = true;
       break;
     case GCIdleTimeAction::kIncrementalStep: {
-      incremental_marking()->AdvanceFromTask();
+      incremental_marking()->AdvanceAndFinalizeIfComplete();
       result = incremental_marking()->IsStopped();
       break;
     }

src/heap/incremental-marking-job.cc

@@ -117,7 +117,7 @@ void IncrementalMarkingJob::Task::RunInternal() {
     heap->new_space()->MarkLabStartInitialized();
     heap->new_lo_space()->ResetPendingObject();
 
-    heap->incremental_marking()->AdvanceFromTask();
+    heap->incremental_marking()->AdvanceAndFinalizeIfComplete();
 
     if (incremental_marking->IsRunning()) {
       // TODO(v8:12775): It is quite suprising that we schedule the task

src/heap/incremental-marking.cc

@@ -655,7 +655,7 @@ void IncrementalMarking::ScheduleBytesToMarkBasedOnTime(double time_ms) {
   }
 }
 
-void IncrementalMarking::AdvanceFromTask() {
+void IncrementalMarking::AdvanceAndFinalizeIfComplete() {
   ScheduleBytesToMarkBasedOnTime(heap()->MonotonicallyIncreasingTimeInMs());
   FastForwardScheduleIfCloseToFinalization();
   Step(kStepSizeInMs, StepOrigin::kTask);
@@ -663,6 +663,16 @@ void IncrementalMarking::AdvanceFromTask() {
       GarbageCollectionReason::kFinalizeMarkingViaTask);
 }
 
+void IncrementalMarking::AdvanceAndFinalizeIfNecessary() {
+  DCHECK(!heap_->always_allocate());
+  AdvanceOnAllocation();
+
+  if (collection_requested_via_stack_guard_) {
+    heap()->FinalizeIncrementalMarkingIfComplete(
+        GarbageCollectionReason::kFinalizeMarkingViaStackGuard);
+  }
+}
+
 void IncrementalMarking::AdvanceForTesting(double max_step_size_in_ms) {
   Step(max_step_size_in_ms, StepOrigin::kV8);
 }

src/heap/incremental-marking.h

@@ -117,7 +117,12 @@ class V8_EXPORT_PRIVATE IncrementalMarking final {
   void UpdateMarkedBytesAfterScavenge(size_t dead_bytes_in_new_space);
 
   // Performs incremental marking step and finalizes marking if complete.
-  void AdvanceFromTask();
+  void AdvanceAndFinalizeIfComplete();
+
+  // Performs incremental marking step and finalizes marking if the stack guard
+  // was already armed. If marking is complete but the stack guard wasn't armed
+  // yet, a finalization task is scheduled.
+  void AdvanceAndFinalizeIfNecessary();
 
   // Performs incremental marking step and schedules job for finalization if
   // marking completes.

src/heap/memory-reducer.cc

@@ -84,7 +84,7 @@ void MemoryReducer::NotifyTimer(const Event& event) {
       // Make progress with pending incremental marking if memory usage has
       // higher priority than latency. This is important for background tabs
       // that do not send idle notifications.
-      heap()->incremental_marking()->AdvanceFromTask();
+      heap()->incremental_marking()->AdvanceAndFinalizeIfComplete();
     }
     // Re-schedule the timer.
     ScheduleTimer(state_.next_gc_start_ms - event.time_ms);