Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Revert "Create root isolate asynchronously" #20937

Merged
merged 1 commit into from
Sep 2, 2020
Merged

Revert "Create root isolate asynchronously" #20937

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
142 changes: 49 additions & 93 deletions runtime/runtime_controller.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,7 @@ namespace flutter {

RuntimeController::RuntimeController(RuntimeDelegate& client,
TaskRunners p_task_runners)
: client_(client),
vm_(nullptr),
task_runners_(p_task_runners),
weak_factory_(this) {}
: client_(client), vm_(nullptr), task_runners_(p_task_runners) {}

RuntimeController::RuntimeController(
RuntimeDelegate& p_client,
Expand Down Expand Up @@ -53,80 +50,48 @@ RuntimeController::RuntimeController(
platform_data_(std::move(p_platform_data)),
isolate_create_callback_(p_isolate_create_callback),
isolate_shutdown_callback_(p_isolate_shutdown_callback),
persistent_isolate_data_(std::move(p_persistent_isolate_data)),
weak_factory_(this) {
// Create the root isolate as soon as the runtime controller is initialized,
// but not using a synchronous way to avoid blocking the platform thread a
// long time as it is waiting while creating `Shell` on that platform thread.
persistent_isolate_data_(std::move(p_persistent_isolate_data)) {
// Create the root isolate as soon as the runtime controller is initialized.
// It will be run at a later point when the engine provides a run
// configuration and then runs the isolate.
create_and_config_root_isolate_ =
std::async(std::launch::deferred, [self = weak_factory_.GetWeakPtr()]() {
if (!self) {
return;
}

auto strong_root_isolate =
DartIsolate::CreateRootIsolate(
self->vm_->GetVMData()->GetSettings(), //
self->isolate_snapshot_, //
self->task_runners_, //
std::make_unique<PlatformConfiguration>(self.get()), //
self->snapshot_delegate_, //
self->io_manager_, //
self->unref_queue_, //
self->image_decoder_, //
self->advisory_script_uri_, //
self->advisory_script_entrypoint_, //
nullptr, //
self->isolate_create_callback_, //
self->isolate_shutdown_callback_ //
)
.lock();

FML_CHECK(strong_root_isolate) << "Could not create root isolate.";

// The root isolate ivar is weak.
self->root_isolate_ = strong_root_isolate;

strong_root_isolate->SetReturnCodeCallback([self](uint32_t code) {
if (!self) {
return;
}

self->root_isolate_return_code_ = {true, code};
});

if (auto* platform_configuration =
self->GetPlatformConfigurationIfAvailable()) {
tonic::DartState::Scope scope(strong_root_isolate);
platform_configuration->DidCreateIsolate();
if (!self->FlushRuntimeStateToIsolate()) {
FML_DLOG(ERROR) << "Could not setup initial isolate state.";
}
} else {
FML_DCHECK(false)
<< "RuntimeController created without window binding.";
}

FML_DCHECK(Dart_CurrentIsolate() == nullptr);

self->client_.OnRootIsolateCreated();
return;
});

// We're still trying to create the root isolate as soon as possible here on
// the UI thread although it's deferred a little bit by
// std::async(std::launch::deferred, ...). So the callers of `GetRootIsolate`
// should get a quick return after this UI thread task.
task_runners_.GetUITaskRunner()->PostTask(
[self = weak_factory_.GetWeakPtr()]() {
if (!self) {
return;
}

self->GetRootIsolate();
});
auto strong_root_isolate =
DartIsolate::CreateRootIsolate(
vm_->GetVMData()->GetSettings(), //
isolate_snapshot_, //
task_runners_, //
std::make_unique<PlatformConfiguration>(this), //
snapshot_delegate_, //
io_manager_, //
unref_queue_, //
image_decoder_, //
p_advisory_script_uri, //
p_advisory_script_entrypoint, //
nullptr, //
isolate_create_callback_, //
isolate_shutdown_callback_ //
)
.lock();

FML_CHECK(strong_root_isolate) << "Could not create root isolate.";

// The root isolate ivar is weak.
root_isolate_ = strong_root_isolate;

strong_root_isolate->SetReturnCodeCallback([this](uint32_t code) {
root_isolate_return_code_ = {true, code};
});

if (auto* platform_configuration = GetPlatformConfigurationIfAvailable()) {
tonic::DartState::Scope scope(strong_root_isolate);
platform_configuration->DidCreateIsolate();
if (!FlushRuntimeStateToIsolate()) {
FML_DLOG(ERROR) << "Could not setup initial isolate state.";
}
} else {
FML_DCHECK(false) << "RuntimeController created without window binding.";
}

FML_DCHECK(Dart_CurrentIsolate() == nullptr);
}

RuntimeController::~RuntimeController() {
Expand All @@ -142,8 +107,8 @@ RuntimeController::~RuntimeController() {
}
}

bool RuntimeController::IsRootIsolateRunning() {
std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
bool RuntimeController::IsRootIsolateRunning() const {
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
if (root_isolate) {
return root_isolate->GetPhase() == DartIsolate::Phase::Running;
}
Expand Down Expand Up @@ -269,7 +234,7 @@ bool RuntimeController::ReportTimings(std::vector<int64_t> timings) {
}

bool RuntimeController::NotifyIdle(int64_t deadline) {
std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
if (!root_isolate) {
return false;
}
Expand Down Expand Up @@ -326,7 +291,7 @@ bool RuntimeController::DispatchSemanticsAction(int32_t id,

PlatformConfiguration*
RuntimeController::GetPlatformConfigurationIfAvailable() {
std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
return root_isolate ? root_isolate->platform_configuration() : nullptr;
}

Expand Down Expand Up @@ -388,17 +353,17 @@ RuntimeController::ComputePlatformResolvedLocale(
}

Dart_Port RuntimeController::GetMainPort() {
std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
return root_isolate ? root_isolate->main_port() : ILLEGAL_PORT;
}

std::string RuntimeController::GetIsolateName() {
std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
return root_isolate ? root_isolate->debug_name() : "";
}

bool RuntimeController::HasLivePorts() {
std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
if (!root_isolate) {
return false;
}
Expand All @@ -407,20 +372,11 @@ bool RuntimeController::HasLivePorts() {
}

tonic::DartErrorHandleType RuntimeController::GetLastError() {
std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
return root_isolate ? root_isolate->GetLastError() : tonic::kNoError;
}

std::weak_ptr<DartIsolate> RuntimeController::GetRootIsolate() {
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
if (root_isolate) {
return root_isolate_;
}

// Root isolate is not yet created, get it and do some configuration.
FML_DCHECK(create_and_config_root_isolate_.valid());
create_and_config_root_isolate_.get();

return root_isolate_;
}

Expand Down
13 changes: 2 additions & 11 deletions runtime/runtime_controller.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#ifndef FLUTTER_RUNTIME_RUNTIME_CONTROLLER_H_
#define FLUTTER_RUNTIME_RUNTIME_CONTROLLER_H_

#include <future>
#include <memory>
#include <vector>

Expand Down Expand Up @@ -341,7 +340,7 @@ class RuntimeController : public PlatformConfigurationClient {
///
/// @return True if root isolate running, False otherwise.
///
virtual bool IsRootIsolateRunning();
virtual bool IsRootIsolateRunning() const;

//----------------------------------------------------------------------------
/// @brief Dispatch the specified platform message to running root
Expand Down Expand Up @@ -423,10 +422,7 @@ class RuntimeController : public PlatformConfigurationClient {
/// @brief Get a weak pointer to the root Dart isolate. This isolate may
/// only be locked on the UI task runner. Callers use this
/// accessor to transition to the root isolate to the running
/// phase. Note that it might take times if the isolate is not yet
/// created, which should be done in a subsequence task after
/// constructing `RuntimeController`, or it should get a quick
/// return otherwise.
/// phase.
///
/// @return The root isolate reference.
///
Expand Down Expand Up @@ -475,16 +471,11 @@ class RuntimeController : public PlatformConfigurationClient {
std::string advisory_script_entrypoint_;
std::function<void(int64_t)> idle_notification_callback_;
PlatformData platform_data_;
std::future<void> create_and_config_root_isolate_;
// Note that `root_isolate_` is created asynchronously from the constructor of
// `RuntimeController`, be careful to use it directly while it might have not
// been created yet. Call `GetRootIsolate()` instead which guarantees that.
std::weak_ptr<DartIsolate> root_isolate_;
std::pair<bool, uint32_t> root_isolate_return_code_ = {false, 0};
const fml::closure isolate_create_callback_;
const fml::closure isolate_shutdown_callback_;
std::shared_ptr<const fml::Mapping> persistent_isolate_data_;
fml::WeakPtrFactory<RuntimeController> weak_factory_;

PlatformConfiguration* GetPlatformConfigurationIfAvailable();

Expand Down
2 changes: 0 additions & 2 deletions runtime/runtime_delegate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ class RuntimeDelegate {

virtual FontCollection& GetFontCollection() = 0;

virtual void OnRootIsolateCreated() = 0;

virtual void UpdateIsolateDescription(const std::string isolate_name,
int64_t isolate_port) = 0;

Expand Down
4 changes: 0 additions & 4 deletions shell/common/engine.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,10 +495,6 @@ void Engine::HandlePlatformMessage(fml::RefPtr<PlatformMessage> message) {
}
}

void Engine::OnRootIsolateCreated() {
delegate_.OnRootIsolateCreated();
}

void Engine::UpdateIsolateDescription(const std::string isolate_name,
int64_t isolate_port) {
delegate_.UpdateIsolateDescription(isolate_name, isolate_port);
Expand Down
12 changes: 0 additions & 12 deletions shell/common/engine.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,15 +185,6 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {
///
virtual void OnPreEngineRestart() = 0;

//--------------------------------------------------------------------------
/// @brief Notifies the shell that the root isolate is created.
/// Currently, this information is to add to the service
/// protocol list of available root isolates running in the VM
/// and their names so that the appropriate isolate can be
/// selected in the tools for debugging and instrumentation.
///
virtual void OnRootIsolateCreated() = 0;

//--------------------------------------------------------------------------
/// @brief Notifies the shell of the name of the root isolate and its
/// port when that isolate is launched, restarted (in the
Expand Down Expand Up @@ -821,9 +812,6 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {
// |RuntimeDelegate|
void HandlePlatformMessage(fml::RefPtr<PlatformMessage> message) override;

// |RuntimeDelegate|
void OnRootIsolateCreated() override;

// |RuntimeDelegate|
void UpdateIsolateDescription(const std::string isolate_name,
int64_t isolate_port) override;
Expand Down
4 changes: 1 addition & 3 deletions shell/common/engine_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ class MockDelegate : public Engine::Delegate {
MOCK_METHOD1(OnEngineHandlePlatformMessage,
void(fml::RefPtr<PlatformMessage>));
MOCK_METHOD0(OnPreEngineRestart, void());
MOCK_METHOD0(OnRootIsolateCreated, void());
MOCK_METHOD2(UpdateIsolateDescription, void(const std::string, int64_t));
MOCK_METHOD1(SetNeedsReportTimings, void(bool));
MOCK_METHOD1(ComputePlatformResolvedLocale,
Expand All @@ -47,7 +46,6 @@ class MockRuntimeDelegate : public RuntimeDelegate {
void(SemanticsNodeUpdates, CustomAccessibilityActionUpdates));
MOCK_METHOD1(HandlePlatformMessage, void(fml::RefPtr<PlatformMessage>));
MOCK_METHOD0(GetFontCollection, FontCollection&());
MOCK_METHOD0(OnRootIsolateCreated, void());
MOCK_METHOD2(UpdateIsolateDescription, void(const std::string, int64_t));
MOCK_METHOD1(SetNeedsReportTimings, void(bool));
MOCK_METHOD1(ComputePlatformResolvedLocale,
Expand All @@ -59,7 +57,7 @@ class MockRuntimeController : public RuntimeController {
public:
MockRuntimeController(RuntimeDelegate& client, TaskRunners p_task_runners)
: RuntimeController(client, p_task_runners) {}
MOCK_METHOD0(IsRootIsolateRunning, bool());
MOCK_CONST_METHOD0(IsRootIsolateRunning, bool());
MOCK_METHOD1(DispatchPlatformMessage, bool(fml::RefPtr<PlatformMessage>));
};

Expand Down
25 changes: 4 additions & 21 deletions shell/common/shell.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,8 @@ bool Shell::Setup(std::unique_ptr<PlatformView> platform_view,

is_setup_ = true;

vm_->GetServiceProtocol()->AddHandler(this, GetServiceProtocolDescription());

PersistentCache::GetCacheForProcess()->AddWorkerTaskRunner(
task_runners_.GetIOTaskRunner());

Expand Down Expand Up @@ -1131,19 +1133,6 @@ void Shell::OnPreEngineRestart() {
latch.Wait();
}

// |Engine::Delegate|
void Shell::OnRootIsolateCreated() {
auto description = GetServiceProtocolDescription();
fml::TaskRunner::RunNowOrPostTask(
task_runners_.GetPlatformTaskRunner(),
[self = weak_factory_.GetWeakPtr(),
description = std::move(description)]() {
if (self) {
self->vm_->GetServiceProtocol()->AddHandler(self.get(), description);
}
});
}

// |Engine::Delegate|
void Shell::UpdateIsolateDescription(const std::string isolate_name,
int64_t isolate_port) {
Expand Down Expand Up @@ -1288,15 +1277,9 @@ bool Shell::HandleServiceProtocolMessage(
// |ServiceProtocol::Handler|
ServiceProtocol::Handler::Description Shell::GetServiceProtocolDescription()
const {
FML_DCHECK(task_runners_.GetUITaskRunner()->RunsTasksOnCurrentThread());

if (!weak_engine_) {
return ServiceProtocol::Handler::Description();
}

return {
weak_engine_->GetUIIsolateMainPort(),
weak_engine_->GetUIIsolateName(),
engine_->GetUIIsolateMainPort(),
engine_->GetUIIsolateName(),
};
}

Expand Down
3 changes: 0 additions & 3 deletions shell/common/shell.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -513,9 +513,6 @@ class Shell final : public PlatformView::Delegate,
// |Engine::Delegate|
void OnPreEngineRestart() override;

// |Engine::Delegate|
void OnRootIsolateCreated() override;

// |Engine::Delegate|
void UpdateIsolateDescription(const std::string isolate_name,
int64_t isolate_port) override;
Expand Down