Avoid reloading the kernel snapshot when spawning an isolate in the same group

runtime/dart_isolate.cc

@@ -196,50 +196,56 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(
     const DartIsolate* spawning_isolate) {
   TRACE_EVENT0("flutter", "DartIsolate::CreateRootIsolate");
 
-  // The child isolate preparer is null but will be set when the isolate is
-  // being prepared to run.
-  auto isolate_group_data =
-      std::make_unique<std::shared_ptr<DartIsolateGroupData>>(
-          std::shared_ptr<DartIsolateGroupData>(new DartIsolateGroupData(
-              settings,                            // settings
-              std::move(isolate_snapshot),         // isolate snapshot
-              context.advisory_script_uri,         // advisory URI
-              context.advisory_script_entrypoint,  // advisory entrypoint
-              nullptr,                             // child isolate preparer
-              isolate_create_callback,             // isolate create callback
-              isolate_shutdown_callback            // isolate shutdown callback
-              )));
+  // Only needed if this is the main isolate for the group.
+  std::unique_ptr<std::shared_ptr<DartIsolateGroupData>> isolate_group_data;
 
   auto isolate_data = std::make_unique<std::shared_ptr<DartIsolate>>(
-      std::shared_ptr<DartIsolate>(new DartIsolate(settings,  // settings
-                                                   true,      // is_root_isolate
-                                                   context    // context
-                                                   )));
+      std::shared_ptr<DartIsolate>(new DartIsolate(
+          /*settings=*/settings,
+          /*is_root_isolate=*/true,
+          /*context=*/context,
+          /*is_spawning_in_group=*/!!spawning_isolate)));
 
   DartErrorString error;
   Dart_Isolate vm_isolate = nullptr;
   auto isolate_flags = flags.Get();
 
   IsolateMaker isolate_maker;
   if (spawning_isolate) {
-    isolate_maker = [spawning_isolate](
-                        std::shared_ptr<DartIsolateGroupData>*
-                            isolate_group_data,
-                        std::shared_ptr<DartIsolate>* isolate_data,
-                        Dart_IsolateFlags* flags, char** error) {
-      return Dart_CreateIsolateInGroup(
-          /*group_member=*/spawning_isolate->isolate(),
-          /*name=*/(*isolate_group_data)->GetAdvisoryScriptEntrypoint().c_str(),
-          /*shutdown_callback=*/
-          reinterpret_cast<Dart_IsolateShutdownCallback>(
-              DartIsolate::SpawnIsolateShutdownCallback),
-          /*cleanup_callback=*/
-          reinterpret_cast<Dart_IsolateCleanupCallback>(
-              DartIsolateCleanupCallback),
-          /*child_isolate_data=*/isolate_data,
-          /*error=*/error);
-    };
+    isolate_maker =
+        [spawning_isolate](
+            std::shared_ptr<DartIsolateGroupData>* isolate_group_data,
+            std::shared_ptr<DartIsolate>* isolate_data,
+            Dart_IsolateFlags* flags, char** error) {
+          return Dart_CreateIsolateInGroup(
+              /*group_member=*/spawning_isolate->isolate(),
+              /*name=*/
+              spawning_isolate->GetIsolateGroupData()
+                  .GetAdvisoryScriptEntrypoint()
+                  .c_str(),
+              /*shutdown_callback=*/
+              reinterpret_cast<Dart_IsolateShutdownCallback>(
+                  DartIsolate::SpawnIsolateShutdownCallback),
+              /*cleanup_callback=*/
+              reinterpret_cast<Dart_IsolateCleanupCallback>(
+                  DartIsolateCleanupCallback),
+              /*child_isolate_data=*/isolate_data,
+              /*error=*/error);
+        };
   } else {
+    // The child isolate preparer is null but will be set when the isolate is
+    // being prepared to run.
+    isolate_group_data =
+        std::make_unique<std::shared_ptr<DartIsolateGroupData>>(
+            std::shared_ptr<DartIsolateGroupData>(new DartIsolateGroupData(
+                settings,                            // settings
+                std::move(isolate_snapshot),         // isolate snapshot
+                context.advisory_script_uri,         // advisory URI
+                context.advisory_script_entrypoint,  // advisory entrypoint
+                nullptr,                             // child isolate preparer
+                isolate_create_callback,             // isolate create callback
+                isolate_shutdown_callback  // isolate shutdown callback
+                )));
     isolate_maker = [](std::shared_ptr<DartIsolateGroupData>*
                            isolate_group_data,
                        std::shared_ptr<DartIsolate>* isolate_data,
@@ -276,7 +282,8 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(
 
 DartIsolate::DartIsolate(const Settings& settings,
                          bool is_root_isolate,
-                         const UIDartState::Context& context)
+                         const UIDartState::Context& context,
+                         bool is_spawning_in_group)
     : UIDartState(settings.task_observer_add,
                   settings.task_observer_remove,
                   settings.log_tag,
@@ -287,7 +294,8 @@ DartIsolate::DartIsolate(const Settings& settings,
                   context),
       may_insecurely_connect_to_all_domains_(
           settings.may_insecurely_connect_to_all_domains),
-      domain_network_policy_(settings.domain_network_policy) {
+      domain_network_policy_(settings.domain_network_policy),
+      is_spawning_in_group_(is_spawning_in_group) {
   phase_ = Phase::Uninitialized;
 }
 
@@ -548,13 +556,13 @@ bool DartIsolate::LoadKernel(const std::shared_ptr<const fml::Mapping>& mapping,
     return false;
   }
 
-  if (!mapping || mapping->GetSize() == 0) {
-    return false;
-  }
-
   tonic::DartState::Scope scope(this);
 
-  if (!child_isolate) {
+  if (!child_isolate && !is_spawning_in_group_) {
+    if (!mapping || mapping->GetSize() == 0) {
+      return false;
+    }
+
     // Use root library provided by kernel in favor of one provided by snapshot.
     Dart_SetRootLibrary(Dart_Null());
 
@@ -923,7 +931,7 @@ Dart_Isolate DartIsolate::DartIsolateGroupCreateCallback(
       });
 
   if (*error) {
-    FML_LOG(ERROR) << "CreateDartIsolateGroup failed: " << error;
+    FML_LOG(ERROR) << "CreateDartIsolateGroup failed: " << *error;
   }
 
   return vm_isolate;

runtime/dart_isolate.h

@@ -20,6 +20,7 @@
 #include "flutter/lib/ui/ui_dart_state.h"
 #include "flutter/lib/ui/window/platform_configuration.h"
 #include "flutter/runtime/dart_snapshot.h"
+#include "runtime/isolate_configuration.h"
 #include "third_party/dart/runtime/include/dart_api.h"
 #include "third_party/tonic/dart_state.h"
 
@@ -412,6 +413,7 @@ class DartIsolate : public UIDartState {
   fml::RefPtr<fml::TaskRunner> message_handling_task_runner_;
   const bool may_insecurely_connect_to_all_domains_;
   std::string domain_network_policy_;
+  const bool is_spawning_in_group_;
 
   static std::weak_ptr<DartIsolate> CreateRootIsolate(
       const Settings& settings,
@@ -425,7 +427,8 @@ class DartIsolate : public UIDartState {
 
   DartIsolate(const Settings& settings,
               bool is_root_isolate,
-              const UIDartState::Context& context);
+              const UIDartState::Context& context,
+              bool is_spawning_in_group = false);
 
   //----------------------------------------------------------------------------
   /// @brief      Initializes the given (current) isolate.

runtime/dart_isolate_unittests.cc

@@ -592,6 +592,112 @@ TEST_F(DartIsolateTest, DartPluginRegistrantIsCalled) {
   ASSERT_EQ(messages[0], "_PluginRegistrant.register() was called");
 }
 
+TEST_F(DartIsolateTest, SpawningAnIsolateDoesNotReloadKernel) {
+  ASSERT_FALSE(DartVMRef::IsInstanceRunning());
+  auto settings = CreateSettingsForFixture();
+  auto vm_ref = DartVMRef::Create(settings);
+  ASSERT_TRUE(vm_ref);
+  auto vm_data = vm_ref.GetVMData();
+  ASSERT_TRUE(vm_data);
+  TaskRunners task_runners(GetCurrentTestName(),    //
+                           GetCurrentTaskRunner(),  //
+                           GetCurrentTaskRunner(),  //
+                           GetCurrentTaskRunner(),  //
+                           GetCurrentTaskRunner()   //
+  );
+
+  size_t get_kernel_count = 0u;
+  if (!DartVM::IsRunningPrecompiledCode()) {
+    ASSERT_TRUE(settings.application_kernels);
+    auto mappings = settings.application_kernels();
+    ASSERT_EQ(mappings.size(), 1u);
+
+    // This feels a little brittle, but the alternative seems to be making
+    // DartIsolate have virtual methods so it can be mocked or exposing weird
+    // test-only API on IsolateConfiguration.
+    settings
+        .application_kernels = fml::MakeCopyable([&get_kernel_count,
+                                                  mapping = std::move(
+                                                      mappings
+                                                          .front())]() mutable
+                                                 -> Mappings {
+      get_kernel_count++;
+      if (get_kernel_count > 1) {
+        FML_LOG(ERROR)
+            << "Unexpectedly got more than one call for the kernel mapping.";
+        abort();
+      }
+      std::vector<std::unique_ptr<const fml::Mapping>> kernel_mappings;
+      kernel_mappings.emplace_back(std::move(mapping));
+      return kernel_mappings;
+    });
+  }
+
+  std::shared_ptr<DartIsolate> root_isolate;
+  {
+    auto isolate_configuration =
+        IsolateConfiguration::InferFromSettings(settings);
+
+    UIDartState::Context context(task_runners);
+    context.advisory_script_uri = "main.dart";
+    context.advisory_script_entrypoint = "main";
+    auto weak_isolate = DartIsolate::CreateRunningRootIsolate(
+        /*settings=*/vm_data->GetSettings(),
+        /*isolate_snapshot=*/vm_data->GetIsolateSnapshot(),
+        /*platform_configuration=*/nullptr,
+        /*flags=*/DartIsolate::Flags{},
+        /*root_isolate_create_callback=*/nullptr,
+        /*isolate_create_callback=*/settings.isolate_create_callback,
+        /*isolate_shutdown_callback=*/settings.isolate_shutdown_callback,
+        /*dart_entrypoint=*/"main",
+        /*dart_entrypoint_library=*/std::nullopt,
+        /*dart_entrypoint_args=*/{},
+        /*isolate_configuration=*/std::move(isolate_configuration),
+        /*context=*/context);
+    root_isolate = weak_isolate.lock();
+  }
+  ASSERT_TRUE(root_isolate);
+  ASSERT_EQ(root_isolate->GetPhase(), DartIsolate::Phase::Running);
+  if (!DartVM::IsRunningPrecompiledCode()) {
+    ASSERT_EQ(get_kernel_count, 1u);
+  }
+
+  {
+    auto isolate_configuration = IsolateConfiguration::InferFromSettings(
+        /*settings=*/settings,
+        /*asset_manager=*/nullptr,
+        /*io_worker=*/nullptr,
+        /*launch_type=*/IsolateLaunchType::kExistingGroup);
+
+    UIDartState::Context context(task_runners);
+    context.advisory_script_uri = "main.dart";
+    context.advisory_script_entrypoint = "main";
+    auto weak_isolate = DartIsolate::CreateRunningRootIsolate(
+        /*settings=*/vm_data->GetSettings(),
+        /*isolate_snapshot=*/vm_data->GetIsolateSnapshot(),
+        /*platform_configuration=*/nullptr,
+        /*flags=*/DartIsolate::Flags{},
+        /*root_isolate_create_callback=*/nullptr,
+        /*isolate_create_callback=*/settings.isolate_create_callback,
+        /*isolate_shutdown_callback=*/settings.isolate_shutdown_callback,
+        /*dart_entrypoint=*/"main",
+        /*dart_entrypoint_library=*/std::nullopt,
+        /*dart_entrypoint_args=*/{},
+        /*isolate_configuration=*/std::move(isolate_configuration),
+        /*context=*/context,
+        /*spawning_isolate=*/root_isolate.get());
+    auto spawned_isolate = weak_isolate.lock();
+    ASSERT_TRUE(spawned_isolate);
+    ASSERT_EQ(spawned_isolate->GetPhase(), DartIsolate::Phase::Running);
+    if (!DartVM::IsRunningPrecompiledCode()) {
+      ASSERT_EQ(get_kernel_count, 1u);
+    }
+    ASSERT_TRUE(spawned_isolate->Shutdown());
+  }
+
+  ASSERT_TRUE(root_isolate->Shutdown());
+}
+
 }  // namespace testing
 }  // namespace flutter
 

runtime/isolate_configuration.cc

@@ -43,6 +43,7 @@ class AppSnapshotIsolateConfiguration final : public IsolateConfiguration {
 
 class KernelIsolateConfiguration : public IsolateConfiguration {
  public:
+  // The kernel mapping may be nullptr if reusing the group's loaded kernel.
   explicit KernelIsolateConfiguration(
       std::unique_ptr<const fml::Mapping> kernel)
       : kernel_(std::move(kernel)) {}
@@ -202,12 +203,17 @@ PrepareKernelMappings(const std::vector<std::string>& kernel_pieces_paths,
 std::unique_ptr<IsolateConfiguration> IsolateConfiguration::InferFromSettings(
     const Settings& settings,
     const std::shared_ptr<AssetManager>& asset_manager,
-    const fml::RefPtr<fml::TaskRunner>& io_worker) {
+    const fml::RefPtr<fml::TaskRunner>& io_worker,
+    IsolateLaunchType launch_type) {
   // Running in AOT mode.
   if (DartVM::IsRunningPrecompiledCode()) {
     return CreateForAppSnapshot();
   }
 
+  if (launch_type == IsolateLaunchType::kExistingGroup) {
+    return CreateForKernel(nullptr);
+  }
+
   if (settings.application_kernels) {
     return CreateForKernelList(settings.application_kernels());
   }

runtime/isolate_configuration.h

@@ -19,6 +19,17 @@
 
 namespace flutter {
 
+/// Describes whether the isolate is part of a group or not.
+///
+/// If the isolate is part of a group, it avoids reloading the kernel snapshot.
+enum class IsolateLaunchType {
+  /// The isolate is launched as a solo isolate or to start a new group.
+  kNewGroup,
+  /// The isolate is launched as part of a group, and avoids reloading the
+  /// kernel snapshot.
+  kExistingGroup,
+};
+
 //------------------------------------------------------------------------------
 /// @brief      An isolate configuration is a collection of snapshots and asset
 ///             managers that the engine will use to configure the isolate
@@ -57,14 +68,19 @@ class IsolateConfiguration {
   /// @param[in]  io_worker      An optional IO worker. Specify `nullptr` if a
   ///                            worker should not be used or one is not
   ///                            available.
+  /// @param[in]  launch_type    Whether the isolate is launching to form a new
+  ///                            group or as part of an existing group. If it is
+  ///                            part of an existing group, the isolate will
+  ///                            reuse resources if it can.
   ///
   /// @return     An isolate configuration if one can be inferred from the
   ///             settings. If not, returns `nullptr`.
   ///
   [[nodiscard]] static std::unique_ptr<IsolateConfiguration> InferFromSettings(
       const Settings& settings,
       const std::shared_ptr<AssetManager>& asset_manager = nullptr,
-      const fml::RefPtr<fml::TaskRunner>& io_worker = nullptr);
+      const fml::RefPtr<fml::TaskRunner>& io_worker = nullptr,
+      IsolateLaunchType launch_type = IsolateLaunchType::kNewGroup);
 
   //----------------------------------------------------------------------------
   /// @brief      Creates an AOT isolate configuration using snapshot symbols