Revert "[vm/shared] Ensure to iterate over mutators, rather than only isolates during deopt."

This reverts commit e5f6611.

Reason for revert: isolate_group_bound_init_test failures on mac bots: https://dart-ci.appspot.com/log/vm-aot-mac-release-x64/vm-aot-mac-release-x64/4425/ffi/isolate_group_bound_init_test/2

Original change's description:
> [vm/shared] Ensure to iterate over mutators, rather than only isolates during deopt.
>
> Iterating over only isolates ignores isolategroup-bound dart code.
>
> Fixes #61326
> TEST=isolate_group_bound_lazy_deopt_test
>
> Change-Id: Ic27679ab7eb37aa2ca703937355dc83d751e0054
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/448254
> Reviewed-by: Slava Egorov <[email protected]>
> Commit-Queue: Alexander Aprelev <[email protected]>

No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Change-Id: Ieb04abbf1b4a7fbc7637e5a7c3a3dce2010f2101
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/448602
Commit-Queue: Slava Egorov <[email protected]>
Reviewed-by: Alexander Aprelev <[email protected]>
Reviewed-by: Slava Egorov <[email protected]>
Bot-Commit: Rubber Stamper <[email protected]>

Author: aam

runtime/vm/compiler/runtime_offsets_extracted.h

@@ -11,7 +11,6 @@
 #include "vm/object.h"
 #include "vm/runtime_entry.h"
 #include "vm/stack_frame.h"
