[vm, gc] Remove force_growth option from RunWithStoppedMutators.

Adjust wait for sweeper to account for whether we're already under a safepoint operation.

TEST=ci
Bug: #61019
Change-Id: Ic30500a1348dc8a719e72be64d69e496986107aa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/437901
Reviewed-by: Alexander Aprelev <[email protected]>
Commit-Queue: Ryan Macnak <[email protected]>

Author: rmacnak-google

runtime/vm/compiler/backend/il_test_helper.cc

@@ -107,8 +107,7 @@ InstructionsPtr BuildInstructions(
                               /*optimized=*/false, /*stats=*/nullptr);
   };
   SafepointWriteRwLocker ml(thread, thread->isolate_group()->program_lock());
-  thread->isolate_group()->RunWithStoppedMutators(install_code_fun,
-                                                  /*use_force_growth=*/true);
+  thread->isolate_group()->RunWithStoppedMutators(install_code_fun);
   return code.instructions();
 }
 

runtime/vm/compiler/jit/compiler.cc

@@ -597,8 +597,7 @@ CodePtr CompileParsedFunctionHelper::Compile() {
         //      requires updating several pointers: We have to ensure all of
         //      those writes are observed atomically.
         //
-        thread()->isolate_group()->RunWithStoppedMutators(
-            install_code_fun, /*use_force_growth=*/true);
+        thread()->isolate_group()->RunWithStoppedMutators(install_code_fun);
       }
       if (!result->IsNull()) {
         // Must be called outside of safepoint.

runtime/vm/heap/heap.cc

@@ -153,7 +153,7 @@ uword Heap::AllocateOld(Thread* thread, intptr_t size, bool is_exec) {
       return addr;
     }
     // Wait for all of the concurrent tasks to finish before giving up.
-    WaitForSweeperTasksAtSafepoint(thread);
+    WaitForSweeperTasks(thread);
     addr = old_space_.TryAllocate(size, is_exec);
     if (addr != 0) {
       return addr;
@@ -165,7 +165,7 @@ uword Heap::AllocateOld(Thread* thread, intptr_t size, bool is_exec) {
     }
     // Before throwing an out-of-memory error try a synchronous GC.
     CollectOldSpaceGarbage(thread, GCType::kMarkCompact, GCReason::kOldSpace);
-    WaitForSweeperTasksAtSafepoint(thread);
+    WaitForSweeperTasks(thread);
     addr = old_space_.TryAllocate(size, is_exec, PageSpace::kForceGrowth);
     if (addr != 0) {
       return addr;
@@ -669,20 +669,14 @@ void Heap::WaitForMarkerTasks(Thread* thread) {
 }
 
 void Heap::WaitForSweeperTasks(Thread* thread) {
-  ASSERT(!thread->OwnsGCSafepoint());
-  MonitorLocker ml(old_space_.tasks_lock());
-  while ((old_space_.phase() == PageSpace::kSweepingLarge) ||
-         (old_space_.phase() == PageSpace::kSweepingRegular)) {
-    ml.WaitWithSafepointCheck(thread);
-  }
-}
-
-void Heap::WaitForSweeperTasksAtSafepoint(Thread* thread) {
-  ASSERT(thread->OwnsGCSafepoint());
   MonitorLocker ml(old_space_.tasks_lock());
   while ((old_space_.phase() == PageSpace::kSweepingLarge) ||
          (old_space_.phase() == PageSpace::kSweepingRegular)) {
-    ml.Wait();
+    if (thread->OwnsSafepoint()) {
+      ml.Wait();
+    } else {
+      ml.WaitWithSafepointCheck(thread);
+    }
   }
 }
 

runtime/vm/heap/heap.h

@@ -119,7 +119,6 @@ class Heap {
   void StartConcurrentMarking(Thread* thread, GCReason reason);
   void WaitForMarkerTasks(Thread* thread);
   void WaitForSweeperTasks(Thread* thread);
-  void WaitForSweeperTasksAtSafepoint(Thread* thread);
 
   // Protect access to the heap. Note: Code pages are made
   // executable/non-executable when 'read_only' is true/false, respectively.

runtime/vm/heap/scavenger.cc

@@ -1015,7 +1015,7 @@ SemiSpace* Scavenger::Prologue(GCReason reason) {
   heap_->isolate_group()->FlushMarkingStacks();
 
   if (FLAG_verify_store_buffer) {
-    heap_->WaitForSweeperTasksAtSafepoint(Thread::Current());
+    heap_->WaitForSweeperTasks(Thread::Current());
     VerifyStoreBuffers("Verifying remembered set before Scavenge");
   }
 
@@ -1118,7 +1118,7 @@ void Scavenger::Epilogue(SemiSpace* from) {
     // are very rare.
     heap_->isolate_group()->ReleaseStoreBuffers();
 
-    heap_->WaitForSweeperTasksAtSafepoint(Thread::Current());
+    heap_->WaitForSweeperTasks(Thread::Current());
     VerifyStoreBuffers("Verifying remembered set after Scavenge");
   }
 
@@ -1854,7 +1854,7 @@ void Scavenger::Scavenge(Thread* thread, GCType type, GCReason reason) {
   }
 
   if (FLAG_verify_before_gc) {
-    heap_->WaitForSweeperTasksAtSafepoint(thread);
+    heap_->WaitForSweeperTasks(thread);
     heap_->VerifyGC("Verifying before Scavenge",
                     thread->is_marking() ? kAllowMarked : kForbidMarked);
   }
@@ -1928,7 +1928,7 @@ void Scavenger::Scavenge(Thread* thread, GCType type, GCReason reason) {
   heap_->old_space()->ResumeConcurrentMarking();
 
   if (FLAG_verify_after_gc) {
-    heap_->WaitForSweeperTasksAtSafepoint(thread);
+    heap_->WaitForSweeperTasks(thread);
     heap_->VerifyGC("Verifying after Scavenge...",
                     thread->is_marking() ? kAllowMarked : kForbidMarked);
   }
@@ -2030,7 +2030,7 @@ void Scavenger::ReverseScavenge(SemiSpace** from) {
 
   // Reverse the partial forwarding from the aborted scavenge. This also
   // rebuilds the remembered set.
-  heap_->WaitForSweeperTasksAtSafepoint(thread);
+  heap_->WaitForSweeperTasks(thread);
   Become::FollowForwardingPointers(thread);
 
   heap_->old_space()->ResetProgressBars();

runtime/vm/isolate.cc

@@ -2909,8 +2909,7 @@ Isolate* IsolateGroup::FirstIsolateLocked() const {
   return isolates_.IsEmpty() ? nullptr : isolates_.First();
 }
 
-void IsolateGroup::RunWithStoppedMutatorsCallable(Callable* callable,
-                                                  bool use_force_growth) {
+void IsolateGroup::RunWithStoppedMutatorsCallable(Callable* callable) {
   auto thread = Thread::Current();
   StoppedMutatorsScope stopped_mutators_scope(thread);
 
@@ -2923,14 +2922,8 @@ void IsolateGroup::RunWithStoppedMutatorsCallable(Callable* callable,
   // We use the more strict safepoint operation scope here (which ensures that
   // all other threads, including auxiliary threads are at a safepoint), even
   // though we only need to ensure that the mutator threads are stopped.
-  if (use_force_growth) {
-    ForceGrowthSafepointOperationScope safepoint_scope(
-        thread, SafepointLevel::kGCAndDeopt);
-    callable->Call();
-  } else {
-    DeoptSafepointOperationScope safepoint_scope(thread);
-    callable->Call();
-  }
+  DeoptSafepointOperationScope safepoint_scope(thread);
+  callable->Call();
 }
 
 void IsolateGroup::VisitObjectPointers(ObjectPointerVisitor* visitor,

runtime/vm/isolate.h

@@ -599,19 +599,16 @@ class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
   //
   // If the current thread is the only mutator in the isolate group,
   // [callable] will be called directly. Otherwise [callable] will be
-  // called inside a [SafepointOperationsScope] (or
-  // [ForceGrowthSafepointOperationScope] if [use_force_growth]
-  // is set).
+  // called inside a [SafepointOperationsScope].
   //
   // During the duration of this function, no new isolates can be added to the
   // isolate group.
-  void RunWithStoppedMutatorsCallable(Callable* callable,
-                                      bool use_force_growth = false);
+  void RunWithStoppedMutatorsCallable(Callable* callable);
 
   template <typename T>
-  void RunWithStoppedMutators(T function, bool use_force_growth = false) {
+  void RunWithStoppedMutators(T function) {
     LambdaCallable<T> callable(function);
-    RunWithStoppedMutatorsCallable(&callable, use_force_growth);
+    RunWithStoppedMutatorsCallable(&callable);
   }
 
 #ifndef PRODUCT

runtime/vm/object.cc

@@ -19743,12 +19743,10 @@ void MegamorphicCache::InsertLocked(const Smi& class_id,
   // NOTE: In the future we might change the megamorphic cache insertions to
   // carefully use store-release barriers on the writer as well as
   // load-acquire barriers on the reader, ...
-  isolate_group->RunWithStoppedMutators(
-      [&]() {
-        EnsureCapacityLocked();
-        InsertEntryLocked(class_id, target);
-      },
-      /*use_force_growth=*/true);
+  isolate_group->RunWithStoppedMutators([&]() {
+    EnsureCapacityLocked();
+    InsertEntryLocked(class_id, target);
+  });
 }
 
 void MegamorphicCache::EnsureCapacityLocked() const {

runtime/vm/stub_code.cc

@@ -282,8 +282,7 @@ CodePtr StubCode::GetAllocationStubForClass(const Class& cls) {
     //
     //   a) We allocate an instructions object, which might cause us to
     //      temporarily flip page protections from (RX -> RW -> RX).
-    thread->isolate_group()->RunWithStoppedMutators(mutator_fun,
-                                                    /*use_force_growth=*/true);
+    thread->isolate_group()->RunWithStoppedMutators(mutator_fun);
 
     // We notify code observers after finalizing the code in order to be
     // outside a [SafepointOperationScope].

runtime/vm/type_testing_stubs.cc

@@ -296,9 +296,7 @@ CodePtr TypeTestingStubGenerator::BuildCodeForType(const AbstractType& type) {
             //
             SafepointWriteRwLocker ml(thread,
                                       thread->isolate_group()->program_lock());
-            thread->isolate_group()->RunWithStoppedMutators(
-                install_code_fun,
-                /*use_force_growth=*/true);
+            thread->isolate_group()->RunWithStoppedMutators(install_code_fun);
 
             Code::NotifyCodeObservers(name, code, /*optimized=*/false);