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

Plumbing refactor to allow the usage of Dart_CreateIsolateInGroup #23549

Merged
gaaclarke merged 4 commits into from
Jan 13, 2021
Merged

Plumbing refactor to allow the usage of Dart_CreateIsolateInGroup #23549

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
29d2a61
Did the plumbing refactor that allows us to call Dart_CreateIsolateIn…
gaaclarke Jan 8, 2021
1e15db8
responded to xiao's feedback
gaaclarke Jan 11, 2021
dc585c6
updated docstrings
gaaclarke Jan 12, 2021
f01c330
added unit test for DartIsolate::SpawnIsolate
gaaclarke Jan 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 72 additions & 14 deletions runtime/dart_isolate.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,29 @@ Dart_IsolateFlags DartIsolate::Flags::Get() const {
return flags_;
}

std::weak_ptr<DartIsolate> DartIsolate::SpawnIsolate(
const Settings& settings,
std::unique_ptr<PlatformConfiguration> platform_configuration,
fml::WeakPtr<SnapshotDelegate> snapshot_delegate,
fml::WeakPtr<HintFreedDelegate> hint_freed_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<std::string> dart_entrypoint,
std::optional<std::string> dart_entrypoint_library,
std::unique_ptr<IsolateConfiguration> isolate_configration) const {
return CreateRunningRootIsolate(
settings, GetIsolateGroupData().GetIsolateSnapshot(), GetTaskRunners(),
std::move(platform_configuration), snapshot_delegate, hint_freed_delegate,
GetIOManager(), GetSkiaUnrefQueue(), GetImageDecoder(),
advisory_script_uri, advisory_script_entrypoint, flags,
isolate_create_callback, isolate_shutdown_callback, dart_entrypoint,
dart_entrypoint_library, std::move(isolate_configration),
GetVolatilePathTracker(), this);
}

std::weak_ptr<DartIsolate> DartIsolate::CreateRunningRootIsolate(
const Settings& settings,
fml::RefPtr<const DartSnapshot> isolate_snapshot,
Expand All @@ -91,7 +114,8 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRunningRootIsolate(
std::optional<std::string> dart_entrypoint,
std::optional<std::string> dart_entrypoint_library,
std::unique_ptr<IsolateConfiguration> isolate_configration,
std::shared_ptr<VolatilePathTracker> volatile_path_tracker) {
std::shared_ptr<VolatilePathTracker> volatile_path_tracker,
const DartIsolate* spawning_isolate) {
if (!isolate_snapshot) {
FML_LOG(ERROR) << "Invalid isolate snapshot.";
return {};
Expand Down Expand Up @@ -135,7 +159,8 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRunningRootIsolate(
});

if (isolate->GetPhase() != DartIsolate::Phase::LibrariesSetup) {
FML_LOG(ERROR) << "Root isolate was created in an incorrect phase.";
FML_LOG(ERROR) << "Root isolate was created in an incorrect phase: "
<< static_cast<int>(isolate->GetPhase());
return {};
}

Expand Down Expand Up @@ -190,7 +215,8 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(
Flags flags,
const fml::closure& isolate_create_callback,
const fml::closure& isolate_shutdown_callback,
std::shared_ptr<VolatilePathTracker> volatile_path_tracker) {
std::shared_ptr<VolatilePathTracker> volatile_path_tracker,
const DartIsolate* spawning_isolate) {
TRACE_EVENT0("flutter", "DartIsolate::CreateRootIsolate");

// The child isolate preparer is null but will be set when the isolate is
Expand Down Expand Up @@ -223,13 +249,28 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(
)));

DartErrorString error;
Dart_Isolate vm_isolate = nullptr;
auto isolate_flags = flags.Get();
Dart_Isolate vm_isolate = CreateDartIsolateGroup(
/// TODO(b/72025) This will be where we call Dart_CreateIsolateInGroup if
/// spawning_isolate != nullptr.
vm_isolate = CreateDartIsolateGroup(
std::move(isolate_group_data), std::move(isolate_data), &isolate_flags,
error.error());
error.error(),
[](std::shared_ptr<DartIsolateGroupData>* isolate_group_data,
std::shared_ptr<DartIsolate>* isolate_data, Dart_IsolateFlags* flags,
char** error) {
return Dart_CreateIsolateGroup(
(*isolate_group_data)->GetAdvisoryScriptURI().c_str(),
(*isolate_group_data)->GetAdvisoryScriptEntrypoint().c_str(),
(*isolate_group_data)->GetIsolateSnapshot()->GetDataMapping(),
(*isolate_group_data)
->GetIsolateSnapshot()
->GetInstructionsMapping(),
flags, isolate_group_data, isolate_data, error);
});

if (error) {
FML_LOG(ERROR) << "CreateDartIsolateGroup failed: " << error.str();
FML_LOG(ERROR) << "CreateRootIsolate failed: " << error.str();
}

if (vm_isolate == nullptr) {
Expand Down Expand Up @@ -809,6 +850,11 @@ DartIsolateGroupData& DartIsolate::GetIsolateGroupData() {
return **isolate_group_data;
}

const DartIsolateGroupData& DartIsolate::GetIsolateGroupData() const {
DartIsolate* non_const_this = const_cast<DartIsolate*>(this);
return non_const_this->GetIsolateGroupData();
}

// |Dart_IsolateGroupCreateCallback|
Dart_Isolate DartIsolate::DartIsolateGroupCreateCallback(
const char* advisory_script_uri,
Expand Down Expand Up @@ -871,7 +917,19 @@ Dart_Isolate DartIsolate::DartIsolateGroupCreateCallback(
nullptr))); // volatile path tracker

Dart_Isolate vm_isolate = CreateDartIsolateGroup(
std::move(isolate_group_data), std::move(isolate_data), flags, error);
std::move(isolate_group_data), std::move(isolate_data), flags, error,
[](std::shared_ptr<DartIsolateGroupData>* isolate_group_data,
std::shared_ptr<DartIsolate>* isolate_data, Dart_IsolateFlags* flags,
char** error) {
return Dart_CreateIsolateGroup(
(*isolate_group_data)->GetAdvisoryScriptURI().c_str(),
(*isolate_group_data)->GetAdvisoryScriptEntrypoint().c_str(),
(*isolate_group_data)->GetIsolateSnapshot()->GetDataMapping(),
(*isolate_group_data)
->GetIsolateSnapshot()
->GetInstructionsMapping(),
flags, isolate_group_data, isolate_data, error);
});

