[vm] Split isolate initialization logic into two pieces

This splits `Dart::InitializeIsolate()` into

* `Dart::InitializeIsolateGroup()` that sets up an `IsolateGroup`
  => This is only done at isolate group creation time when the very
     first isolate is created.

* `Dart::InitializeIsolate()` that sets up an `Isolate`.
  => This is done for every isolate.

This is purely refactoring / code cleanup.

TEST=ci

Change-Id: Ica906444f79fe49849b9e11e96f7c89184cb9d09
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/311603
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>

runtime/vm/dart.cc

@@ -448,7 +448,7 @@ char* Dart::DartInit(const Dart_InitializeParams* params) {
         OS::PrintErr("Size of vm isolate snapshot = %" Pd "\n",
                      snapshot->length());
         vm_isolate_group()->heap()->PrintSizes();
-        MegamorphicCacheTable::PrintSizes(vm_isolate_);
+        MegamorphicCacheTable::PrintSizes(T);
         intptr_t size;
         intptr_t capacity;
         Symbols::GetStats(vm_isolate_->group(), &size, &capacity);
@@ -738,7 +738,7 @@ char* Dart::Cleanup() {
   }
   OSThread::DisableOSThreadCreation();
 
-  ShutdownIsolate();
+  ShutdownIsolate(Thread::Current());
   vm_isolate_ = nullptr;
   ASSERT(Isolate::IsolateListLength() == 0);
   Service::Cleanup();
@@ -816,30 +816,24 @@ Isolate* Dart::CreateIsolate(const char* name_prefix,
   return isolate;
 }
 
-ErrorPtr Dart::InitIsolateFromSnapshot(Thread* T,
-                                       Isolate* I,
-                                       const uint8_t* snapshot_data,
-                                       const uint8_t* snapshot_instructions,
-                                       const uint8_t* kernel_buffer,
-                                       intptr_t kernel_buffer_size) {
-  auto IG = I->group();
-  if (kernel_buffer != nullptr) {
-    SafepointReadRwLocker reader(T, IG->program_lock());
-    I->field_table()->MarkReadyToUse();
-  }
-
+ErrorPtr Dart::InitIsolateGroupFromSnapshot(
+    Thread* T,
+    const uint8_t* snapshot_data,
+    const uint8_t* snapshot_instructions,
+    const uint8_t* kernel_buffer,
+    intptr_t kernel_buffer_size) {
+  auto IG = T->isolate_group();
   Error& error = Error::Handle(T->zone());
   error = Object::Init(IG, kernel_buffer, kernel_buffer_size);
   if (!error.IsNull()) {
     return error.ptr();
   }
-  if ((snapshot_data != nullptr) && kernel_buffer == nullptr) {
+  if (snapshot_data != nullptr && kernel_buffer == nullptr) {
     // Read the snapshot and setup the initial state.
 #if defined(SUPPORT_TIMELINE)
     TimelineBeginEndScope tbes(T, Timeline::GetIsolateStream(),
                                "ReadProgramSnapshot");
 #endif  // defined(SUPPORT_TIMELINE)
-    // TODO(turnidge): Remove once length is not part of the snapshot.
     const Snapshot* snapshot = Snapshot::SetupFromBuffer(snapshot_data);
     if (snapshot == nullptr) {
       const String& message = String::Handle(String::New("Invalid snapshot"));
@@ -861,12 +855,7 @@ ErrorPtr Dart::InitIsolateFromSnapshot(Thread* T,
       return error.ptr();
     }
 
-    T->SetupDartMutatorStateDependingOnSnapshot(I);
-    {
-      SafepointReadRwLocker reader(T, IG->program_lock());
-      I->set_field_table(T, IG->initial_field_table()->Clone(I));
-      I->field_table()->MarkReadyToUse();
-    }
+    T->SetupDartMutatorStateDependingOnSnapshot(IG);
 
 #if defined(SUPPORT_TIMELINE)
     if (tbes.enabled()) {
@@ -878,7 +867,7 @@ ErrorPtr Dart::InitIsolateFromSnapshot(Thread* T,
 #endif  // defined(SUPPORT_TIMELINE)
     if (FLAG_trace_isolates) {
       IG->heap()->PrintSizes();
-      MegamorphicCacheTable::PrintSizes(I);
+      MegamorphicCacheTable::PrintSizes(T);
     }
   } else {
     if ((vm_snapshot_kind_ != Snapshot::kNone) && kernel_buffer == nullptr) {
@@ -888,7 +877,7 @@ ErrorPtr Dart::InitIsolateFromSnapshot(Thread* T,
     }
   }
 #if !defined(PRODUCT) || defined(FORCE_INCLUDE_SAMPLING_HEAP_PROFILER)
-  I->group()->class_table()->PopulateUserVisibleNames();
+  IG->class_table()->PopulateUserVisibleNames();
 #endif
 
   return Error::null();
@@ -931,53 +920,22 @@ static void FinalizeBuiltinClasses(Thread* thread) {
   ENSURE_FINALIZED(ByteBuffer)
 }
 
-ErrorPtr Dart::InitializeIsolate(const uint8_t* snapshot_data,
-                                 const uint8_t* snapshot_instructions,
-                                 const uint8_t* kernel_buffer,
-                                 intptr_t kernel_buffer_size,
-                                 IsolateGroup* source_isolate_group,
-                                 void* isolate_data) {
-  // Initialize the new isolate.
-  Thread* T = Thread::Current();
-  Isolate* I = T->isolate();
-  auto IG = T->isolate_group();
-#if defined(SUPPORT_TIMELINE)
-  TimelineBeginEndScope tbes(T, Timeline::GetIsolateStream(),
-                             "InitializeIsolate");
-  tbes.SetNumArguments(1);
-  tbes.CopyArgument(0, "isolateName", I->name());
-#endif
-  ASSERT(I != nullptr);
-  StackZone zone(T);
-  HandleScope handle_scope(T);
-  bool was_child_cloned_into_existing_isolate = false;
-  if (source_isolate_group != nullptr) {
-    // If a static field gets registered in [IsolateGroup::RegisterStaticField]:
-    //
-    //   * before this block it will ignore this isolate. The [Clone] of the
-    //     initial field table will pick up the new value.
-    //   * after this block it will add the new static field to this isolate.
-    {
-      SafepointReadRwLocker reader(T, source_isolate_group->program_lock());
-      I->set_field_table(T,
-                         source_isolate_group->initial_field_table()->Clone(I));
-      I->field_table()->MarkReadyToUse();
-    }
-
-    was_child_cloned_into_existing_isolate = true;
-  } else {
-    const Error& error = Error::Handle(
-        InitIsolateFromSnapshot(T, I, snapshot_data, snapshot_instructions,
-                                kernel_buffer, kernel_buffer_size));
-    if (!error.IsNull()) {
-      return error.ptr();
-    }
+ErrorPtr Dart::InitializeIsolateGroup(Thread* T,
+                                      const uint8_t* snapshot_data,
+                                      const uint8_t* snapshot_instructions,
+                                      const uint8_t* kernel_buffer,
+                                      intptr_t kernel_buffer_size) {
+  auto& error = Error::Handle(
+      InitIsolateGroupFromSnapshot(T, snapshot_data, snapshot_instructions,
+                                   kernel_buffer, kernel_buffer_size));
+  if (!error.IsNull()) {
+    return error.ptr();
   }
 
   Object::VerifyBuiltinVtables();
-  if (T->isolate()->origin_id() == 0) {
-    DEBUG_ONLY(IG->heap()->Verify("InitializeIsolate", kForbidMarked));
-  }
+
+  auto IG = T->isolate_group();
+  DEBUG_ONLY(IG->heap()->Verify("InitializeIsolate", kForbidMarked));
 
 #if defined(DART_PRECOMPILED_RUNTIME)
   const bool kIsAotRuntime = true;
@@ -985,62 +943,79 @@ ErrorPtr Dart::InitializeIsolate(const uint8_t* snapshot_data,
   const bool kIsAotRuntime = false;
 #endif
 
-  if (kIsAotRuntime || was_child_cloned_into_existing_isolate) {
+  auto object_store = IG->object_store();
+  if (kIsAotRuntime) {
 #if !defined(TARGET_ARCH_IA32)
-    ASSERT(IG->object_store()->build_generic_method_extractor_code() !=
-           Code::null());
-    ASSERT(IG->object_store()->build_nongeneric_method_extractor_code() !=
+    ASSERT(object_store->build_generic_method_extractor_code() != Code::null());
+    ASSERT(object_store->build_nongeneric_method_extractor_code() !=
            Code::null());
 #endif
   } else {
     FinalizeBuiltinClasses(T);
 #if !defined(TARGET_ARCH_IA32)
-    if (I != Dart::vm_isolate()) {
-      if (IG->object_store()->build_generic_method_extractor_code() !=
-          nullptr) {
+    if (IG != Dart::vm_isolate_group()) {
+      if (object_store->build_generic_method_extractor_code() != nullptr ||
+          object_store->build_nongeneric_method_extractor_code() != nullptr) {
         SafepointWriteRwLocker ml(T, IG->program_lock());
-        if (IG->object_store()->build_generic_method_extractor_code() !=
-            nullptr) {
-          IG->object_store()->set_build_generic_method_extractor_code(
-              Code::Handle(
-                  StubCode::GetBuildGenericMethodExtractorStub(nullptr)));
+        if (object_store->build_generic_method_extractor_code() != nullptr) {
+          object_store->set_build_generic_method_extractor_code(Code::Handle(
+              StubCode::GetBuildGenericMethodExtractorStub(nullptr)));
         }
-      }
-      if (IG->object_store()->build_nongeneric_method_extractor_code() !=
-          nullptr) {
-        SafepointWriteRwLocker ml(T, IG->program_lock());
-        if (IG->object_store()->build_nongeneric_method_extractor_code() !=
-            nullptr) {
-          IG->object_store()->set_build_nongeneric_method_extractor_code(
-              Code::Handle(
-                  StubCode::GetBuildNonGenericMethodExtractorStub(nullptr)));
+        if (object_store->build_nongeneric_method_extractor_code() != nullptr) {
+          object_store->set_build_nongeneric_method_extractor_code(Code::Handle(
+              StubCode::GetBuildNonGenericMethodExtractorStub(nullptr)));
         }
       }
     }
 #endif  // !defined(TARGET_ARCH_IA32)
   }
 
-  Error& error = Error::Handle();
   if (snapshot_data == nullptr || kernel_buffer != nullptr) {
-    error ^= IG->object_store()->PreallocateObjects();
+    error ^= object_store->PreallocateObjects();
     if (!error.IsNull()) {
       return error.ptr();
     }
   }
+
+  if (FLAG_print_class_table) {
+    IG->class_table()->Print();
+  }
+
+  return Error::null();
+}
+
+ErrorPtr Dart::InitializeIsolate(Thread* T,
+                                 bool is_first_isolate_in_group,
+                                 void* isolate_data) {
+  auto I = T->isolate();
+  auto IG = T->isolate_group();
+  auto Z = T->zone();
+
+  // If a static field gets registered in [IsolateGroup::RegisterStaticField]:
+  //
+  //   * before this block it will ignore this isolate. The [Clone] of the
+  //     initial field table will pick up the new value.
+  //   * after this block it will add the new static field to this isolate.
+  {
+    SafepointReadRwLocker reader(T, IG->program_lock());
+    I->set_field_table(T, IG->initial_field_table()->Clone(I));
+    I->field_table()->MarkReadyToUse();
+  }
+
   const auto& out_of_memory =
       Object::Handle(IG->object_store()->out_of_memory());
-  error ^= I->isolate_object_store()->PreallocateObjects(out_of_memory);
+  const auto& error = Error::Handle(
+      Z, I->isolate_object_store()->PreallocateObjects(out_of_memory));
   if (!error.IsNull()) {
     return error.ptr();
   }
 
   I->set_init_callback_data(isolate_data);
-  if (FLAG_print_class_table) {
-    IG->class_table()->Print();
-  }
+
 #if !defined(PRODUCT)
-  ServiceIsolate::MaybeMakeServiceIsolate(I);
-  if (!Isolate::IsSystemIsolate(I)) {
+  if (Isolate::IsSystemIsolate(I)) {
+    ServiceIsolate::MaybeMakeServiceIsolate(I);
+  } else {
     I->message_handler()->set_should_pause_on_start(
         FLAG_pause_isolates_on_start);
     I->message_handler()->set_should_pause_on_exit(FLAG_pause_isolates_on_exit);
@@ -1189,18 +1164,8 @@ void Dart::RunShutdownCallback() {
   }
 }
 
-void Dart::ShutdownIsolate(Isolate* isolate) {
-  ASSERT(Isolate::Current() == nullptr);
-  // We need to enter the isolate in order to shut it down.
-  Thread::EnterIsolate(isolate);
-  ShutdownIsolate();
-  // Since the isolate is shutdown and deleted, there is no need to
-  // exit the isolate here.
-  ASSERT(Isolate::Current() == nullptr);
-}
-
-void Dart::ShutdownIsolate() {
-  Isolate::Current()->Shutdown();
+void Dart::ShutdownIsolate(Thread* T) {
+  T->isolate()->Shutdown();
 }
 
 bool Dart::VmIsolateNameEquals(const char* name) {

runtime/vm/dart.h

@@ -46,29 +46,30 @@ class Dart : public AllStatic {
                                 const Dart_IsolateFlags& api_flags,
                                 IsolateGroup* isolate_group);
 
-  // Initialize an isolate, either from a snapshot, from a Kernel binary, or
-  // from SDK library sources.  If the snapshot_buffer is non-null,
-  // initialize from a snapshot or a Kernel binary depending on the value of
-  // from_kernel.  Otherwise, initialize from sources.
-  static ErrorPtr InitializeIsolate(const uint8_t* snapshot_data,
-                                    const uint8_t* snapshot_instructions,
-                                    const uint8_t* kernel_buffer,
-                                    intptr_t kernel_buffer_size,
-                                    IsolateGroup* source_isolate_group,
-                                    void* data);
-  static ErrorPtr InitIsolateFromSnapshot(Thread* T,
-                                          Isolate* I,
-                                          const uint8_t* snapshot_data,
-                                          const uint8_t* snapshot_instructions,
-                                          const uint8_t* kernel_buffer,
-                                          intptr_t kernel_buffer_size);
+  // Initialize an isolate group either from a snapshot or from a Kernel binary.
+  static ErrorPtr InitializeIsolateGroup(Thread* T,
+                                         const uint8_t* snapshot_data,
+                                         const uint8_t* snapshot_instructions,
+                                         const uint8_t* kernel_buffer,
+                                         intptr_t kernel_buffer_size);
+  static ErrorPtr InitIsolateGroupFromSnapshot(
+      Thread* T,
+      const uint8_t* snapshot_data,
+      const uint8_t* snapshot_instructions,
+      const uint8_t* kernel_buffer,
+      intptr_t kernel_buffer_size);
+  static ErrorPtr InitializeIsolate(Thread* T,
+                                    bool is_first_isolate_in_group,
+                                    void* isolate_data);
 
   static void RunShutdownCallback();
-  static void ShutdownIsolate(Isolate* isolate);
-  static void ShutdownIsolate();
+  static void ShutdownIsolate(Thread* T);
 
   static Isolate* vm_isolate() { return vm_isolate_; }
-  static IsolateGroup* vm_isolate_group() { return vm_isolate_->group(); }
+  static IsolateGroup* vm_isolate_group() {
+    if (vm_isolate_ == nullptr) return nullptr;
+    return vm_isolate_->group();
+  }
   static ThreadPool* thread_pool() { return thread_pool_; }
   static bool VmIsolateNameEquals(const char* name);
 

runtime/vm/dart_api_impl.cc

@@ -1280,15 +1280,28 @@ static Dart_Isolate CreateIsolate(IsolateGroup* group,
   bool success = false;
   {
     StackZone zone(T);
+    HandleScope handle_scope(T);
+
+#if defined(SUPPORT_TIMELINE)
+    TimelineBeginEndScope tbes(T, Timeline::GetIsolateStream(),
+                               "InitializeIsolate");
+    tbes.SetNumArguments(1);
+    tbes.CopyArgument(0, "isolateName", I->name());
+#endif
+
     // We enter an API scope here as InitializeIsolate could compile some
     // bootstrap library files which call out to a tag handler that may create
     // Api Handles when an error is encountered.
     T->EnterApiScope();
-    const Error& error_obj = Error::Handle(
-        Z, Dart::InitializeIsolate(
-               source->snapshot_data, source->snapshot_instructions,
-               source->kernel_buffer, source->kernel_buffer_size,
-               is_new_group ? nullptr : group, isolate_data));
+    auto& error_obj = Error::Handle(Z);
+    if (is_new_group) {
+      error_obj = Dart::InitializeIsolateGroup(
+          T, source->snapshot_data, source->snapshot_instructions,
+          source->kernel_buffer, source->kernel_buffer_size);
+    }
+    if (error_obj.IsNull()) {
+      error_obj = Dart::InitializeIsolate(T, is_new_group, isolate_data);
+    }
     if (error_obj.IsNull()) {
 #if defined(DEBUG) && !defined(DART_PRECOMPILED_RUNTIME)
       if (FLAG_check_function_fingerprints && !FLAG_precompiled_mode) {
@@ -1316,7 +1329,7 @@ static Dart_Isolate CreateIsolate(IsolateGroup* group,
     return Api::CastIsolate(I);
   }
 
-  Dart::ShutdownIsolate();
+  Dart::ShutdownIsolate(T);
   return static_cast<Dart_Isolate>(nullptr);
 }
 
@@ -1485,7 +1498,7 @@ DART_EXPORT void Dart_ShutdownIsolate() {
 #endif
     Dart::RunShutdownCallback();
   }
-  Dart::ShutdownIsolate();
+  Dart::ShutdownIsolate(T);
 }
 
 DART_EXPORT Dart_Isolate Dart_CurrentIsolate() {

runtime/vm/isolate.cc

@@ -833,7 +833,7 @@ void IsolateGroup::ExitTemporaryIsolate() {
   Thread* thread = Thread::Current();
   ASSERT(thread != nullptr);
   thread->set_execution_state(Thread::kThreadInVM);
-  Dart::ShutdownIsolate();
+  Dart::ShutdownIsolate(thread);
 }
 
 void IsolateGroup::RehashConstants() {
@@ -2327,7 +2327,7 @@ void Isolate::LowLevelShutdown() {
 
 #if !defined(PRODUCT)
   if (FLAG_dump_megamorphic_stats) {
-    MegamorphicCacheTable::PrintSizes(this);
+    MegamorphicCacheTable::PrintSizes(thread);
   }
   if (FLAG_dump_symbol_stats) {
     Symbols::DumpStats(group());
@@ -2390,9 +2390,9 @@ void Isolate::Shutdown() {
 
   {
     StackZone zone(thread);
-    HandleScope handle_scope(thread);
     ServiceIsolate::SendIsolateShutdownMessage();
 #if !defined(PRODUCT)
+    HandleScope handle_scope(thread);
     debugger()->Shutdown();
     Profiler::IsolateShutdown(thread);
 #endif

runtime/vm/megamorphic_cache_table.cc

@@ -44,8 +44,7 @@ MegamorphicCachePtr MegamorphicCacheTable::Lookup(Thread* thread,
   return cache.ptr();
 }
 
-void MegamorphicCacheTable::PrintSizes(Isolate* isolate) {
-  auto thread = Thread::Current();
+void MegamorphicCacheTable::PrintSizes(Thread* thread) {
   auto isolate_group = thread->isolate_group();
   SafepointMutexLocker ml(isolate_group->megamorphic_table_mutex());
 

runtime/vm/megamorphic_cache_table.h

@@ -11,7 +11,6 @@
 namespace dart {
 
 class Array;
-class Isolate;
 class String;
 class Thread;
 
@@ -21,7 +20,7 @@ class MegamorphicCacheTable : public AllStatic {
                                     const String& name,
                                     const Array& descriptor);
 
-  static void PrintSizes(Isolate* isolate);
+  static void PrintSizes(Thread* thread);
 };
 
 }  // namespace dart

runtime/vm/service_isolate.cc

@@ -261,7 +261,7 @@ bool ServiceIsolate::SendIsolateShutdownMessage() {
     return false;
   }
 
-  Dart_Port main_port = Dart_GetMainPortId();
+  Dart_Port main_port = isolate->main_port();
   if (FLAG_trace_service) {
     OS::PrintErr(DART_VM_SERVICE_ISOLATE_NAME ": Isolate %s %" Pd64
                                               " deregistered.\n",

runtime/vm/thread.cc

@@ -1331,10 +1331,10 @@ void Thread::SetupDartMutatorState(Isolate* isolate) {
   field_table_values_ = isolate->field_table_->table();
   isolate->mutator_thread_ = this;
 
-  SetupDartMutatorStateDependingOnSnapshot(isolate);
+  SetupDartMutatorStateDependingOnSnapshot(isolate->group());
 }
 
-void Thread::SetupDartMutatorStateDependingOnSnapshot(Isolate* isolate) {
+void Thread::SetupDartMutatorStateDependingOnSnapshot(IsolateGroup* group) {
   // The snapshot may or may not have been read at this point (on isolate group
   // creation, the first isolate is first time entered before the snapshot is
   // read)
@@ -1342,7 +1342,6 @@ void Thread::SetupDartMutatorStateDependingOnSnapshot(Isolate* isolate) {
   // So we call this code explicitly after snapshot reading time and whenever we
   // enter an isolate with a new thread object.
 #if defined(DART_PRECOMPILED_RUNTIME)
-  auto group = isolate->group();
   auto object_store = group->object_store();
   if (object_store != nullptr) {
     global_object_pool_ = object_store->global_object_pool();

runtime/vm/thread.h

@@ -1410,7 +1410,7 @@ class Thread : public ThreadState {
   void ResetMutatorState();
 
   void SetupDartMutatorState(Isolate* isolate);
-  void SetupDartMutatorStateDependingOnSnapshot(Isolate* isolate);
+  void SetupDartMutatorStateDependingOnSnapshot(IsolateGroup* group);
   void ResetDartMutatorState(Isolate* isolate);
 
   static void SuspendThreadInternal(Thread* thread, VMTag::VMTagId tag);