diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index 910b0d4580ed8..6bde83da9790c 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -196,25 +196,15 @@ std::weak_ptr 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(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> isolate_group_data; auto isolate_data = std::make_unique>( - std::shared_ptr(new DartIsolate(settings, // settings - true, // is_root_isolate - context // context - ))); + std::shared_ptr(new DartIsolate( + /*settings=*/settings, + /*is_root_isolate=*/true, + /*context=*/context, + /*is_spawning_in_group=*/!!spawning_isolate))); DartErrorString error; Dart_Isolate vm_isolate = nullptr; @@ -222,24 +212,40 @@ std::weak_ptr DartIsolate::CreateRootIsolate( IsolateMaker isolate_maker; if (spawning_isolate) { - isolate_maker = [spawning_isolate]( - std::shared_ptr* - isolate_group_data, - std::shared_ptr* 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( - DartIsolate::SpawnIsolateShutdownCallback), - /*cleanup_callback=*/ - reinterpret_cast( - DartIsolateCleanupCallback), - /*child_isolate_data=*/isolate_data, - /*error=*/error); - }; + isolate_maker = + [spawning_isolate]( + std::shared_ptr* isolate_group_data, + std::shared_ptr* 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( + DartIsolate::SpawnIsolateShutdownCallback), + /*cleanup_callback=*/ + reinterpret_cast( + 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(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* isolate_group_data, std::shared_ptr* isolate_data, @@ -276,7 +282,8 @@ std::weak_ptr 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& 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; diff --git a/runtime/dart_isolate.h b/runtime/dart_isolate.h index 04e413a68bb29..0cc5e90345b5d 100644 --- a/runtime/dart_isolate.h +++ b/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 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 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. diff --git a/runtime/dart_isolate_unittests.cc b/runtime/dart_isolate_unittests.cc index 2a299f3150c2f..b8a42860ae7e0 100644 --- a/runtime/dart_isolate_unittests.cc +++ b/runtime/dart_isolate_unittests.cc @@ -592,6 +592,109 @@ 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++; + EXPECT_EQ(get_kernel_count, 1u) + << "Unexpectedly got more than one call for the kernel mapping."; + EXPECT_TRUE(mapping); + std::vector> kernel_mappings; + if (mapping) { + kernel_mappings.emplace_back(std::move(mapping)); + } + return kernel_mappings; + }); + } + + std::shared_ptr 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 diff --git a/runtime/isolate_configuration.cc b/runtime/isolate_configuration.cc index 54eb6c35ee3c4..ffbc28e035496 100644 --- a/runtime/isolate_configuration.cc +++ b/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 kernel) : kernel_(std::move(kernel)) {} @@ -202,12 +203,17 @@ PrepareKernelMappings(const std::vector& kernel_pieces_paths, std::unique_ptr IsolateConfiguration::InferFromSettings( const Settings& settings, const std::shared_ptr& asset_manager, - const fml::RefPtr& io_worker) { + const fml::RefPtr& 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()); } diff --git a/runtime/isolate_configuration.h b/runtime/isolate_configuration.h index 46f9c9368c2d3..a4c809ee9fbea 100644 --- a/runtime/isolate_configuration.h +++ b/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,6 +68,10 @@ 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`. @@ -64,7 +79,8 @@ class IsolateConfiguration { [[nodiscard]] static std::unique_ptr InferFromSettings( const Settings& settings, const std::shared_ptr& asset_manager = nullptr, - const fml::RefPtr& io_worker = nullptr); + const fml::RefPtr& io_worker = nullptr, + IsolateLaunchType launch_type = IsolateLaunchType::kNewGroup); //---------------------------------------------------------------------------- /// @brief Creates an AOT isolate configuration using snapshot symbols