Don't use heap iteration order when seeding the VM isolate.

Bug: https://github.com/dart-lang/sdk/issues/31427
Change-Id: I74c5c9f6e992bf68913178a49916892c0ef808c9
Reviewed-on: https://dart-review.googlesource.com/26440
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>

runtime/vm/clustered_snapshot.cc

@@ -12,6 +12,7 @@
 #include "vm/native_entry.h"
 #include "vm/object.h"
 #include "vm/object_store.h"
+#include "vm/program_visitor.h"
 #include "vm/stub_code.h"
 #include "vm/symbols.h"
 #include "vm/timeline.h"
@@ -4797,8 +4798,8 @@ void Serializer::Push(RawObject* object) {
     num_written_objects_++;
 
 #if defined(SNAPSHOT_BACKTRACE)
-    parent_pairs_.Add(&Object::Handle(object));
-    parent_pairs_.Add(&Object::Handle(current_parent_));
+    parent_pairs_.Add(&Object::Handle(zone_, object));
+    parent_pairs_.Add(&Object::Handle(zone_, current_parent_));
 #endif
   }
 }
@@ -5553,37 +5554,73 @@ void Deserializer::ReadIsolateSnapshot(ObjectStore* object_store) {
   Bootstrap::SetupNativeResolver();
 }
 
-// An object visitor which iterates the heap looking for objects to write into
+// Iterates the program structure looking for objects to write into
 // the VM isolate's snapshot, causing them to be shared across isolates.
