commit a1b7e761eb2ca4046c40aaa0d2458ca31615841c
author Martin Kustermann <kustermann@google.com> Wed Apr 19 07:48:26 2023 +0000
committer Commit Queue <dart-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Apr 19 07:48:26 2023 +0000
tree a887859bfa4f0372b460025e0152f04946cec79f
parent abeb88a038a21489e708c00b65103ca9a8c24296

[vm] Make vm/cc/SafepointOperation_SafepointPointTest more robust

Closes https://github.com/dart-lang/sdk/issues/48716

TEST=vm/cc/SafepointOperation_SafepointPointTest

Change-Id: I03e392710fdc37da4a7c3b2c0934f4c4c71958fc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/295780
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>

runtime/vm/heap/safepoint_test.cc

diff --git a/runtime/vm/heap/safepoint_test.cc b/runtime/vm/heap/safepoint_test.cc
index daedca2..c04aa74 100644
--- a/runtime/vm/heap/safepoint_test.cc
+++ b/runtime/vm/heap/safepoint_test.cc
@@ -295,15 +295,18 @@
     Data(IsolateGroup* isolate_group,
          SafepointLevel level,
          std::atomic<intptr_t>* gc_only_checkins,
-         std::atomic<intptr_t>* deopt_checkin)
+         std::atomic<intptr_t>* deopt_checkin,
+         std::atomic<intptr_t>* timeout_checkin)
         : StateMachineTask::Data(isolate_group),
           level(level),
           gc_only_checkins(gc_only_checkins),
-          deopt_checkin(deopt_checkin) {}
+          deopt_checkin(deopt_checkin),
+          timeout_checkin(timeout_checkin) {}
 
     SafepointLevel level;
     std::atomic<intptr_t>* gc_only_checkins;
     std::atomic<intptr_t>* deopt_checkin;
+    std::atomic<intptr_t>* timeout_checkin;
   };
 
   explicit CheckinTask(std::shared_ptr<Data> data) : StateMachineTask(data) {}
@@ -348,9 +351,9 @@
       // thread continue.
       const auto now = OS::GetCurrentTimeMillis();
       if ((now - last_sync) > 1000) {
-        thread_->EnterSafepoint();
-        thread_->ExitSafepoint();
-        last_sync = now;
+        if (SafepointIfRequested(thread_, data()->timeout_checkin)) {
+          last_sync = now;
+        }
       }
     }
   }
@@ -372,20 +375,55 @@
 ISOLATE_UNIT_TEST_CASE(SafepointOperation_SafepointPointTest) {
   auto isolate_group = thread->isolate_group();
 
-  const intptr_t kTaskCount = 5;
+  const intptr_t kTaskCount = 6;
   std::atomic<intptr_t> gc_only_checkins[kTaskCount];
   std::atomic<intptr_t> deopt_checkin[kTaskCount];
+  std::atomic<intptr_t> timeout_checkins[kTaskCount];
   for (intptr_t i = 0; i < kTaskCount; ++i) {
     gc_only_checkins[i] = 0;
     deopt_checkin[i] = 0;
+    timeout_checkins[i] = 0;
   }
 
+  auto task_to_level = [](intptr_t task_id) -> SafepointLevel {
+    switch (task_id) {
+      case 0:
+      case 1:
+      case 2:
+        return SafepointLevel::kGC;
+      case 3:
+      case 4:
+      case 5:
+        return SafepointLevel::kGCAndDeopt;
+      default:
+        UNREACHABLE();
+        return SafepointLevel::kGC;
+    }
+  };
+  auto wait_for_sync = [&](intptr_t syncs) {
+    while (true) {
+      bool ready = true;
+      for (intptr_t i = 0; i < kTaskCount; ++i) {
+        const intptr_t all =
+            gc_only_checkins[i] + deopt_checkin[i] + timeout_checkins[i];
+        if (all != syncs) {
+          ready = false;
+          break;
+        }
+      }
+      if (ready) {
+        return;
+      }
+      OS::SleepMicros(1000);
+    }
+  };
+
   std::vector<std::shared_ptr<CheckinTask::Data>> threads;
   for (intptr_t i = 0; i < kTaskCount; ++i) {
-    const auto level =
-        (i % 2) == 0 ? SafepointLevel::kGC : SafepointLevel::kGCAndDeopt;
-    std::unique_ptr<CheckinTask::Data> data(new CheckinTask::Data(
-        isolate_group, level, &gc_only_checkins[i], &deopt_checkin[i]));
+    const auto level = task_to_level(i);
+    std::unique_ptr<CheckinTask::Data> data(
+        new CheckinTask::Data(isolate_group, level, &gc_only_checkins[i],
+                              &deopt_checkin[i], &timeout_checkins[i]));
     threads.push_back(std::move(data));
   }
 
@@ -404,11 +442,11 @@
     }
     {
       { GcSafepointOperationScope safepoint_operation(thread); }
-      OS::SleepMicros(1000);  // Wait for threads to exit safepoint
+      wait_for_sync(1);  // Wait for threads to exit safepoint
       { DeoptSafepointOperationScope safepoint_operation(thread); }
-      OS::SleepMicros(1000);  // Wait for threads to exit safepoint
+      wait_for_sync(2);  // Wait for threads to exit safepoint
       { GcSafepointOperationScope safepoint_operation(thread); }
-      OS::SleepMicros(1000);  // Wait for threads to exit safepoint
+      wait_for_sync(3);  // Wait for threads to exit safepoint
       { DeoptSafepointOperationScope safepoint_operation(thread); }
     }
     for (intptr_t i = 0; i < kTaskCount; i++) {
@@ -418,16 +456,16 @@
       threads[i]->WaitUntil(CheckinTask::kExited);
     }
     for (intptr_t i = 0; i < kTaskCount; ++i) {
-      const auto level =
-          (i % 2) == 0 ? SafepointLevel::kGC : SafepointLevel::kGCAndDeopt;
-      switch (level) {
+      switch (task_to_level(i)) {
         case SafepointLevel::kGC:
           EXPECT_EQ(0, deopt_checkin[i]);
           EXPECT_EQ(2, gc_only_checkins[i]);
+          EXPECT_EQ(2, timeout_checkins[i]);
           break;
         case SafepointLevel::kGCAndDeopt:
           EXPECT_EQ(4, deopt_checkin[i]);
           EXPECT_EQ(0, gc_only_checkins[i]);
+          EXPECT_EQ(0, timeout_checkins[i]);
           break;
         case SafepointLevel::kNumLevels:
           UNREACHABLE();