[vm/send_and_exit] Allow for safepoints when validating exit message.

``` before
$ dart isolate_exit_pause_time_test.dart
.Min(RunTimeRaw): 0.001 ms.
.Avg(RunTimeRaw): 212.399592 ms.
.Percentile50(RunTimeRaw): 49.055 ms.
.Percentile90(RunTimeRaw): 809.049 ms.
.Percentile95(RunTimeRaw): 1009.049 ms.
.Percentile99(RunTimeRaw): 1169.049 ms.
.Max(RunTimeRaw): 1208.049 ms.
```

``` after
$ dart isolate_exit_pause_time_test.dart
.Min(RunTimeRaw): 0.001 ms.
.Avg(RunTimeRaw): 53.87510125 ms.
.Percentile50(RunTimeRaw): 8.05 ms.
.Percentile90(RunTimeRaw): 174.005 ms.
.Percentile95(RunTimeRaw): 217.975 ms.
.Percentile99(RunTimeRaw): 275.033 ms.
.Max(RunTimeRaw): 314.033 ms.
```

Remaining latency after the change is due to large root set formed by pointers_to_verify_at_exit_ that have to be visited during GC.

Fixes #49050
TEST=ci

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

Author: aam

runtime/lib/isolate.cc

@@ -144,6 +144,17 @@ static ObjectPtr ValidateMessageObject(Zone* zone,
           visited_(visited),
           working_set_(working_set) {}
 
+    void VisitObject(ObjectPtr obj) {
+      if (!obj->IsHeapObject() || obj->untag()->IsCanonical()) {
+        return;
+      }
+      if (visited_->GetValueExclusive(obj) == 1) {
+        return;
+      }
+      visited_->SetValueExclusive(obj, 1);
+      working_set_->Add(obj);
+    }
+
    private:
     void VisitPointers(ObjectPtr* from, ObjectPtr* to) {
       for (ObjectPtr* ptr = from; ptr <= to; ptr++) {
@@ -159,17 +170,6 @@ static ObjectPtr ValidateMessageObject(Zone* zone,
       }
     }
 
-    void VisitObject(ObjectPtr obj) {
-      if (!obj->IsHeapObject() || obj->untag()->IsCanonical()) {
-        return;
-      }
-      if (visited_->GetValueExclusive(obj) == 1) {
-        return;
-      }
-      visited_->SetValueExclusive(obj, 1);
-      working_set_->Add(obj);
-    }
-
     WeakTable* visited_;
     MallocGrowableArray<ObjectPtr>* const working_set_;
   };
@@ -185,55 +185,76 @@ static ObjectPtr ValidateMessageObject(Zone* zone,
   Function& erroneous_closure_function = Function::Handle(zone);
   Class& erroneous_nativewrapper_class = Class::Handle(zone);
   Class& erroneous_finalizable_class = Class::Handle(zone);
+  Array& array = Array::Handle(zone);
   const char* error_message = nullptr;
+  Thread* thread = Thread::Current();
 
-  {
-    NoSafepointScope no_safepoint;
-    // working_set contains only elements that have not been visited yet that
-    // need to be processed.
-    // So before adding elements to working_set ensure to check visited flag,
-    // set visited flag at the same time as the element is added.
-    MallocGrowableArray<ObjectPtr> working_set;
-    std::unique_ptr<WeakTable> visited(new WeakTable());
-
-    SendMessageValidator visitor(isolate->group(), visited.get(), &working_set);
-
-    visited->SetValueExclusive(obj.ptr(), 1);
-    working_set.Add(obj.ptr());
-
-    while (!working_set.is_empty() && !error_found) {
-      ObjectPtr raw = working_set.RemoveLast();
-
-      const intptr_t cid = raw->GetClassId();
-      // Keep the list in sync with the one in runtime/vm/object_graph_copy.cc
-      switch (cid) {
-        // Can be shared.
-        case kOneByteStringCid:
-        case kTwoByteStringCid:
-        case kExternalOneByteStringCid:
-        case kExternalTwoByteStringCid:
-        case kMintCid:
-        case kImmutableArrayCid:
-        case kNeverCid:
-        case kSentinelCid:
-        case kInt32x4Cid:
-        case kSendPortCid:
-        case kCapabilityCid:
-        case kRegExpCid:
-        case kStackTraceCid:
-          continue;
-        // Cannot be shared due to possibly being mutable boxes for unboxed
-        // fields in JIT, but can be transferred via Isolate.exit()
-        case kDoubleCid:
-        case kFloat32x4Cid:
-        case kFloat64x2Cid:
-          continue;
-
-        case kClosureCid:
-          closure ^= raw;
-          // Only context has to be checked.
-          working_set.Add(closure.context());
-          continue;
+  // working_set contains only elements that have not been visited yet that
+  // need to be processed.
+  // So before adding elements to working_set ensure to check visited flag,
+  // set visited flag at the same time as the element is added.
+
+  // This working set of raw pointers is visited by GC, only one for a given
+  // isolate should be in use.
+  MallocGrowableArray<ObjectPtr>* const working_set =
+      isolate->pointers_to_verify_at_exit();
+  ASSERT(working_set->length() == 0);
+  std::unique_ptr<WeakTable> visited(new WeakTable());
+
+  SendMessageValidator visitor(isolate->group(), visited.get(), working_set);
+
+  visited->SetValueExclusive(obj.ptr(), 1);
+  working_set->Add(obj.ptr());
+
+  while (!working_set->is_empty() && !error_found) {
+    thread->CheckForSafepoint();
+
+    ObjectPtr raw = working_set->RemoveLast();
+
+    const intptr_t cid = raw->GetClassId();
+    // Keep the list in sync with the one in runtime/vm/object_graph_copy.cc
+    switch (cid) {
+      // Can be shared.
+      case kOneByteStringCid:
+      case kTwoByteStringCid:
+      case kExternalOneByteStringCid:
+      case kExternalTwoByteStringCid:
+      case kMintCid:
+      case kImmutableArrayCid:
+      case kNeverCid:
+      case kSentinelCid:
+      case kInt32x4Cid:
+      case kSendPortCid:
+      case kCapabilityCid:
+      case kRegExpCid:
+      case kStackTraceCid:
+        continue;
+      // Cannot be shared due to possibly being mutable boxes for unboxed
+      // fields in JIT, but can be transferred via Isolate.exit()
+      case kDoubleCid:
+      case kFloat32x4Cid:
+      case kFloat64x2Cid:
+        continue;
+
+      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) {
+            thread->CheckForSafepoint();
+          }
+        }
+        continue;
+      }
+      case kClosureCid:
+        closure ^= raw;
+        // Only context has to be checked.
+        working_set->Add(closure.context());
+        continue;
 
 #define MESSAGE_SNAPSHOT_ILLEGAL(type)                                         \
   case k##type##Cid:                                                           \
@@ -242,33 +263,32 @@ static ObjectPtr ValidateMessageObject(Zone* zone,
     error_found = true;                                                        \
     break;
 
-          MESSAGE_SNAPSHOT_ILLEGAL(DynamicLibrary);
-          // TODO(http://dartbug.com/47777): Send and exit support: remove this.
-          MESSAGE_SNAPSHOT_ILLEGAL(Finalizer);
-          MESSAGE_SNAPSHOT_ILLEGAL(NativeFinalizer);
-          MESSAGE_SNAPSHOT_ILLEGAL(MirrorReference);
-          MESSAGE_SNAPSHOT_ILLEGAL(Pointer);
-          MESSAGE_SNAPSHOT_ILLEGAL(ReceivePort);
-          MESSAGE_SNAPSHOT_ILLEGAL(UserTag);
-          MESSAGE_SNAPSHOT_ILLEGAL(SuspendState);
-
-        default:
-          if (cid >= kNumPredefinedCids) {
-            klass = class_table->At(cid);
-            if (klass.num_native_fields() != 0) {
-              erroneous_nativewrapper_class = klass.ptr();
-              error_found = true;
-              break;
-            }
-            if (klass.implements_finalizable()) {
-              erroneous_finalizable_class = klass.ptr();
-              error_found = true;
-              break;
-            }
+        MESSAGE_SNAPSHOT_ILLEGAL(DynamicLibrary);
+        // TODO(http://dartbug.com/47777): Send and exit support: remove this.
+        MESSAGE_SNAPSHOT_ILLEGAL(Finalizer);
+        MESSAGE_SNAPSHOT_ILLEGAL(NativeFinalizer);
+        MESSAGE_SNAPSHOT_ILLEGAL(MirrorReference);
+        MESSAGE_SNAPSHOT_ILLEGAL(Pointer);
+        MESSAGE_SNAPSHOT_ILLEGAL(ReceivePort);
+        MESSAGE_SNAPSHOT_ILLEGAL(UserTag);
+        MESSAGE_SNAPSHOT_ILLEGAL(SuspendState);
+
+      default:
+        if (cid >= kNumPredefinedCids) {
+          klass = class_table->At(cid);
+          if (klass.num_native_fields() != 0) {
+            erroneous_nativewrapper_class = klass.ptr();
+            error_found = true;
+            break;
           }
-      }
-      raw->untag()->VisitPointers(&visitor);
+          if (klass.implements_finalizable()) {
+            erroneous_finalizable_class = klass.ptr();
+            error_found = true;
+            break;
+          }
+        }
     }
+    raw->untag()->VisitPointers(&visitor);
   }
   if (error_found) {
     const char* exception_message;
@@ -292,9 +312,11 @@ static ObjectPtr ValidateMessageObject(Zone* zone,
                                       " : (object implements Finalizable - %s)",
                                       erroneous_finalizable_class.ToCString());
     }
+    working_set->Clear();
     return Exceptions::CreateUnhandledException(
         zone, Exceptions::kArgumentValue, exception_message);
   }
+  ASSERT(working_set->length() == 0);
   isolate->set_forward_table_new(nullptr);
   return obj.ptr();
 }

runtime/vm/isolate.cc

@@ -2823,6 +2823,11 @@ 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 IsolateGroup::ReleaseStoreBuffers() {

runtime/vm/isolate.h

@@ -1477,6 +1477,10 @@ class Isolate : public BaseIsolate, 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_;
+  }
+
  private:
   friend class Dart;                  // Init, InitOnce, Shutdown.
   friend class IsolateKillerVisitor;  // Kill().
@@ -1735,6 +1739,8 @@ class Isolate : public BaseIsolate, public IntrusiveDListEntry<Isolate> {
 
   ArrayPtr loaded_prefixes_set_storage_;
 
+  MallocGrowableArray<ObjectPtr> pointers_to_verify_at_exit_;
+
 #define REUSABLE_FRIEND_DECLARATION(name)                                      \
   friend class Reusable##name##HandleScope;
   REUSABLE_HANDLE_LIST(REUSABLE_FRIEND_DECLARATION)