-class SeedVMIsolateVisitor : public ObjectVisitor {
+// Duplicates will be removed by Serializer::Push.
+class SeedVMIsolateVisitor : public ClassVisitor, public FunctionVisitor {
  public:
   SeedVMIsolateVisitor(Zone* zone, bool include_code)
       : zone_(zone),
         include_code_(include_code),
         objects_(new (zone) ZoneGrowableArray<Object*>(4 * KB)),
-        code_(new (zone) ZoneGrowableArray<Code*>(4 * KB)) {}
+        codes_(new (zone) ZoneGrowableArray<Code*>(4 * KB)),
+        script_(Script::Handle(zone)),
+        code_(Code::Handle(zone)),
+        stack_maps_(Array::Handle(zone)) {}
 
-  void VisitObject(RawObject* obj) {
-    if (obj->IsTokenStream()) {
-      objects_->Add(&Object::Handle(zone_, obj));
-    } else if (include_code_) {
-      if (obj->IsStackMap() || obj->IsPcDescriptors() ||
-          obj->IsCodeSourceMap()) {
-        objects_->Add(&Object::Handle(zone_, obj));
-      } else if (obj->IsCode()) {
-        code_->Add(&Code::Handle(zone_, Code::RawCast(obj)));
+  void Visit(const Class& cls) {
+    script_ = cls.script();
+    if (!script_.IsNull()) {
+      objects_->Add(&Object::Handle(zone_, script_.tokens()));
+    }
+
+    if (!include_code_) return;
+
+    code_ = cls.allocation_stub();
+    Visit(code_);
+  }
+
+  void Visit(const Function& function) {
+    script_ = function.script();
+    if (!script_.IsNull()) {
+      objects_->Add(&Object::Handle(zone_, script_.tokens()));
+    }
+
+    if (!include_code_) return;
+
+    code_ = function.CurrentCode();
+    Visit(code_);
+    code_ = function.unoptimized_code();
+    Visit(code_);
+  }
+
+  ZoneGrowableArray<Object*>* objects() { return objects_; }
+  ZoneGrowableArray<Code*>* codes() { return codes_; }
+
+ private:
+  void Visit(const Code& code) {
+    ASSERT(include_code_);
+    if (code.IsNull()) return;
+
+    codes_->Add(&Code::Handle(zone_, code.raw()));
+    objects_->Add(&Object::Handle(zone_, code.pc_descriptors()));
+    objects_->Add(&Object::Handle(zone_, code.code_source_map()));
+
+    stack_maps_ = code_.stackmaps();
+    if (!stack_maps_.IsNull()) {
+      for (intptr_t i = 0; i < stack_maps_.Length(); i++) {
+        objects_->Add(&Object::Handle(zone_, stack_maps_.At(i)));
       }
     }
   }
 
-  ZoneGrowableArray<Object*>* objects() { return objects_; }
-  ZoneGrowableArray<Code*>* code() { return code_; }
-
- private:
   Zone* zone_;
   bool include_code_;
   ZoneGrowableArray<Object*>* objects_;
-  ZoneGrowableArray<Code*>* code_;
+  ZoneGrowableArray<Code*>* codes_;
+  Script& script_;
+  Code& code_;
+  Array& stack_maps_;
 };
 
 FullSnapshotWriter::FullSnapshotWriter(Snapshot::Kind kind,
@@ -5629,13 +5666,12 @@ FullSnapshotWriter::FullSnapshotWriter(Snapshot::Kind kind,
     NOT_IN_PRODUCT(TimelineDurationScope tds(
         thread(), Timeline::GetIsolateStream(), "PrepareNewVMIsolate"));
 
-    HeapIterationScope iteration(thread());
     SeedVMIsolateVisitor visitor(thread()->zone(),
                                  Snapshot::IncludesCode(kind));
-    iteration.IterateObjects(&visitor);
-    iteration.IterateVMIsolateObjects(&visitor);
+    ProgramVisitor::VisitClasses(&visitor);
+    ProgramVisitor::VisitFunctions(&visitor);
     seed_objects_ = visitor.objects();
-    seed_code_ = visitor.code();
+    seed_code_ = visitor.codes();
 
     // Tuck away the current symbol table.
     saved_symbol_table_ = object_store->symbol_table();

runtime/vm/isolate.cc

@@ -84,9 +84,6 @@ static void DeterministicModeHandler(bool value) {
   if (value) {
     FLAG_background_compilation = false;
     FLAG_collect_code = false;
-    // Parallel marking doesn't introduce non-determinism in the object
-    // iteration order.
-    FLAG_concurrent_sweep = false;
     FLAG_random_seed = 0x44617274;  // "Dart"
 #if !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
     FLAG_load_deferred_eagerly = true;

runtime/vm/object.h

@@ -1555,7 +1555,7 @@ class Class : public Object {
   friend class Object;
   friend class Type;
   friend class Intrinsifier;
-  friend class ProgramVisitor;
+  friend class ClassFunctionVisitor;
 };
 
 // Unresolved class is used for storing unresolved names which will be resolved

runtime/vm/program_visitor.cc

@@ -20,6 +20,8 @@ void ProgramVisitor::VisitClasses(ClassVisitor* visitor) {
       GrowableObjectArray::Handle(zone, isolate->object_store()->libraries());
   Library& lib = Library::Handle(zone);
   Class& cls = Class::Handle(zone);
+  Object& entry = Object::Handle(zone);
+  GrowableObjectArray& patches = GrowableObjectArray::Handle(zone);
 
   for (intptr_t i = 0; i < libraries.Length(); i++) {
     lib ^= libraries.At(i);
@@ -31,64 +33,82 @@ void ProgramVisitor::VisitClasses(ClassVisitor* visitor) {
       }
       visitor->Visit(cls);
     }
+    patches = lib.patch_classes();
+    for (intptr_t j = 0; j < patches.Length(); j++) {
+      entry = patches.At(j);
+      if (entry.IsClass()) {
+        visitor->Visit(Class::Cast(entry));
+      }
+    }
   }
 }
 
+class ClassFunctionVisitor : public ClassVisitor {
+ public:
+  ClassFunctionVisitor(Zone* zone, FunctionVisitor* visitor)
+      : visitor_(visitor),
+        functions_(Array::Handle(zone)),
+        function_(Function::Handle(zone)),
+        object_(Object::Handle(zone)),
+        fields_(Array::Handle(zone)),
+        field_(Field::Handle(zone)) {}
+
+  void Visit(const Class& cls) {
+    if (cls.IsDynamicClass()) {
+      return;  // class 'dynamic' is in the read-only VM isolate.
+    }
+
+    functions_ = cls.functions();
+    for (intptr_t j = 0; j < functions_.Length(); j++) {
+      function_ ^= functions_.At(j);
+      visitor_->Visit(function_);
+      if (function_.HasImplicitClosureFunction()) {
+        function_ = function_.ImplicitClosureFunction();
+        visitor_->Visit(function_);
+      }
+    }
+
+    functions_ = cls.invocation_dispatcher_cache();
+    for (intptr_t j = 0; j < functions_.Length(); j++) {
+      object_ = functions_.At(j);
+      if (object_.IsFunction()) {
+        function_ ^= functions_.At(j);
+        visitor_->Visit(function_);
+      }
+    }
+
+    fields_ = cls.fields();
+    for (intptr_t j = 0; j < fields_.Length(); j++) {
+      field_ ^= fields_.At(j);
+      if (field_.is_static() && field_.HasPrecompiledInitializer()) {
+        function_ ^= field_.PrecompiledInitializer();
+        visitor_->Visit(function_);
+      }
+    }
+  }
+
+ private:
+  FunctionVisitor* visitor_;
+  Array& functions_;
+  Function& function_;
+  Object& object_;
+  Array& fields_;
+  Field& field_;
+};
+
 void ProgramVisitor::VisitFunctions(FunctionVisitor* visitor) {
   Thread* thread = Thread::Current();
   Isolate* isolate = thread->isolate();
   Zone* zone = thread->zone();
-  GrowableObjectArray& libraries =
-      GrowableObjectArray::Handle(zone, isolate->object_store()->libraries());
-  Library& lib = Library::Handle(zone);
-  Class& cls = Class::Handle(zone);
-  Array& functions = Array::Handle(zone);
-  Array& fields = Array::Handle(zone);
-  Field& field = Field::Handle(zone);
-  Object& object = Object::Handle(zone);
+
+  ClassFunctionVisitor class_visitor(zone, visitor);
+  VisitClasses(&class_visitor);
+
   Function& function = Function::Handle(zone);
-  GrowableObjectArray& closures = GrowableObjectArray::Handle(zone);
-
-  for (intptr_t i = 0; i < libraries.Length(); i++) {
-    lib ^= libraries.At(i);
-    ClassDictionaryIterator it(lib, ClassDictionaryIterator::kIteratePrivate);
-    while (it.HasNext()) {
-      cls = it.GetNextClass();
-      if (cls.IsDynamicClass()) {
-        continue;  // class 'dynamic' is in the read-only VM isolate.
-      }
-
-      functions = cls.functions();
-      for (intptr_t j = 0; j < functions.Length(); j++) {
-        function ^= functions.At(j);
-        visitor->Visit(function);
-        if (function.HasImplicitClosureFunction()) {
-          function = function.ImplicitClosureFunction();
-          visitor->Visit(function);
-        }
-      }
-
-      functions = cls.invocation_dispatcher_cache();
-      for (intptr_t j = 0; j < functions.Length(); j++) {
-        object = functions.At(j);
-        if (object.IsFunction()) {
-          function ^= functions.At(j);
-          visitor->Visit(function);
-        }
-      }
-      fields = cls.fields();
-      for (intptr_t j = 0; j < fields.Length(); j++) {
-        field ^= fields.At(j);
-        if (field.is_static() && field.HasPrecompiledInitializer()) {
-          function ^= field.PrecompiledInitializer();
-          visitor->Visit(function);
-        }
-      }
-    }
-  }
-  closures = isolate->object_store()->closure_functions();
-  for (intptr_t j = 0; j < closures.Length(); j++) {
-    function ^= closures.At(j);
+  const GrowableObjectArray& closures = GrowableObjectArray::Handle(
+      zone, isolate->object_store()->closure_functions());
+  for (intptr_t i = 0; i < closures.Length(); i++) {
+    function ^= closures.At(i);
     visitor->Visit(function);
     ASSERT(!function.HasImplicitClosureFunction());
   }