From 2d01bf3779dca7204e349d87b80bf6f3da214b8d Mon Sep 17 00:00:00 2001
From: Ryan Macnak <rmacnak@google.com>
Date: Thu, 14 Dec 2023 17:27:49 +0000
Subject: [PATCH] [vm, gc] Interrupt to finalize concurrent marking.

STW marking is no longer O(new-space), so there's no longer a reason to delay finalizing marking hoping for a scavenge to make new-space mostly empty.

TEST=ci
Change-Id: Ie782e88852714d30e0c75aa9aecac62e56c434ce
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/319880
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
---
 runtime/vm/heap/heap.cc     | 14 ++++++++++++++
 runtime/vm/heap/heap.h      |  1 +
 runtime/vm/heap/marker.cc   |  1 +
 runtime/vm/isolate.cc       |  7 +++++++
 runtime/vm/isolate.h        |  2 ++
 runtime/vm/runtime_entry.cc |  1 +
 runtime/vm/thread.cc        |  1 +
 7 files changed, 27 insertions(+)

diff --git a/runtime/vm/heap/heap.cc b/runtime/vm/heap/heap.cc
index 237f71c2373..19de6f1717d 100644
--- a/runtime/vm/heap/heap.cc
+++ b/runtime/vm/heap/heap.cc
@@ -613,6 +613,20 @@ void Heap::CheckConcurrentMarking(Thread* thread,
   }
 }
 
+void Heap::CheckFinalizeMarking(Thread* thread) {
+  ASSERT(!thread->force_growth());
+
+  PageSpace::Phase phase;
+  {
+    MonitorLocker ml(old_space_.tasks_lock());
+    phase = old_space_.phase();
+  }
+
+  if (phase == PageSpace::kAwaitingFinalization) {
+    CollectOldSpaceGarbage(thread, GCType::kMarkSweep, GCReason::kFinalize);
+  }
+}
+
 void Heap::StartConcurrentMarking(Thread* thread, GCReason reason) {
   GcSafepointOperationScope safepoint_operation(thread);
   RecordBeforeGC(GCType::kStartConcurrentMark, reason);
diff --git a/runtime/vm/heap/heap.h b/runtime/vm/heap/heap.h
index 057edb3bcbd..95b0db4c665 100644
--- a/runtime/vm/heap/heap.h
+++ b/runtime/vm/heap/heap.h
@@ -115,6 +115,7 @@ class Heap {
 
   void CheckCatchUp(Thread* thread);
   void CheckConcurrentMarking(Thread* thread, GCReason reason, intptr_t size);
+  void CheckFinalizeMarking(Thread* thread);
   void StartConcurrentMarking(Thread* thread, GCReason reason);
   void WaitForMarkerTasks(Thread* thread);
   void WaitForSweeperTasks(Thread* thread);
diff --git a/runtime/vm/heap/marker.cc b/runtime/vm/heap/marker.cc
index 4cfb542ec37..4f0cfcc66ff 100644
--- a/runtime/vm/heap/marker.cc
+++ b/runtime/vm/heap/marker.cc
@@ -920,6 +920,7 @@ class ConcurrentMarkTask : public ThreadPool::Task {
       ASSERT(page_space_->phase() == PageSpace::kMarking);
       if (page_space_->concurrent_marker_tasks() == 0) {
         page_space_->set_phase(PageSpace::kAwaitingFinalization);
+        isolate_group_->ScheduleInterrupts(Thread::kVMInterrupt);
       }
       ml.NotifyAll();
     }
diff --git a/runtime/vm/isolate.cc b/runtime/vm/isolate.cc
index 50c0425532a..39a5d338456 100644
--- a/runtime/vm/isolate.cc
+++ b/runtime/vm/isolate.cc
@@ -1886,6 +1886,13 @@ void IsolateGroup::SetupImagePage(const uint8_t* image_buffer,
                          is_executable);
 }
 
+void IsolateGroup::ScheduleInterrupts(uword interrupt_bits) {
+  SafepointReadRwLocker ml(Thread::Current(), isolates_lock_.get());
+  for (Isolate* isolate : isolates_) {
+    isolate->ScheduleInterrupts(interrupt_bits);
+  }
+}
+
 void Isolate::ScheduleInterrupts(uword interrupt_bits) {
   // We take the threads lock here to ensure that the mutator thread does not
   // exit the isolate while we are trying to schedule interrupts on it.
diff --git a/runtime/vm/isolate.h b/runtime/vm/isolate.h
index 71c6c57185d..d36a562af6c 100644
--- a/runtime/vm/isolate.h
+++ b/runtime/vm/isolate.h
@@ -335,6 +335,8 @@ class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
 
   void RunWithLockedGroup(std::function<void()> fun);
 
+  void ScheduleInterrupts(uword interrupt_bits);
+
   ThreadRegistry* thread_registry() const { return thread_registry_.get(); }
   SafepointHandler* safepoint_handler() { return safepoint_handler_.get(); }
 
diff --git a/runtime/vm/runtime_entry.cc b/runtime/vm/runtime_entry.cc
index bcbaf607d5d..2d128b26ea9 100644
--- a/runtime/vm/runtime_entry.cc
+++ b/runtime/vm/runtime_entry.cc
@@ -3152,6 +3152,7 @@ DEFINE_RUNTIME_ENTRY(InterruptOrStackOverflow, 0) {
   // Handle interrupts:
   //  - store buffer overflow
   //  - OOB message (vm-service or dart:isolate)
+  //  - marking ready for finalization
   const Error& error = Error::Handle(thread->HandleInterrupts());
   ThrowIfError(error);
 
diff --git a/runtime/vm/thread.cc b/runtime/vm/thread.cc
index 021d57587dd..f54c685f914 100644
--- a/runtime/vm/thread.cc
+++ b/runtime/vm/thread.cc
@@ -741,6 +741,7 @@ ErrorPtr Thread::HandleInterrupts() {
       // occur that does promote them.
       heap()->CollectGarbage(this, GCType::kEvacuate, GCReason::kStoreBuffer);
     }
+    heap()->CheckFinalizeMarking(this);
 
 #if !defined(PRODUCT)
     if (isolate()->TakeHasCompletedBlocks()) {