From 0713bce1fe9b4ef4ebcc7b749687799693dedaeb Mon Sep 17 00:00:00 2001
From: Ryan Macnak <rmacnak@google.com>
Date: Wed, 12 Jun 2024 20:53:18 +0000
Subject: [PATCH] [vm, gc] Check for pause requests when marking large arrays.

This was sometimes responsible for the largest GC pause in IsolateSendExitLatency, with a scavenge waiting for a concurrent marker to finish a whole array and pause.

TEST=ci
Change-Id: Ib2d8417ffe6cee3e8436a8c382d2db660e1eef41
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/371180
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
---
 runtime/lib/isolate.cc          | 3 +--
 runtime/vm/heap/marker.cc       | 6 ++++++
 runtime/vm/heap/page.h          | 5 +++++
 runtime/vm/object.cc            | 8 ++++----
 runtime/vm/object_graph_copy.cc | 4 ++--
 5 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/runtime/lib/isolate.cc b/runtime/lib/isolate.cc
index 463aa4b4e35..33ad7820a46 100644
--- a/runtime/lib/isolate.cc
+++ b/runtime/lib/isolate.cc
@@ -225,11 +225,10 @@ static ObjectPtr ValidateMessageObject(Zone* zone,
       case kArrayCid: {
         array ^= Array::RawCast(raw);
         visitor.VisitObject(array.GetTypeArguments());
-        const intptr_t batch_size = (2 << 14) - 1;
         for (intptr_t i = 0; i < array.Length(); ++i) {
           ObjectPtr ptr = array.At(i);
           visitor.VisitObject(ptr);
-          if ((i & batch_size) == batch_size) {
+          if (((i + 1) % kSlotsPerInterruptCheck) == 0) {
             thread->CheckForSafepoint();
           }
         }
diff --git a/runtime/vm/heap/marker.cc b/runtime/vm/heap/marker.cc
index 623ad6fb797..265e104e222 100644
--- a/runtime/vm/heap/marker.cc
+++ b/runtime/vm/heap/marker.cc
@@ -213,6 +213,12 @@ class MarkingVisitorBase : public ObjectPointerVisitor {
         has_evacuation_candidate_ = false;
         page->RememberCard(card_from);
       }
+
+      if (((i + 1) % kCardsPerInterruptCheck) == 0) {
+        if (UNLIKELY(page_space_->pause_concurrent_marking())) {
+          YieldConcurrentMarking();
+        }
+      }
     }
 
     return obj->untag()->HeapSize();
diff --git a/runtime/vm/heap/page.h b/runtime/vm/heap/page.h
index 8d106ec7c8c..7deb9819073 100644
--- a/runtime/vm/heap/page.h
+++ b/runtime/vm/heap/page.h
@@ -172,6 +172,7 @@ class Page {
 
   // 1 card = 32 slots.
   static constexpr intptr_t kSlotsPerCardLog2 = 5;
+  static constexpr intptr_t kSlotsPerCard = 1 << kSlotsPerCardLog2;
   static constexpr intptr_t kBytesPerCardLog2 =
       kCompressedWordSizeLog2 + kSlotsPerCardLog2;
 
@@ -355,6 +356,10 @@ class Page {
   DISALLOW_IMPLICIT_CONSTRUCTORS(Page);
 };
 
+static constexpr intptr_t kSlotsPerInterruptCheck = KB;
+static constexpr intptr_t kCardsPerInterruptCheck =
+    kSlotsPerInterruptCheck / Page::kSlotsPerCard;
+
 }  // namespace dart
 
 #endif  // RUNTIME_VM_HEAP_PAGE_H_
diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
index 8803f5bd139..1a7c11988c1 100644
--- a/runtime/vm/object.cc
+++ b/runtime/vm/object.cc
@@ -24778,7 +24778,7 @@ ArrayPtr Array::New(intptr_t class_id, intptr_t len, Heap::Space space) {
   result.SetTypeArguments(Object::null_type_arguments());
   for (intptr_t i = 0; i < len; i++) {
     result.SetAt(i, Object::null_object(), thread);
-    if (((i + 1) % KB) == 0) {
+    if (((i + 1) % kSlotsPerInterruptCheck) == 0) {
       thread->CheckForSafepoint();
     }
   }
@@ -24804,7 +24804,7 @@ ArrayPtr Array::Slice(intptr_t start,
   } else {
     for (int i = 0; i < count; i++) {
       dest.untag()->set_element(i, untag()->element(i + start), thread);
-      if (((i + 1) % KB) == 0) {
+      if (((i + 1) % kSlotsPerInterruptCheck) == 0) {
         thread->CheckForSafepoint();
       }
     }
@@ -24855,13 +24855,13 @@ ArrayPtr Array::Grow(const Array& source,
   } else {
     for (intptr_t i = 0; i < old_length; i++) {
       result.untag()->set_element(i, source.untag()->element(i), thread);
-      if (((i + 1) % KB) == 0) {
+      if (((i + 1) % kSlotsPerInterruptCheck) == 0) {
         thread->CheckForSafepoint();
       }
     }
     for (intptr_t i = old_length; i < new_length; i++) {
       result.untag()->set_element(i, Object::null(), thread);
-      if (((i + 1) % KB) == 0) {
+      if (((i + 1) % kSlotsPerInterruptCheck) == 0) {
         thread->CheckForSafepoint();
       }
     }
diff --git a/runtime/vm/object_graph_copy.cc b/runtime/vm/object_graph_copy.cc
index 4e57cab781f..a6abebdf9a2 100644
--- a/runtime/vm/object_graph_copy.cc
+++ b/runtime/vm/object_graph_copy.cc
@@ -497,7 +497,7 @@ class IdentityMap {
         malloc(hash_table_capacity_ * sizeof(uint32_t)));
     for (intptr_t i = 0; i < hash_table_capacity_; i++) {
       hash_table_[i] = 0;
-      if (check_for_safepoint && (((i + 1) % KB) == 0)) {
+      if (check_for_safepoint && (((i + 1) % kSlotsPerInterruptCheck) == 0)) {
         thread_->CheckForSafepoint();
       }
     }
@@ -513,7 +513,7 @@ class IdentityMap {
         }
         probe = (probe + 1) & mask;
       }
-      if (check_for_safepoint && (((id + 2) % KB) == 0)) {
+      if (check_for_safepoint && (((id + 2) % kSlotsPerInterruptCheck) == 0)) {
         thread_->CheckForSafepoint();
       }
     }