From da7796c917852dad8bad59d15a988264e1e815fd Mon Sep 17 00:00:00 2001 From: Alexander Biggs Date: Tue, 21 Dec 2021 11:53:47 -0800 Subject: [PATCH] Remove unused SpawnIsolate function. (flutter/engine#30364) This function is only used by a single unit test and makes it more difficult to read how isolates get created on different platforms. --- engine/src/flutter/runtime/dart_isolate.cc | 38 --------- engine/src/flutter/runtime/dart_isolate.h | 32 +------- .../flutter/runtime/dart_isolate_unittests.cc | 77 ------------------- engine/src/flutter/shell/common/engine.h | 4 +- 4 files changed, 2 insertions(+), 149 deletions(-) diff --git a/engine/src/flutter/runtime/dart_isolate.cc b/engine/src/flutter/runtime/dart_isolate.cc index 969d2e0327..7a02186f37 100644 --- a/engine/src/flutter/runtime/dart_isolate.cc +++ b/engine/src/flutter/runtime/dart_isolate.cc @@ -81,44 +81,6 @@ Dart_IsolateFlags DartIsolate::Flags::Get() const { return flags_; } -std::weak_ptr DartIsolate::SpawnIsolate( - const Settings& settings, - std::unique_ptr platform_configuration, - fml::WeakPtr snapshot_delegate, - std::string advisory_script_uri, - std::string advisory_script_entrypoint, - Flags flags, - const fml::closure& isolate_create_callback, - const fml::closure& isolate_shutdown_callback, - std::optional dart_entrypoint, - std::optional dart_entrypoint_library, - const std::vector& dart_entrypoint_args, - std::unique_ptr isolate_configuration) const { - return CreateRunningRootIsolate( - settings, // - GetIsolateGroupData().GetIsolateSnapshot(), // - std::move(platform_configuration), // - flags, // - nullptr, // - isolate_create_callback, // - isolate_shutdown_callback, // - dart_entrypoint, // - dart_entrypoint_library, // - dart_entrypoint_args, // - std::move(isolate_configuration), // - UIDartState::Context{GetTaskRunners(), // - snapshot_delegate, // - GetIOManager(), // - GetSkiaUnrefQueue(), // - GetImageDecoder(), // - GetImageGeneratorRegistry(), // - advisory_script_uri, // - advisory_script_entrypoint, // - GetVolatilePathTracker()}, // - this // - ); -} - std::weak_ptr DartIsolate::CreateRunningRootIsolate( const Settings& settings, fml::RefPtr isolate_snapshot, diff --git a/engine/src/flutter/runtime/dart_isolate.h b/engine/src/flutter/runtime/dart_isolate.h index 6c172d6b61..a7c3911340 100644 --- a/engine/src/flutter/runtime/dart_isolate.h +++ b/engine/src/flutter/runtime/dart_isolate.h @@ -199,8 +199,7 @@ class DartIsolate : public UIDartState { /// @param[in] context Engine-owned state which is /// accessed by the root dart isolate. /// @param[in] spawning_isolate The isolate that is spawning the - /// new isolate. See also - /// DartIsolate::SpawnIsolate. + /// new isolate. /// @return A weak pointer to the root Dart isolate. The caller must /// ensure that the isolate is not referenced for long periods of /// time as it prevents isolate collection when the isolate @@ -222,35 +221,6 @@ class DartIsolate : public UIDartState { const UIDartState::Context& context, const DartIsolate* spawning_isolate = nullptr); - //---------------------------------------------------------------------------- - /// @brief Creates a running DartIsolate who shares as many resources as - /// possible with the caller DartIsolate. This allows them to - /// occupy less memory together and to be created faster. - /// @details Shared components will be destroyed when the last live - /// DartIsolate is destroyed. SpawnIsolate can only be used to - /// create DartIsolates whose executable code is shared with the - /// calling DartIsolate. - /// @attention Only certain setups can take advantage of the most savings - /// currently, AOT specifically. - /// @return A weak pointer to a new running DartIsolate. The caller must - /// ensure that the isolate is not referenced for long periods of - /// time as it prevents isolate collection when the isolate - /// terminates itself. The caller may also only use the isolate on - /// the thread on which the isolate was created. - std::weak_ptr SpawnIsolate( - const Settings& settings, - std::unique_ptr platform_configuration, - fml::WeakPtr snapshot_delegate, - std::string advisory_script_uri, - std::string advisory_script_entrypoint, - Flags flags, - const fml::closure& isolate_create_callback, - const fml::closure& isolate_shutdown_callback, - std::optional dart_entrypoint, - std::optional dart_entrypoint_library, - const std::vector& dart_entrypoint_args, - std::unique_ptr isolate_configuration) const; - // |UIDartState| ~DartIsolate() override; diff --git a/engine/src/flutter/runtime/dart_isolate_unittests.cc b/engine/src/flutter/runtime/dart_isolate_unittests.cc index cc9272edc0..53b7749209 100644 --- a/engine/src/flutter/runtime/dart_isolate_unittests.cc +++ b/engine/src/flutter/runtime/dart_isolate_unittests.cc @@ -73,83 +73,6 @@ TEST_F(DartIsolateTest, RootIsolateCreationAndShutdown) { ASSERT_TRUE(root_isolate->Shutdown()); } -TEST_F(DartIsolateTest, SpawnIsolate) { - 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() // - ); - - auto isolate_configuration = - IsolateConfiguration::InferFromSettings(settings); - - UIDartState::Context context(std::move(task_runners)); - context.advisory_script_uri = "main.dart"; - context.advisory_script_entrypoint = "main"; - auto weak_isolate = DartIsolate::CreateRunningRootIsolate( - vm_data->GetSettings(), // settings - vm_data->GetIsolateSnapshot(), // isolate snapshot - nullptr, // platform configuration - DartIsolate::Flags{}, // flags - nullptr, // root_isolate_create_callback - settings.isolate_create_callback, // isolate create callback - settings.isolate_shutdown_callback, // isolate shutdown callback - "main", // dart entrypoint - std::nullopt, // dart entrypoint library - {}, // dart entrypoint arguments - std::move(isolate_configuration), // isolate configuration - std::move(context) // engine context - ); - auto root_isolate = weak_isolate.lock(); - ASSERT_TRUE(root_isolate); - ASSERT_EQ(root_isolate->GetPhase(), DartIsolate::Phase::Running); - - auto spawn_configuration = IsolateConfiguration::InferFromSettings(settings); - - auto weak_spawn = root_isolate->SpawnIsolate( - /*settings=*/vm_data->GetSettings(), - /*platform_configuration=*/nullptr, - /*snapshot_delegate=*/{}, - /*advisory_script_uri=*/"main.dart", - /*advisory_script_entrypoint=*/"main", - /*flags=*/DartIsolate::Flags{}, - /*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(spawn_configuration)); - auto spawn = weak_spawn.lock(); - ASSERT_TRUE(spawn); - ASSERT_EQ(spawn->GetPhase(), DartIsolate::Phase::Running); - - // TODO(74520): Remove conditional once isolate groups are supported by JIT. - if (DartVM::IsRunningPrecompiledCode()) { - Dart_IsolateGroup isolate_group; - { - auto isolate_scope = tonic::DartIsolateScope(root_isolate->isolate()); - isolate_group = Dart_CurrentIsolateGroup(); - } - { - auto isolate_scope = tonic::DartIsolateScope(root_isolate->isolate()); - Dart_IsolateGroup spawn_isolate_group = Dart_CurrentIsolateGroup(); - ASSERT_TRUE(isolate_group != nullptr); - ASSERT_EQ(isolate_group, spawn_isolate_group); - } - } - - ASSERT_TRUE(spawn->Shutdown()); - ASSERT_TRUE(spawn->IsShuttingDown()); - ASSERT_TRUE(root_isolate->Shutdown()); -} - TEST_F(DartIsolateTest, IsolateShutdownCallbackIsInIsolateScope) { ASSERT_FALSE(DartVMRef::IsInstanceRunning()); auto settings = CreateSettingsForFixture(); diff --git a/engine/src/flutter/shell/common/engine.h b/engine/src/flutter/shell/common/engine.h index af4b8d37d7..0c37d176d6 100644 --- a/engine/src/flutter/shell/common/engine.h +++ b/engine/src/flutter/shell/common/engine.h @@ -364,9 +364,7 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { /// @brief Create a Engine that shares as many resources as /// possible with the calling Engine such that together /// they occupy less memory and be created faster. - /// @details This method ultimately calls DartIsolate::SpawnIsolate to make - /// sure resources are shared. This should only be called on - /// running Engines. + /// @details This should only be called on running Engines. /// @return A new Engine with a running isolate. /// @see Engine::Engine /// @see DartIsolate::SpawnIsolate