if (*error) {
FML_LOG(ERROR) << "CreateDartIsolateGroup failed: " << error;
Expand Down Expand Up @@ -933,16 +991,16 @@ Dart_Isolate DartIsolate::CreateDartIsolateGroup(
std::unique_ptr<std::shared_ptr<DartIsolateGroupData>> isolate_group_data,
std::unique_ptr<std::shared_ptr<DartIsolate>> isolate_data,
Dart_IsolateFlags* flags,
char** error) {
char** error,
std::function<Dart_Isolate(std::shared_ptr<DartIsolateGroupData>*,
std::shared_ptr<DartIsolate>*,
Dart_IsolateFlags*,
char**)> make_isolate) {
TRACE_EVENT0("flutter", "DartIsolate::CreateDartIsolateGroup");

// Create the Dart VM isolate and give it the embedder object as the baton.
Dart_Isolate isolate = Dart_CreateIsolateGroup(
(*isolate_group_data)->GetAdvisoryScriptURI().c_str(),
(*isolate_group_data)->GetAdvisoryScriptEntrypoint().c_str(),
(*isolate_group_data)->GetIsolateSnapshot()->GetDataMapping(),
(*isolate_group_data)->GetIsolateSnapshot()->GetInstructionsMapping(),
flags, isolate_group_data.get(), isolate_data.get(), error);
Dart_Isolate isolate =
make_isolate(isolate_group_data.get(), isolate_data.get(), flags, error);

if (isolate == nullptr) {
return nullptr;
Expand Down
40 changes: 36 additions & 4 deletions runtime/dart_isolate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
#include "flutter/lib/ui/io_manager.h"
#include "flutter/lib/ui/snapshot_delegate.h"
#include "flutter/lib/ui/ui_dart_state.h"
#include "flutter/lib/ui/volatile_path_tracker.h"
#include "flutter/lib/ui/window/platform_configuration.h"
#include "flutter/runtime/dart_snapshot.h"
#include "third_party/dart/runtime/include/dart_api.h"
Expand Down Expand Up @@ -232,7 +231,33 @@ class DartIsolate : public UIDartState {
std::optional<std::string> dart_entrypoint,
std::optional<std::string> dart_entrypoint_library,
std::unique_ptr<IsolateConfiguration> isolate_configration,
std::shared_ptr<VolatilePathTracker> volatile_path_tracker);
std::shared_ptr<VolatilePathTracker> volatile_path_tracker,
const DartIsolate* spawning_isolate = nullptr);

@xster xster Jan 12, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you change the signature, update the method doc

@gaaclarke gaaclarke Jan 12, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


//----------------------------------------------------------------------------
/// @brief Creates a running DartIsolate who shares as many resources as

@xster xster Jan 12, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creates a new running DartIsolate from the current DartIsolate

@gaaclarke gaaclarke Jan 12, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that is much of an improvement. Hopefully the other edits cover the information you are trying to convey with that.

/// 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 DartIsolate

@xster xster Jan 12, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when the last DartIsolate among the group. Might be misunderstood as if you make 1, 2, 3, 4, shared components will be destroyed if 4 is destroyed.

@gaaclarke gaaclarke Jan 12, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

/// is destroyed. SpawnIsolate can only be used to create
/// DartIsolates whose executable code is shared with the calling
/// DartIsolate.

@xster xster Jan 12, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the current DartIsolate need to be in any particular states before calling this?

@gaaclarke gaaclarke Jan 12, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure of the hard requirements of Dart_CreateIsolateInGroup. It isn't spelled out in its documentation.

/// @attention Only certain setups can take advantage of the most savings
/// currently, AOT specifically.

@xster xster Jan 12, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add the same disclosure for why it's a weak pointer, who owns it, performance considerations etc.

@gaaclarke gaaclarke Jan 12, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

/// @return A running DartIsolate.

@xster xster Jan 12, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A new, running DartIsolate.

@gaaclarke gaaclarke Jan 12, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

std::weak_ptr<DartIsolate> SpawnIsolate(
const Settings& settings,
std::unique_ptr<PlatformConfiguration> platform_configuration,
fml::WeakPtr<SnapshotDelegate> snapshot_delegate,
fml::WeakPtr<HintFreedDelegate> hint_freed_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<std::string> dart_entrypoint,
std::optional<std::string> dart_entrypoint_library,
std::unique_ptr<IsolateConfiguration> isolate_configration) const;

// |UIDartState|
~DartIsolate() override;
Expand Down Expand Up @@ -433,7 +458,8 @@ class DartIsolate : public UIDartState {
Flags flags,
const fml::closure& isolate_create_callback,
const fml::closure& isolate_shutdown_callback,
std::shared_ptr<VolatilePathTracker> volatile_path_tracker);
std::shared_ptr<VolatilePathTracker> volatile_path_tracker,
const DartIsolate* spawning_isolate = nullptr);

DartIsolate(const Settings& settings,
TaskRunners task_runners,
Expand Down Expand Up @@ -463,6 +489,8 @@ class DartIsolate : public UIDartState {

DartIsolateGroupData& GetIsolateGroupData();

const DartIsolateGroupData& GetIsolateGroupData() const;

// |Dart_IsolateGroupCreateCallback|
static Dart_Isolate DartIsolateGroupCreateCallback(
const char* advisory_script_uri,
Expand All @@ -487,7 +515,11 @@ class DartIsolate : public UIDartState {
std::unique_ptr<std::shared_ptr<DartIsolateGroupData>> isolate_group_data,
std::unique_ptr<std::shared_ptr<DartIsolate>> isolate_data,
Dart_IsolateFlags* flags,
char** error);
char** error,
std::function<Dart_Isolate(std::shared_ptr<DartIsolateGroupData>*,
std::shared_ptr<DartIsolate>*,
Dart_IsolateFlags*,
char**)> make_isolate);

static bool InitializeIsolate(std::shared_ptr<DartIsolate> embedder_isolate,
Dart_Isolate isolate,
Expand Down
14 changes: 12 additions & 2 deletions runtime/dart_vm_lifecycle.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ class DartVMRef {
fml::RefPtr<DartSnapshot> vm_snapshot = nullptr,
fml::RefPtr<DartSnapshot> isolate_snapshot = nullptr);

DartVMRef(const DartVMRef&) = default;

DartVMRef(DartVMRef&&);

~DartVMRef();
Expand All @@ -53,11 +55,21 @@ class DartVMRef {
return vm_.get();
}

const DartVM* get() const {
FML_DCHECK(vm_);
return vm_.get();
}

DartVM* operator->() {
FML_DCHECK(vm_);
return vm_.get();
}

const DartVM* operator->() const {
FML_DCHECK(vm_);
return vm_.get();
}

DartVM* operator&() {
FML_DCHECK(vm_);
return vm_.get();
Expand All @@ -72,8 +84,6 @@ class DartVMRef {

// Only used by Dart Isolate to register itself with the VM.
static DartVM* GetRunningVM();

FML_DISALLOW_COPY_AND_ASSIGN(DartVMRef);
};

} // namespace flutter
Expand Down
19 changes: 19 additions & 0 deletions runtime/runtime_controller.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,25 @@ RuntimeController::RuntimeController(
persistent_isolate_data_(std::move(p_persistent_isolate_data)),
volatile_path_tracker_(std::move(p_volatile_path_tracker)) {}

std::unique_ptr<RuntimeController> RuntimeController::Spawn(
RuntimeDelegate& client,
std::string advisory_script_uri,
std::string advisory_script_entrypoint,
const std::function<void(int64_t)>& idle_notification_callback,
const fml::closure& isolate_create_callback,
const fml::closure& isolate_shutdown_callback,
std::shared_ptr<const fml::Mapping> persistent_isolate_data) const {
auto result = std::make_unique<RuntimeController>(
client, vm_, isolate_snapshot_, task_runners_, snapshot_delegate_,
hint_freed_delegate_, io_manager_, unref_queue_, image_decoder_,
advisory_script_uri, advisory_script_entrypoint,
idle_notification_callback, platform_data_, isolate_create_callback,
isolate_shutdown_callback, persistent_isolate_data,
volatile_path_tracker_);
result->spawning_isolate_ = root_isolate_;
return result;
}

RuntimeController::~RuntimeController() {
FML_DCHECK(Dart_CurrentIsolate() == nullptr);
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
Expand Down
34 changes: 34 additions & 0 deletions runtime/runtime_controller.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,22 @@ class RuntimeController : public PlatformConfigurationClient {
std::shared_ptr<const fml::Mapping> persistent_isolate_data,
std::shared_ptr<VolatilePathTracker> volatile_path_tracker);

//----------------------------------------------------------------------------
/// @brief Create a RuntimeController that shares as many resources as
/// possible with the calling RuntimeController such that together
/// they occupy less memory.
/// @return A RuntimeController with a running isolate.
/// @see RuntimeController::RuntimeController
///
std::unique_ptr<RuntimeController> Spawn(
RuntimeDelegate& client,
std::string advisory_script_uri,
std::string advisory_script_entrypoint,
const std::function<void(int64_t)>& idle_notification_callback,
const fml::closure& isolate_create_callback,
const fml::closure& isolate_shutdown_callback,
std::shared_ptr<const fml::Mapping> persistent_isolate_data) const;

// |PlatformConfigurationClient|
~RuntimeController() override;

Expand Down Expand Up @@ -542,6 +558,23 @@ class RuntimeController : public PlatformConfigurationClient {

// |PlatformConfigurationClient|
void RequestDartDeferredLibrary(intptr_t loading_unit_id) override;
const fml::WeakPtr<IOManager>& GetIOManager() const { return io_manager_; }

DartVM* GetDartVM() const { return vm_; }

const fml::RefPtr<const DartSnapshot>& GetIsolateSnapshot() const {
return isolate_snapshot_;
}

const PlatformData& GetPlatformData() const { return platform_data_; }

const fml::RefPtr<SkiaUnrefQueue>& GetSkiaUnrefQueue() const {
return unref_queue_;
}

const fml::WeakPtr<SnapshotDelegate>& GetSnapshotDelegate() const {
return snapshot_delegate_;
}

protected:
/// Constructor for Mocks.
Expand Down Expand Up @@ -576,6 +609,7 @@ class RuntimeController : public PlatformConfigurationClient {
std::function<void(int64_t)> idle_notification_callback_;
PlatformData platform_data_;
std::weak_ptr<DartIsolate> root_isolate_;
std::weak_ptr<DartIsolate> spawning_isolate_;
std::optional<uint32_t> root_isolate_return_code_;
const fml::closure isolate_create_callback_;
const fml::closure isolate_shutdown_callback_;
Expand Down
27 changes: 27 additions & 0 deletions shell/common/engine.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,33 @@ Engine::Engine(Delegate& delegate,
);
}

std::unique_ptr<Engine> Engine::Spawn(
Delegate& delegate,
const PointerDataDispatcherMaker& dispatcher_maker,
Settings settings,
std::unique_ptr<Animator> animator) const {
auto result = std::make_unique<Engine>(
/*delegate=*/delegate,
/*dispatcher_maker=*/dispatcher_maker,
/*image_decoder_task_runner=*/
runtime_controller_->GetDartVM()->GetConcurrentWorkerTaskRunner(),
/*task_runners=*/task_runners_,
/*settings=*/settings,
/*animator=*/std::move(animator),
/*io_manager=*/runtime_controller_->GetIOManager(),
/*runtime_controller=*/nullptr);
result->runtime_controller_ = runtime_controller_->Spawn(
*result, // runtime delegate
settings_.advisory_script_uri, // advisory script uri
settings_.advisory_script_entrypoint, // advisory script entrypoint
settings_.idle_notification_callback, // idle notification callback
settings_.isolate_create_callback, // isolate create callback
settings_.isolate_shutdown_callback, // isolate shutdown callback
settings_.persistent_isolate_data // persistent isolate data
);
return result;
}

Engine::~Engine() = default;

fml::WeakPtr<Engine> Engine::GetWeakPtr() const {
Expand Down
Loading