Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 0895dee

Browse files
dnfielddnfield
authored
Avoid reloading the kernel snapshot when spawning an isolate in the same group (#48478)
Found by @mraleph while looking at flutter_tester related things with me. I took some suggestions from him and added a test. This should eventually help speed up running large multi-file test suites. The test is a little bit wonky because we don't have great inspection points (e.g. something to mock or examine from a test). It should break if we change the way we load kernel assets in the test harness, and is verifying that we only ask for the test asset once where we used to ask for it twice.
1 parent 58502dc commit 0895dee

5 files changed

+181
-45
lines changed

‎runtime/dart_isolate.cc

Lines changed: 50 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -196,50 +196,56 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(
196196
const DartIsolate* spawning_isolate) {
197197
TRACE_EVENT0("flutter", "DartIsolate::CreateRootIsolate");
198198

199-
// The child isolate preparer is null but will be set when the isolate is
200-
// being prepared to run.
201-
auto isolate_group_data =
202-
std::make_unique<std::shared_ptr<DartIsolateGroupData>>(
203-
std::shared_ptr<DartIsolateGroupData>(new DartIsolateGroupData(
204-
settings, // settings
205-
std::move(isolate_snapshot), // isolate snapshot
206-
context.advisory_script_uri, // advisory URI
207-
context.advisory_script_entrypoint, // advisory entrypoint
208-
nullptr, // child isolate preparer
209-
isolate_create_callback, // isolate create callback
210-
isolate_shutdown_callback // isolate shutdown callback
211-
)));
199+
// Only needed if this is the main isolate for the group.
200+
std::unique_ptr<std::shared_ptr<DartIsolateGroupData>> isolate_group_data;
212201

213202
auto isolate_data = std::make_unique<std::shared_ptr<DartIsolate>>(
214-
std::shared_ptr<DartIsolate>(new DartIsolate(settings, // settings
215-
true, // is_root_isolate
216-
context // context
217-
)));
203+
std::shared_ptr<DartIsolate>(new DartIsolate(
204+
/*settings=*/settings,
205+
/*is_root_isolate=*/true,
206+
/*context=*/context,
207+
/*is_spawning_in_group=*/!!spawning_isolate)));
218208

219209
DartErrorString error;
220210
Dart_Isolate vm_isolate = nullptr;
221211
auto isolate_flags = flags.Get();
222212

223213
IsolateMaker isolate_maker;
224214
if (spawning_isolate) {
225-
isolate_maker = [spawning_isolate](
226-
std::shared_ptr<DartIsolateGroupData>*
227-
isolate_group_data,
228-
std::shared_ptr<DartIsolate>* isolate_data,
229-
Dart_IsolateFlags* flags, char** error) {
230-
return Dart_CreateIsolateInGroup(
231-
/*group_member=*/spawning_isolate->isolate(),
232-
/*name=*/(*isolate_group_data)->GetAdvisoryScriptEntrypoint().c_str(),
233-
/*shutdown_callback=*/
234-
reinterpret_cast<Dart_IsolateShutdownCallback>(
235-
DartIsolate::SpawnIsolateShutdownCallback),
236-
/*cleanup_callback=*/
237-
reinterpret_cast<Dart_IsolateCleanupCallback>(
238-
DartIsolateCleanupCallback),
239-
/*child_isolate_data=*/isolate_data,
240-
/*error=*/error);
241-
};
215+
isolate_maker =
216+
[spawning_isolate](
217+
std::shared_ptr<DartIsolateGroupData>* isolate_group_data,
218+
std::shared_ptr<DartIsolate>* isolate_data,
219+
Dart_IsolateFlags* flags, char** error) {
220+
return Dart_CreateIsolateInGroup(
221+
/*group_member=*/spawning_isolate->isolate(),
222+
/*name=*/
223+
spawning_isolate->GetIsolateGroupData()
224+
.GetAdvisoryScriptEntrypoint()
225+
.c_str(),
226+
/*shutdown_callback=*/
227+
reinterpret_cast<Dart_IsolateShutdownCallback>(
228+
DartIsolate::SpawnIsolateShutdownCallback),
229+
/*cleanup_callback=*/
230+
reinterpret_cast<Dart_IsolateCleanupCallback>(
231+
DartIsolateCleanupCallback),
232+
/*child_isolate_data=*/isolate_data,
233+
/*error=*/error);
234+
};
242235
} else {
236+
// The child isolate preparer is null but will be set when the isolate is
237+
// being prepared to run.
238+
isolate_group_data =
239+
std::make_unique<std::shared_ptr<DartIsolateGroupData>>(
240+
std::shared_ptr<DartIsolateGroupData>(new DartIsolateGroupData(
241+
settings, // settings
242+
std::move(isolate_snapshot), // isolate snapshot
243+
context.advisory_script_uri, // advisory URI
244+
context.advisory_script_entrypoint, // advisory entrypoint
245+
nullptr, // child isolate preparer
246+
isolate_create_callback, // isolate create callback
247+
isolate_shutdown_callback // isolate shutdown callback
248+
)));
243249
isolate_maker = [](std::shared_ptr<DartIsolateGroupData>*
244250
isolate_group_data,
245251
std::shared_ptr<DartIsolate>* isolate_data,
@@ -276,7 +282,8 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(
276282

277283
DartIsolate::DartIsolate(const Settings& settings,
278284
bool is_root_isolate,
279-
const UIDartState::Context& context)
285+
const UIDartState::Context& context,
286+
bool is_spawning_in_group)
280287
: UIDartState(settings.task_observer_add,
281288
settings.task_observer_remove,
282289
settings.log_tag,
@@ -287,7 +294,8 @@ DartIsolate::DartIsolate(const Settings& settings,
287294
context),
288295
may_insecurely_connect_to_all_domains_(
289296
settings.may_insecurely_connect_to_all_domains),
290-
domain_network_policy_(settings.domain_network_policy) {
297+
domain_network_policy_(settings.domain_network_policy),
298+
is_spawning_in_group_(is_spawning_in_group) {
291299
phase_ = Phase::Uninitialized;
292300
}
293301

@@ -548,13 +556,13 @@ bool DartIsolate::LoadKernel(const std::shared_ptr<const fml::Mapping>& mapping,
548556
return false;
549557
}
550558

551-
if (!mapping || mapping->GetSize() == 0) {
552-
return false;
553-
}
554-
555559
tonic::DartState::Scope scope(this);
556560

557-
if (!child_isolate) {
561+
if (!child_isolate && !is_spawning_in_group_) {
562+
if (!mapping || mapping->GetSize() == 0) {
563+
return false;
564+
}
565+
558566
// Use root library provided by kernel in favor of one provided by snapshot.
559567
Dart_SetRootLibrary(Dart_Null());
560568

@@ -923,7 +931,7 @@ Dart_Isolate DartIsolate::DartIsolateGroupCreateCallback(
923931
});
924932

925933
if (*error) {
926-
FML_LOG(ERROR) << "CreateDartIsolateGroup failed: " << error;
934+
FML_LOG(ERROR) << "CreateDartIsolateGroup failed: " << *error;
927935
}
928936

929937
return vm_isolate;

‎runtime/dart_isolate.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "flutter/lib/ui/ui_dart_state.h"
2121
#include "flutter/lib/ui/window/platform_configuration.h"
2222
#include "flutter/runtime/dart_snapshot.h"
23+
#include "runtime/isolate_configuration.h"
2324
#include "third_party/dart/runtime/include/dart_api.h"
2425
#include "third_party/tonic/dart_state.h"
2526

@@ -412,6 +413,7 @@ class DartIsolate : public UIDartState {
412413
fml::RefPtr<fml::TaskRunner> message_handling_task_runner_;
413414
const bool may_insecurely_connect_to_all_domains_;
414415
std::string domain_network_policy_;
416+
const bool is_spawning_in_group_;
415417

416418
static std::weak_ptr<DartIsolate> CreateRootIsolate(
417419
const Settings& settings,
@@ -425,7 +427,8 @@ class DartIsolate : public UIDartState {
425427

426428
DartIsolate(const Settings& settings,
427429
bool is_root_isolate,
428-
const UIDartState::Context& context);
430+
const UIDartState::Context& context,
431+
bool is_spawning_in_group = false);
429432

430433
//----------------------------------------------------------------------------
431434
/// @brief Initializes the given (current) isolate.

‎runtime/dart_isolate_unittests.cc

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -592,6 +592,109 @@ TEST_F(DartIsolateTest, DartPluginRegistrantIsCalled) {
592592
ASSERT_EQ(messages[0], "_PluginRegistrant.register() was called");
593593
}
594594

595+
TEST_F(DartIsolateTest, SpawningAnIsolateDoesNotReloadKernel) {
596+
ASSERT_FALSE(DartVMRef::IsInstanceRunning());
597+
auto settings = CreateSettingsForFixture();
598+
auto vm_ref = DartVMRef::Create(settings);
599+
ASSERT_TRUE(vm_ref);
600+
auto vm_data = vm_ref.GetVMData();
601+
ASSERT_TRUE(vm_data);
602+
TaskRunners task_runners(GetCurrentTestName(), //
603+
GetCurrentTaskRunner(), //
604+
GetCurrentTaskRunner(), //
605+
GetCurrentTaskRunner(), //
606+
GetCurrentTaskRunner() //
607+
);
608+
609+
size_t get_kernel_count = 0u;
610+
if (!DartVM::IsRunningPrecompiledCode()) {
611+
ASSERT_TRUE(settings.application_kernels);
612+
auto mappings = settings.application_kernels();
613+
ASSERT_EQ(mappings.size(), 1u);
614+
615+
// This feels a little brittle, but the alternative seems to be making
616+
// DartIsolate have virtual methods so it can be mocked or exposing weird
617+
// test-only API on IsolateConfiguration.
618+
settings.application_kernels = fml::MakeCopyable(
619+
[&get_kernel_count,
620+
mapping = std::move(mappings.front())]() mutable -> Mappings {
621+
get_kernel_count++;
622+
EXPECT_EQ(get_kernel_count, 1u)
623+
<< "Unexpectedly got more than one call for the kernel mapping.";
624+
EXPECT_TRUE(mapping);
625+
std::vector<std::unique_ptr<const fml::Mapping>> kernel_mappings;
626+
if (mapping) {
627+
kernel_mappings.emplace_back(std::move(mapping));
628+
}
629+
return kernel_mappings;
630+
});
631+
}
632+
633+
std::shared_ptr<DartIsolate> root_isolate;
634+
{
635+
auto isolate_configuration =
636+
IsolateConfiguration::InferFromSettings(settings);
637+
638+
UIDartState::Context context(task_runners);
639+
context.advisory_script_uri = "main.dart";
640+
context.advisory_script_entrypoint = "main";
641+
auto weak_isolate = DartIsolate::CreateRunningRootIsolate(
642+
/*settings=*/vm_data->GetSettings(),
643+
/*isolate_snapshot=*/vm_data->GetIsolateSnapshot(),
644+
/*platform_configuration=*/nullptr,
645+
/*flags=*/DartIsolate::Flags{},
646+
/*root_isolate_create_callback=*/nullptr,
647+
/*isolate_create_callback=*/settings.isolate_create_callback,
648+
/*isolate_shutdown_callback=*/settings.isolate_shutdown_callback,
649+
/*dart_entrypoint=*/"main",
650+
/*dart_entrypoint_library=*/std::nullopt,
651+
/*dart_entrypoint_args=*/{},
652+
/*isolate_configuration=*/std::move(isolate_configuration),
653+
/*context=*/context);
654+
root_isolate = weak_isolate.lock();
655+
}
656+
ASSERT_TRUE(root_isolate);
657+
ASSERT_EQ(root_isolate->GetPhase(), DartIsolate::Phase::Running);
658+
if (!DartVM::IsRunningPrecompiledCode()) {
659+
ASSERT_EQ(get_kernel_count, 1u);
660+
}
661+
662+
{
663+
auto isolate_configuration = IsolateConfiguration::InferFromSettings(
664+
/*settings=*/settings,
665+
/*asset_manager=*/nullptr,
666+
/*io_worker=*/nullptr,
667+
/*launch_type=*/IsolateLaunchType::kExistingGroup);
668+
669+
UIDartState::Context context(task_runners);
670+
context.advisory_script_uri = "main.dart";
671+
context.advisory_script_entrypoint = "main";
672+
auto weak_isolate = DartIsolate::CreateRunningRootIsolate(
673+
/*settings=*/vm_data->GetSettings(),
674+
/*isolate_snapshot=*/vm_data->GetIsolateSnapshot(),
675+
/*platform_configuration=*/nullptr,
676+
/*flags=*/DartIsolate::Flags{},
677+
/*root_isolate_create_callback=*/nullptr,
678+
/*isolate_create_callback=*/settings.isolate_create_callback,
679+
/*isolate_shutdown_callback=*/settings.isolate_shutdown_callback,
680+
/*dart_entrypoint=*/"main",
681+
/*dart_entrypoint_library=*/std::nullopt,
682+
/*dart_entrypoint_args=*/{},
683+
/*isolate_configuration=*/std::move(isolate_configuration),
684+
/*context=*/context,
685+
/*spawning_isolate=*/root_isolate.get());
686+
auto spawned_isolate = weak_isolate.lock();
687+
ASSERT_TRUE(spawned_isolate);
688+
ASSERT_EQ(spawned_isolate->GetPhase(), DartIsolate::Phase::Running);
689+
if (!DartVM::IsRunningPrecompiledCode()) {
690+
ASSERT_EQ(get_kernel_count, 1u);
691+
}
692+
ASSERT_TRUE(spawned_isolate->Shutdown());
693+
}
694+
695+
ASSERT_TRUE(root_isolate->Shutdown());
696+
}
697+
595698
} // namespace testing
596699
} // namespace flutter
597700

‎runtime/isolate_configuration.cc

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ class AppSnapshotIsolateConfiguration final : public IsolateConfiguration {
4343

4444
class KernelIsolateConfiguration : public IsolateConfiguration {
4545
public:
46+
// The kernel mapping may be nullptr if reusing the group's loaded kernel.
4647
explicit KernelIsolateConfiguration(
4748
std::unique_ptr<const fml::Mapping> kernel)
4849
: kernel_(std::move(kernel)) {}
@@ -202,12 +203,17 @@ PrepareKernelMappings(const std::vector<std::string>& kernel_pieces_paths,
202203
std::unique_ptr<IsolateConfiguration> IsolateConfiguration::InferFromSettings(
203204
const Settings& settings,
204205
const std::shared_ptr<AssetManager>& asset_manager,
205-
const fml::RefPtr<fml::TaskRunner>& io_worker) {
206+
const fml::RefPtr<fml::TaskRunner>& io_worker,
207+
IsolateLaunchType launch_type) {
206208
// Running in AOT mode.
207209
if (DartVM::IsRunningPrecompiledCode()) {
208210
return CreateForAppSnapshot();
209211
}
210212

213+
if (launch_type == IsolateLaunchType::kExistingGroup) {
214+
return CreateForKernel(nullptr);
215+
}
216+
211217
if (settings.application_kernels) {
212218
return CreateForKernelList(settings.application_kernels());
213219
}

‎runtime/isolate_configuration.h

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,17 @@
1919

2020
namespace flutter {
2121

22+
/// Describes whether the isolate is part of a group or not.
23+
///
24+
/// If the isolate is part of a group, it avoids reloading the kernel snapshot.
25+
enum class IsolateLaunchType {
26+
/// The isolate is launched as a solo isolate or to start a new group.
27+
kNewGroup,
28+
/// The isolate is launched as part of a group, and avoids reloading the
29+
/// kernel snapshot.
30+
kExistingGroup,
31+
};
32+
2233
//------------------------------------------------------------------------------
2334
/// @brief An isolate configuration is a collection of snapshots and asset
2435
/// managers that the engine will use to configure the isolate
@@ -57,14 +68,19 @@ class IsolateConfiguration {
5768
/// @param[in] io_worker An optional IO worker. Specify `nullptr` if a
5869
/// worker should not be used or one is not
5970
/// available.
71+
/// @param[in] launch_type Whether the isolate is launching to form a new
72+
/// group or as part of an existing group. If it is
73+
/// part of an existing group, the isolate will
74+
/// reuse resources if it can.
6075
///
6176
/// @return An isolate configuration if one can be inferred from the
6277
/// settings. If not, returns `nullptr`.
6378
///
6479
[[nodiscard]] static std::unique_ptr<IsolateConfiguration> InferFromSettings(
6580
const Settings& settings,
6681
const std::shared_ptr<AssetManager>& asset_manager = nullptr,
67-
const fml::RefPtr<fml::TaskRunner>& io_worker = nullptr);
82+
const fml::RefPtr<fml::TaskRunner>& io_worker = nullptr,
83+
IsolateLaunchType launch_type = IsolateLaunchType::kNewGroup);
6884

6985
//----------------------------------------------------------------------------
7086
/// @brief Creates an AOT isolate configuration using snapshot symbols

0 commit comments

Comments
 (0)