[vm/shared] Move forward_table from Isolate to Thread.

Having forwarding tables on the Thread allows for those tables to be used in dart mutator thread running in IsolateGroup-shared context. On 32-bit platforms(arm) the forwarding tables are used during [SendPort.send] message verification.

Fixes #60817
TEST=isolate_group_shared_send_test

Change-Id: I58b33c14026584330b594776e812fe1d48bc2fd5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/431942
Reviewed-by: Ryan Macnak <[email protected]>
Commit-Queue: Alexander Aprelev <[email protected]>

Author: aam

runtime/lib/isolate.cc

@@ -260,15 +260,15 @@ class WorkSet {
 class WorkSet {
  public:
   explicit WorkSet(Thread* thread)
-      : isolate_(thread->isolate()), list_(GrowableObjectArray::Handle()) {
-    isolate_->set_forward_table_new(new WeakTable());
-    isolate_->set_forward_table_old(new WeakTable());
+      : thread_(thread), list_(GrowableObjectArray::Handle()) {
+    thread_->set_forward_table_new(new WeakTable());
+    thread_->set_forward_table_old(new WeakTable());
     list_ = GrowableObjectArray::New(256);
     cursor_ = 0;
   }
   ~WorkSet() {
-    isolate_->set_forward_table_new(nullptr);
-    isolate_->set_forward_table_old(nullptr);
+    thread_->set_forward_table_new(nullptr);
+    thread_->set_forward_table_old(nullptr);
   }
 
   void Push(const Object& obj) {
@@ -292,22 +292,22 @@ class WorkSet {
   DART_FORCE_INLINE
   intptr_t GetObjectId(ObjectPtr object) {
     if (object->IsNewObject()) {
-      return isolate_->forward_table_new()->GetValueExclusive(object);
+      return thread_->forward_table_new()->GetValueExclusive(object);
     } else {
-      return isolate_->forward_table_old()->GetValueExclusive(object);
+      return thread_->forward_table_old()->GetValueExclusive(object);
     }
   }
 
   DART_FORCE_INLINE
   void SetObjectId(ObjectPtr object, intptr_t id) {
     if (object->IsNewObject()) {
-      isolate_->forward_table_new()->SetValueExclusive(object, id);
+      thread_->forward_table_new()->SetValueExclusive(object, id);
     } else {
-      isolate_->forward_table_old()->SetValueExclusive(object, id);
+      thread_->forward_table_old()->SetValueExclusive(object, id);
     }
   }
 
-  Isolate* isolate_;
+  Thread* thread_;
   GrowableObjectArray& list_;
   intptr_t cursor_;
 };
@@ -475,15 +475,14 @@ class MessageValidator : private WorkSet {
                   const char* exception_message,
                   const Object& root) {
     Thread* thread = Thread::Current();
-    Isolate* isolate = thread->isolate();
     Zone* zone = thread->zone();
     const Array& args = Array::Handle(zone, Array::New(3));
     args.SetAt(0, illegal_object);
     args.SetAt(2, String::Handle(
                       zone, String::NewFormatted(
                                 "%s%s",
                                 FindRetainingPath(
-                                    zone, isolate, root, illegal_object,
+                                    zone, thread, root, illegal_object,
                                     TraversalRules::kInternalToIsolateGroup),
                                 exception_message)));
     const Object& exception = Object::Handle(

runtime/vm/heap/become_test.cc

@@ -95,34 +95,33 @@ ISOLATE_UNIT_TEST_CASE(BecomeForwardObjectId) {
 }
 
 ISOLATE_UNIT_TEST_CASE(BecomeForwardMessageId) {
-  Isolate* isolate = Isolate::Current();
-  isolate->set_forward_table_new(new WeakTable());
-  isolate->set_forward_table_old(new WeakTable());
+  thread->set_forward_table_new(new WeakTable());
+  thread->set_forward_table_old(new WeakTable());
 
   const Array& before_obj = Array::Handle(Array::New(0, Heap::kOld));
   const Array& after_obj = Array::Handle(Array::New(0, Heap::kOld));
   EXPECT(before_obj.ptr() != after_obj.ptr());
 
   intptr_t id = 42;
   intptr_t no_id = 0;
-  isolate->forward_table_old()->SetValueExclusive(before_obj.ptr(), id);
+  thread->forward_table_old()->SetValueExclusive(before_obj.ptr(), id);
   EXPECT_EQ(id,
-            isolate->forward_table_old()->GetValueExclusive(before_obj.ptr()));
+            thread->forward_table_old()->GetValueExclusive(before_obj.ptr()));
   EXPECT_EQ(no_id,
-            isolate->forward_table_old()->GetValueExclusive(after_obj.ptr()));
+            thread->forward_table_old()->GetValueExclusive(after_obj.ptr()));
 
   Become become;
   become.Add(before_obj, after_obj);
   become.Forward();
 
   EXPECT(before_obj.ptr() == after_obj.ptr());
   EXPECT_EQ(id,
-            isolate->forward_table_old()->GetValueExclusive(before_obj.ptr()));
+            thread->forward_table_old()->GetValueExclusive(before_obj.ptr()));
   EXPECT_EQ(id,
-            isolate->forward_table_old()->GetValueExclusive(after_obj.ptr()));
+            thread->forward_table_old()->GetValueExclusive(after_obj.ptr()));
 
-  isolate->set_forward_table_new(nullptr);
-  isolate->set_forward_table_old(nullptr);
+  thread->set_forward_table_new(nullptr);
+  thread->set_forward_table_old(nullptr);
 }
 
 ISOLATE_UNIT_TEST_CASE(BecomeForwardRememberedObject) {

runtime/vm/heap/heap.cc

@@ -943,23 +943,21 @@ void Heap::ForwardWeakEntries(ObjectPtr before_object, ObjectPtr after_object) {
     }
   }
 
-  isolate_group()->ForEachIsolate(
-      [&](Isolate* isolate) {
-        auto before_table = before_object->IsImmediateOrOldObject()
-                                ? isolate->forward_table_old()
-                                : isolate->forward_table_new();
-        if (before_table != nullptr) {
-          intptr_t entry = before_table->RemoveValueExclusive(before_object);
-          if (entry != 0) {
-            auto after_table = after_object->IsImmediateOrOldObject()
-                                   ? isolate->forward_table_old()
-                                   : isolate->forward_table_new();
-            ASSERT(after_table != nullptr);
-            after_table->SetValueExclusive(after_object, entry);
-          }
-        }
-      },
-      /*at_safepoint=*/true);
+  isolate_group()->thread_registry()->ForEachThread([&](Thread* thread) {
+    auto before_table = before_object->IsImmediateOrOldObject()
+                            ? thread->forward_table_old()
+                            : thread->forward_table_new();
+    if (before_table != nullptr) {
+      intptr_t entry = before_table->RemoveValueExclusive(before_object);
+      if (entry != 0) {
+        auto after_table = after_object->IsImmediateOrOldObject()
+                               ? thread->forward_table_old()
+                               : thread->forward_table_new();
+        ASSERT(after_table != nullptr);
+        after_table->SetValueExclusive(after_object, entry);
+      }
+    }
+  });
 }
 
 void Heap::ForwardWeakTables(ObjectPointerVisitor* visitor) {
@@ -972,12 +970,10 @@ void Heap::ForwardWeakTables(ObjectPointerVisitor* visitor) {
 
   // Isolates might have forwarding tables (used for during snapshotting in
   // isolate communication).
-  isolate_group()->ForEachIsolate(
-      [&](Isolate* isolate) {
-        auto table_old = isolate->forward_table_old();
-        if (table_old != nullptr) table_old->Forward(visitor);
-      },
-      /*at_safepoint=*/true);
+  isolate_group()->thread_registry()->ForEachThread([&](Thread* thread) {
+    auto table_old = thread->forward_table_old();
+    if (table_old != nullptr) table_old->Forward(visitor);
+  });
 }
 
 #ifndef PRODUCT

runtime/vm/heap/scavenger.cc

@@ -1459,19 +1459,17 @@ void Scavenger::MournWeakTables() {
     delete table;
   }
 
-  // Each isolate might have a weak table used for fast snapshot writing (i.e.
+  // Each thread  might have a weak table used for fast snapshot writing (i.e.
   // isolate communication). Rehash those tables if need be.
-  heap_->isolate_group()->ForEachIsolate(
-      [&](Isolate* isolate) {
-        auto table = isolate->forward_table_new();
-        if (table != nullptr) {
-          auto replacement = WeakTable::NewFrom(table);
-          rehash_weak_table(table, replacement, isolate->forward_table_old(),
-                            nullptr);
-          isolate->set_forward_table_new(replacement);
-        }
-      },
-      /*at_safepoint=*/true);
+  heap_->isolate_group()->thread_registry()->ForEachThread([&](Thread* thread) {
+    auto table = thread->forward_table_new();
+    if (table != nullptr) {
+      auto replacement = WeakTable::NewFrom(table);
+      rehash_weak_table(table, replacement, thread->forward_table_old(),
+                        nullptr);
+      thread->set_forward_table_new(replacement);
+    }
+  });
 }
 
 void Scavenger::Forward(MarkingStackBlock* reading) {

runtime/vm/isolate.cc

@@ -2595,15 +2595,6 @@ void IsolateGroup::MaybeIncreaseReloadEveryNStackOverflowChecks() {
 }
 #endif  // !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
 
-void Isolate::set_forward_table_new(WeakTable* table) {
-  std::unique_ptr<WeakTable> value(table);
-  forward_table_new_ = std::move(value);
-}
-void Isolate::set_forward_table_old(WeakTable* table) {
-  std::unique_ptr<WeakTable> value(table);
-  forward_table_old_ = std::move(value);
-}
-
 void Isolate::Shutdown() {
   Thread* thread = Thread::Current();
   ASSERT(this == thread->isolate());
@@ -2799,11 +2790,6 @@ void Isolate::VisitObjectPointers(ObjectPointerVisitor* visitor,
 
   visitor->VisitPointer(
       reinterpret_cast<ObjectPtr*>(&loaded_prefixes_set_storage_));
-
-  if (pointers_to_verify_at_exit_.length() != 0) {
-    visitor->VisitPointers(&pointers_to_verify_at_exit_[0],
-                           pointers_to_verify_at_exit_.length());
-  }
 }
 
 void Isolate::VisitStackPointers(ObjectPointerVisitor* visitor,

runtime/vm/isolate.h

@@ -85,7 +85,6 @@ class StoreBuffer;
 class StubCode;
 class ThreadRegistry;
 class UserTag;
-class WeakTable;
 
 class IsolateVisitor {
  public:
@@ -1461,14 +1460,6 @@ class Isolate : public IntrusiveDListEntry<Isolate> {
   }
   static bool IsVMInternalIsolate(const Isolate* isolate);
 
-  // The weak table used in the snapshot writer for the purpose of fast message
-  // sending.
-  WeakTable* forward_table_new() { return forward_table_new_.get(); }
-  void set_forward_table_new(WeakTable* table);
-
-  WeakTable* forward_table_old() { return forward_table_old_.get(); }
-  void set_forward_table_old(WeakTable* table);
-
   void RememberLiveTemporaries();
   void DeferredMarkLiveTemporaries();
 
@@ -1484,10 +1475,6 @@ class Isolate : public IntrusiveDListEntry<Isolate> {
   bool IsPrefixLoaded(const LibraryPrefix& prefix) const;
   void SetPrefixIsLoaded(const LibraryPrefix& prefix);
 
-  MallocGrowableArray<ObjectPtr>* pointers_to_verify_at_exit() {
-    return &pointers_to_verify_at_exit_;
-  }
-
   bool SetOwnerThread(ThreadId expected_old_owner, ThreadId new_owner);
 
   // Must be invoked with a valid PortMap::Locker, or while this isolate is the
@@ -1689,10 +1676,6 @@ class Isolate : public IntrusiveDListEntry<Isolate> {
   Monitor spawn_count_monitor_;
   intptr_t spawn_count_ = 0;
 
-  // Used during message sending of messages between isolates.
-  std::unique_ptr<WeakTable> forward_table_new_;
-  std::unique_ptr<WeakTable> forward_table_old_;
-
   // Signals whether the isolate can receive messages (e.g. KillAllIsolates can
   // send a kill message).
   // This is protected by [isolate_creation_monitor_].
@@ -1734,8 +1717,6 @@ class Isolate : public IntrusiveDListEntry<Isolate> {
 
   ArrayPtr loaded_prefixes_set_storage_;
 
-  MallocGrowableArray<ObjectPtr> pointers_to_verify_at_exit_;
-
   bool is_system_isolate_ = false;
 
 #define REUSABLE_FRIEND_DECLARATION(name)                                      \

runtime/vm/message_snapshot.cc

@@ -208,9 +208,9 @@ class MessageSerializer : public BaseSerializer {
     ASSERT(id != WeakTable::kNoValue);
     WeakTable* table;
     if (object->IsImmediateOrOldObject()) {
-      table = isolate()->forward_table_old();
+      table = thread()->forward_table_old();
     } else {
-      table = isolate()->forward_table_new();
+      table = thread()->forward_table_new();
     }
     return table->MarkValueExclusive(object, id);
   }
@@ -219,19 +219,19 @@ class MessageSerializer : public BaseSerializer {
     ASSERT(id != WeakTable::kNoValue);
     WeakTable* table;
     if (object->IsImmediateOrOldObject()) {
-      table = isolate()->forward_table_old();
+      table = thread()->forward_table_old();
     } else {
-      table = isolate()->forward_table_new();
+      table = thread()->forward_table_new();
     }
     table->SetValueExclusive(object, id);
   }
 
   intptr_t GetObjectId(ObjectPtr object) const {
     const WeakTable* table;
     if (object->IsImmediateOrOldObject()) {
-      table = isolate()->forward_table_old();
+      table = thread()->forward_table_old();
     } else {
-      table = isolate()->forward_table_new();
+      table = thread()->forward_table_new();
     }
     return table->GetValueExclusive(object);
   }
@@ -264,14 +264,14 @@ class MessageSerializer : public BaseSerializer {
   Thread* thread() const {
     return static_cast<Thread*>(StackResource::thread());
   }
-  Isolate* isolate() const { return thread()->isolate(); }
   IsolateGroup* isolate_group() const { return thread()->isolate_group(); }
 
   bool HasRef(ObjectPtr object) const {
     return GetObjectId(object) != WeakTable::kNoValue;
   }
 
  private:
+  Thread* thread_;
   WeakTable* forward_table_new_;
   WeakTable* forward_table_old_;
   GrowableArray<Object*> stack_;
@@ -2705,13 +2705,13 @@ MessageSerializer::MessageSerializer(Thread* thread)
       forward_table_new_(),
       forward_table_old_(),
       stack_(thread->zone(), 0) {
-  isolate()->set_forward_table_new(new WeakTable());
-  isolate()->set_forward_table_old(new WeakTable());
+  thread->set_forward_table_new(new WeakTable());
+  thread->set_forward_table_old(new WeakTable());
 }
 
 MessageSerializer::~MessageSerializer() {
-  isolate()->set_forward_table_new(nullptr);
-  isolate()->set_forward_table_old(nullptr);
+  thread()->set_forward_table_new(nullptr);
+  thread()->set_forward_table_old(nullptr);
 }
 
 ApiMessageSerializer::ApiMessageSerializer(Zone* zone)
@@ -2757,7 +2757,7 @@ void MessageSerializer::Trace(const Object& root, Object* object) {
       // as they are not going to be used again here.
       const char* message = OS::SCreate(
           zone_, "is a regular instance reachable via %s",
-          FindRetainingPath(zone_, isolate(), root, *object,
+          FindRetainingPath(zone_, thread(), root, *object,
                             TraversalRules::kExternalBetweenIsolateGroups));
       IllegalObject(*object, message);
     }
@@ -2802,7 +2802,7 @@ void MessageSerializer::Trace(const Object& root, Object* object) {
       // as they are not going to be used again here.
       const char* message = OS::SCreate(
           zone_, "is a %s reachable via %s", illegal_cid_string,
-          FindRetainingPath(zone_, isolate(), root, *object,
+          FindRetainingPath(zone_, thread(), root, *object,
                             TraversalRules::kExternalBetweenIsolateGroups));
       IllegalObject(*object, message);
     }