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.

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

Do asset IO on bg thread #29016

Closed
wants to merge 12 commits into from
11 changes: 11 additions & 0 deletions fml/concurrent_message_loop.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include <algorithm>

#include "concurrent_message_loop.h"
#include "flutter/fml/thread.h"
#include "flutter/fml/trace_event.h"

Expand Down Expand Up @@ -148,6 +149,16 @@ std::vector<fml::closure> ConcurrentMessageLoop::GetThreadTasksLocked() {
return pending_tasks;
}

bool ConcurrentMessageLoop::RunsTasksOnCurrentThread() {
std::scoped_lock lock(tasks_mutex_);
for (const auto& worker_thread_id : worker_thread_ids_) {
if (worker_thread_id == std::this_thread::get_id()) {
return true;
}
}
return false;
}

ConcurrentTaskRunner::ConcurrentTaskRunner(
std::weak_ptr<ConcurrentMessageLoop> weak_loop)
: weak_loop_(std::move(weak_loop)) {}
Expand Down
2 changes: 2 additions & 0 deletions fml/concurrent_message_loop.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ class ConcurrentMessageLoop

void PostTaskToAllWorkers(fml::closure task);

bool RunsTasksOnCurrentThread();

private:
friend ConcurrentTaskRunner;

Expand Down
17 changes: 10 additions & 7 deletions shell/common/engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ std::unique_ptr<Engine> Engine::Spawn(
/*delegate=*/delegate,
/*dispatcher_maker=*/dispatcher_maker,
/*image_decoder_task_runner=*/
runtime_controller_->GetDartVM()->GetConcurrentWorkerTaskRunner(),
concurrent_task_runner(),
/*task_runners=*/task_runners_,
/*settings=*/settings,
/*animator=*/std::move(animator),
Expand Down Expand Up @@ -547,16 +547,19 @@ void Engine::HandleAssetPlatformMessage(
std::string asset_name(reinterpret_cast<const char*>(data.GetMapping()),
data.GetSize());

if (asset_manager_) {
if (!asset_manager_) {
response->CompleteEmpty();
return;
}
concurrent_task_runner()->PostTask([asset_manager = asset_manager_,
asset_name = std::move(asset_name),
response = std::move(response)] {
std::unique_ptr<fml::Mapping> asset_mapping =
asset_manager_->GetAsMapping(asset_name);
asset_manager->GetAsMapping(asset_name);
if (asset_mapping) {
response->Complete(std::move(asset_mapping));
return;
}
}

response->CompleteEmpty();
});
}

const std::string& Engine::GetLastEntrypoint() const {
Expand Down
10 changes: 10 additions & 0 deletions shell/common/engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@

namespace flutter {

namespace testing {
class EngineTest;
}

//------------------------------------------------------------------------------
/// The engine is a component owned by the shell that resides on the UI task
/// runner and is responsible for managing the needs of the root isolate and its
Expand Down Expand Up @@ -979,6 +983,12 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {

bool GetAssetAsBuffer(const std::string& name, std::vector<uint8_t>* data);

std::shared_ptr<fml::ConcurrentTaskRunner> concurrent_task_runner() const {
FML_DCHECK(runtime_controller_);
return runtime_controller_->GetDartVM()->GetConcurrentWorkerTaskRunner();
}

friend class testing::EngineTest;
friend class testing::ShellTest;

FML_DISALLOW_COPY_AND_ASSIGN(Engine);
Expand Down
93 changes: 91 additions & 2 deletions shell/common/engine_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include <cstring>

#include "flutter/fml/synchronization/count_down_latch.h"
#include "flutter/runtime/dart_vm_lifecycle.h"
#include "flutter/shell/common/thread_host.h"
#include "flutter/testing/fixture_test.h"
Expand All @@ -15,11 +16,13 @@
#include "rapidjson/stringbuffer.h"
#include "rapidjson/writer.h"

#include "flutter/fml/backtrace.h"

///\note Deprecated MOCK_METHOD macros used until this issue is resolved:
// https://github.com/google/googletest/issues/2490

namespace flutter {

namespace testing {
namespace {
class MockDelegate : public Engine::Delegate {
public:
Expand Down Expand Up @@ -100,6 +103,41 @@ std::unique_ptr<PlatformMessage> MakePlatformMessage(
return message;
}

class FakeAssetResolver : public AssetResolver {
public:
FakeAssetResolver(std::shared_ptr<fml::ConcurrentMessageLoop> concurrent_loop)
: concurrent_loop_(std::move(concurrent_loop)) {}

// |AssetResolver|
bool IsValid() const override { return true; }

// |AssetResolver|
bool IsValidAfterAssetManagerChange() const override { return true; }

// |AssetResolver|
AssetResolverType GetType() const {
return AssetResolverType::kApkAssetProvider;
}

// |AssetResolver|
std::unique_ptr<fml::Mapping> GetAsMapping(
const std::string& asset_name) const override {
mapping_requests.push_back(asset_name);
// TODO(dnfield): FontManifest is loaded differently than most assets.
// Should we load it on a background thread?
if (asset_name != "FontManifest.json")
EXPECT_TRUE(concurrent_loop_->RunsTasksOnCurrentThread())
<< fml::BacktraceHere();
return nullptr;
}

mutable std::vector<std::string> mapping_requests;

private:
std::shared_ptr<fml::ConcurrentMessageLoop> concurrent_loop_;
};
} // namespace

class EngineTest : public testing::FixtureTest {
public:
EngineTest()
Expand All @@ -123,6 +161,11 @@ class EngineTest : public testing::FixtureTest {
latch.Wait();
}

void HandlePlatformMessage(Engine* engine,
std::unique_ptr<PlatformMessage> message) {
engine->HandlePlatformMessage(std::move(message));
}

protected:
void SetUp() override {
settings_ = CreateSettingsForFixture();
Expand All @@ -141,7 +184,6 @@ class EngineTest : public testing::FixtureTest {
std::unique_ptr<RuntimeController> runtime_controller_;
std::shared_ptr<fml::ConcurrentTaskRunner> image_decoder_task_runner_;
};
} // namespace

TEST_F(EngineTest, Create) {
PostUITaskSync([this] {
Expand Down Expand Up @@ -328,4 +370,51 @@ TEST_F(EngineTest, PassesLoadDartDeferredLibraryErrorToRuntime) {
});
}

TEST_F(EngineTest, AssetIOIsHandledOnWorkerThread) {
PostUITaskSync([this] {
MockRuntimeDelegate client;
auto mock_runtime_controller =
std::make_unique<MockRuntimeController>(client, task_runners_);
auto vm_ref = DartVMRef::Create(settings_);
EXPECT_CALL(*mock_runtime_controller, GetDartVM())
.WillRepeatedly(::testing::Return(vm_ref.get()));
EXPECT_CALL(*mock_runtime_controller, IsRootIsolateRunning())
.WillRepeatedly(::testing::Return(true));
EXPECT_CALL(*mock_runtime_controller, DispatchPlatformMessage(::testing::_))
.WillRepeatedly(::testing::Return(true));
auto engine = std::make_unique<Engine>(
/*delegate=*/delegate_,
/*dispatcher_maker=*/dispatcher_maker_,
/*image_decoder_task_runner=*/image_decoder_task_runner_,
/*task_runners=*/task_runners_,
/*settings=*/settings_,
/*animator=*/std::move(animator_),
/*io_manager=*/io_manager_,
/*font_collection=*/std::make_shared<FontCollection>(),
/*runtime_controller=*/std::move(mock_runtime_controller));

fml::RefPtr<PlatformMessageResponse> response =
fml::MakeRefCounted<MockResponse>();
std::string asset_name = "foo.png";
auto message = std::make_unique<PlatformMessage>(
"flutter/assets",
fml::MallocMapping::Copy(asset_name.c_str(), asset_name.length()),
response);

auto asset_manager = std::make_shared<AssetManager>();
auto concurrent_loop = vm_ref->GetConcurrentMessageLoop();
auto resolver = std::make_unique<FakeAssetResolver>(concurrent_loop);
auto raw_resolver = resolver.get();
asset_manager->PushBack(std::move(resolver));
engine->UpdateAssetManager(asset_manager);
HandlePlatformMessage(engine.get(), std::move(message));

fml::CountDownLatch latch(concurrent_loop->GetWorkerCount());
concurrent_loop->PostTaskToAllWorkers([&latch] { latch.CountDown(); });
latch.Wait();
EXPECT_EQ(raw_resolver->mapping_requests.back(), "foo.png");
});
}

} // namespace testing
} // namespace flutter