-#include "vm/thread_registry.h"
 
 namespace dart {
 
@@ -56,22 +55,28 @@ void WeakCodeReferences::DisableCode(bool are_mutators_stopped) {
   auto isolate_group = IsolateGroup::Current();
   auto disable_code_fun = [&]() {
     Code& code = Code::Handle();
-    isolate_group->ForEachMutatorAtASafepoint([&](Thread* thread) {
-      DartFrameIterator iterator(
-          thread, StackFrameIterator::kAllowCrossThreadIteration);
-      StackFrame* frame = iterator.NextFrame();
-      while (frame != nullptr) {
-        if (!frame->is_interpreted()) {
-          code = frame->LookupDartCode();
-
-          if (set.ContainsKey(code)) {
-            ReportDeoptimization(code);
-            DeoptimizeAt(thread, code, frame);
+    isolate_group->ForEachIsolate(
+        [&](Isolate* isolate) {
+          auto mutator_thread = isolate->mutator_thread();
+          if (mutator_thread == nullptr) {
+            return;
           }
-        }
-        frame = iterator.NextFrame();
-      }
-    });
+          DartFrameIterator iterator(
+              mutator_thread, StackFrameIterator::kAllowCrossThreadIteration);
+          StackFrame* frame = iterator.NextFrame();
+          while (frame != nullptr) {
+            if (!frame->is_interpreted()) {
+              code = frame->LookupDartCode();
+
+              if (set.ContainsKey(code)) {
+                ReportDeoptimization(code);
+                DeoptimizeAt(mutator_thread, code, frame);
+              }
+            }
+            frame = iterator.NextFrame();
+          }
+        },
+        /*at_safepoint=*/true);
 
     // Switch functions that use dependent code to unoptimized code.
     Object& owner = Object::Handle();

runtime/vm/heap/weak_code.cc

@@ -320,7 +320,6 @@ IsolateGroup::IsolateGroup(std::shared_ptr<IsolateGroupSource> source,
       thread_pool_(),
       isolates_lock_(new SafepointRwLock()),
       isolates_(),
-      mutators_(),
       start_time_micros_(OS::GetCurrentMonotonicMicros()),
       is_system_isolate_group_(source->flags.is_system_isolate),
       random_(),
@@ -474,15 +473,13 @@ bool IsolateGroup::UnregisterIsolateDecrementCount() {
   return isolate_count_ == 0;
 }
 
-void IsolateGroup::RegisterIsolateGroupMutator(Thread* mutator) {
+void IsolateGroup::RegisterIsolateGroupMutator() {
   SafepointWriteRwLocker ml(Thread::Current(), isolates_lock_.get());
-  mutators_.Append(mutator);
   group_mutator_count_++;
 }
 
-void IsolateGroup::UnregisterIsolateGroupMutator(Thread* mutator) {
+void IsolateGroup::UnregisterIsolateGroupMutator() {
   SafepointWriteRwLocker ml(Thread::Current(), isolates_lock_.get());
-  mutators_.Remove(mutator);
   group_mutator_count_--;
 }
 
@@ -2917,27 +2914,6 @@ Isolate* IsolateGroup::FirstIsolateLocked() const {
   return isolates_.IsEmpty() ? nullptr : isolates_.First();
 }
 
-void IsolateGroup::ForEachMutatorAtASafepoint(
-    std::function<void(Thread* thread)> function) {
-  auto thread = Thread::Current();
-  ASSERT(thread->OwnsSafepoint() ||
-         (thread->task_kind() == Thread::kMutatorTask) ||
-         (thread->task_kind() == Thread::kMarkerTask) ||
-         (thread->task_kind() == Thread::kCompactorTask) ||
-         (thread->task_kind() == Thread::kScavengerTask) ||
-         (thread->task_kind() == Thread::kIncrementalCompactorTask));
-  for (Isolate* isolate : isolates_) {
-    auto thread = isolate->mutator_thread();
-    if (thread != nullptr) {
-      function(thread);
-    }
-  }
-  for (Thread* mutator : mutators_) {
-    ASSERT(mutator != nullptr);
-    function(mutator);
-  }
-}
-
 void IsolateGroup::RunWithStoppedMutatorsCallable(Callable* callable) {
   auto thread = Thread::Current();
   StoppedMutatorsScope stopped_mutators_scope(thread);

runtime/vm/isolate.cc

@@ -374,8 +374,8 @@ class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
   // Returns `true` if this was the last isolate and the caller is responsible
   // for deleting the isolate group.
   bool UnregisterIsolateDecrementCount();
-  void RegisterIsolateGroupMutator(Thread* mutator);
-  void UnregisterIsolateGroupMutator(Thread* mutator);
+  void RegisterIsolateGroupMutator();
+  void UnregisterIsolateGroupMutator();
 
   bool ContainsOnlyOneIsolate();
 
@@ -650,8 +650,6 @@ class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
   Isolate* FirstIsolate() const;
   Isolate* FirstIsolateLocked() const;
 
-  void ForEachMutatorAtASafepoint(std::function<void(Thread* thread)> function);
-
   // Ensures mutators are stopped during execution of the provided function.
   //
   // If the current thread is the only mutator in the isolate group,
@@ -909,7 +907,6 @@ class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
   IntrusiveDList<Isolate> isolates_;
   RelaxedAtomic<Dart_Port> interrupt_port_ = ILLEGAL_PORT;
   intptr_t isolate_count_ = 0;
-  IntrusiveDList<Thread> mutators_;
   intptr_t group_mutator_count_ = 0;
   bool initial_spawn_successful_ = false;
   Dart_LibraryTagHandler library_tag_handler_ = nullptr;

runtime/vm/isolate.h

@@ -537,10 +537,10 @@ void Thread::EnterIsolateGroupAsMutator(IsolateGroup* isolate_group,
   isolate_group->IncreaseMutatorCount(/*thread=*/nullptr,
                                       /*is_nested_reenter=*/true,
                                       /*was_stolen=*/false);
+  isolate_group->RegisterIsolateGroupMutator();
+
   Thread* thread = AddActiveThread(isolate_group, /*isolate=*/nullptr,
                                    kMutatorTask, bypass_safepoint);
-  isolate_group->RegisterIsolateGroupMutator(thread);
-
   RELEASE_ASSERT(thread != nullptr);
   // Even if [bypass_safepoint] is true, a thread may need mutator state (e.g.
   // parallel scavenger threads write to the [Thread]s storebuffer)
@@ -584,7 +584,7 @@ void Thread::ExitIsolateGroupAsMutator(bool bypass_safepoint) {
   SuspendThreadInternal(thread, VMTag::kInvalidTagId);
   auto group = thread->isolate_group();
   FreeActiveThread(thread, /*isolate=*/nullptr, bypass_safepoint);
-  group->UnregisterIsolateGroupMutator(thread);
+  group->UnregisterIsolateGroupMutator();
   group->DecreaseMutatorCount(/*is_nested_exit=*/true);
 }
 

runtime/vm/thread.cc

@@ -23,7 +23,6 @@
 #include "vm/handles.h"
 #include "vm/heap/pointer_block.h"
 #include "vm/heap/sampler.h"
-#include "vm/intrusive_dlist.h"
 #include "vm/os_thread.h"
 #include "vm/pending_deopts.h"
 #include "vm/random.h"
@@ -365,7 +364,7 @@ class MutatorThreadVisitor {
 // a thread is allocated by ThreadRegistry::GetFromFreelistLocked either
 // before entering an isolate or entering an isolate group, and destroyed
 // automatically when the underlying OS thread exits.
-class Thread : public ThreadState, public IntrusiveDListEntry<Thread> {
+class Thread : public ThreadState {
  public:
   // The kind of task this thread is performing. Sampled by the profiler.
   enum TaskKind {