Skip to content

Commit

Permalink
[engine] move asset mapping copy to background thread (#39918)
Browse files Browse the repository at this point in the history
[engine] move asset mapping copy to background thread
  • Loading branch information
jonahwilliams committed Mar 8, 2023
1 parent a7449c0 commit 7684aa0
Show file tree
Hide file tree
Showing 6 changed files with 145 additions and 16 deletions.
10 changes: 10 additions & 0 deletions fml/concurrent_message_loop.cc
Original file line number Diff line number Diff line change
Expand Up @@ -170,4 +170,14 @@ void ConcurrentTaskRunner::PostTask(const fml::closure& task) {
task();
}

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;
}

} // namespace fml
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(const fml::closure& task);

bool RunsTasksOnCurrentThread();

private:
friend ConcurrentTaskRunner;

Expand Down
7 changes: 6 additions & 1 deletion lib/ui/painting.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6294,7 +6294,12 @@ class ImmutableBuffer extends NativeFieldWrapperClass1 {
final ImmutableBuffer instance = ImmutableBuffer._(0);
return _futurize((_Callback<int> callback) {
return instance._initFromAsset(encodedKey, callback);
}).then((int length) => instance.._length = length);
}).then((int length) {
if (length == -1) {
throw Exception('Asset not found');
}
return instance.._length = length;
});
}

/// Create a buffer from the file with [path].
Expand Down
64 changes: 49 additions & 15 deletions lib/ui/painting/immutable_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ Dart_Handle ImmutableBuffer::init(Dart_Handle buffer_handle,
return Dart_Null();
}

Dart_Handle ImmutableBuffer::initFromAsset(Dart_Handle buffer_handle,
Dart_Handle ImmutableBuffer::initFromAsset(Dart_Handle raw_buffer_handle,
Dart_Handle asset_name_handle,
Dart_Handle callback_handle) {
UIDartState::ThrowIfUIOperationsProhibited();
Expand All @@ -62,21 +62,55 @@ Dart_Handle ImmutableBuffer::initFromAsset(Dart_Handle buffer_handle,
std::string asset_name = std::string{reinterpret_cast<const char*>(chars),
static_cast<size_t>(asset_length)};

std::shared_ptr<AssetManager> asset_manager = UIDartState::Current()
->platform_configuration()
->client()
->GetAssetManager();
std::unique_ptr<fml::Mapping> data = asset_manager->GetAsMapping(asset_name);
if (data == nullptr) {
return tonic::ToDart("Asset not found");
}
auto* dart_state = UIDartState::Current();
auto ui_task_runner = dart_state->GetTaskRunners().GetUITaskRunner();
auto buffer_callback =
std::make_unique<tonic::DartPersistentValue>(dart_state, callback_handle);
auto buffer_handle = std::make_unique<tonic::DartPersistentValue>(
dart_state, raw_buffer_handle);
auto asset_manager = UIDartState::Current()
->platform_configuration()
->client()
->GetAssetManager();

auto size = data->GetSize();
const void* bytes = static_cast<const void*>(data->GetMapping());
auto sk_data = MakeSkDataWithCopy(bytes, size);
auto buffer = fml::MakeRefCounted<ImmutableBuffer>(sk_data);
buffer->AssociateWithDartWrapper(buffer_handle);
tonic::DartInvoke(callback_handle, {tonic::ToDart(size)});
auto ui_task = fml::MakeCopyable(
[buffer_callback = std::move(buffer_callback),
buffer_handle = std::move(buffer_handle)](const sk_sp<SkData>& sk_data,
size_t buffer_size) mutable {
auto dart_state = buffer_callback->dart_state().lock();
if (!dart_state) {
return;
}
tonic::DartState::Scope scope(dart_state);
if (!sk_data) {
// -1 is used as a sentinel that the file could not be opened.
tonic::DartInvoke(buffer_callback->Get(), {tonic::ToDart(-1)});
return;
}
auto buffer = fml::MakeRefCounted<ImmutableBuffer>(sk_data);
buffer->AssociateWithDartWrapper(buffer_handle->Get());
tonic::DartInvoke(buffer_callback->Get(), {tonic::ToDart(buffer_size)});
});

dart_state->GetConcurrentTaskRunner()->PostTask(
[asset_name = std::move(asset_name),
asset_manager = std::move(asset_manager),
ui_task_runner = std::move(ui_task_runner), ui_task] {
std::unique_ptr<fml::Mapping> mapping =
asset_manager->GetAsMapping(asset_name);

sk_sp<SkData> sk_data;
size_t buffer_size = 0;
if (mapping != nullptr) {
buffer_size = mapping->GetSize();
const void* bytes = static_cast<const void*>(mapping->GetMapping());
sk_data = MakeSkDataWithCopy(bytes, buffer_size);
}
ui_task_runner->PostTask(
[sk_data = std::move(sk_data), ui_task = ui_task, buffer_size]() {
ui_task(sk_data, buffer_size);
});
});
return Dart_Null();
}

Expand Down
8 changes: 8 additions & 0 deletions shell/common/fixtures/shell_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -470,3 +470,11 @@ Future<void> testPluginUtilitiesCallbackHandle() async {
}
notifyNativeBool(true);
}

@pragma('vm:entry-point')
Future<void> testThatAssetLoadingHappensOnWorkerThread() async {
try {
await ImmutableBuffer.fromAsset('DoesNotExist');
} catch (err) { /* Do nothing */ }
notifyNative();
}
70 changes: 70 additions & 0 deletions shell/common/shell_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "flutter/flow/layers/layer_raster_cache_item.h"
#include "flutter/flow/layers/platform_view_layer.h"
#include "flutter/flow/layers/transform_layer.h"
#include "flutter/fml/backtrace.h"
#include "flutter/fml/command_line.h"
#include "flutter/fml/dart/dart_converter.h"
#include "flutter/fml/make_copyable.h"
Expand Down Expand Up @@ -194,6 +195,42 @@ class TestAssetResolver : public AssetResolver {
AssetResolver::AssetResolverType type_;
};

class ThreadCheckingAssetResolver : public AssetResolver {
public:
explicit ThreadCheckingAssetResolver(
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 {
if (asset_name == "FontManifest.json") {
// This file is loaded directly by the engine.
return nullptr;
}
mapping_requests.push_back(asset_name);
EXPECT_TRUE(concurrent_loop_->RunsTasksOnCurrentThread())
<< fml::BacktraceHere();
return nullptr;
}

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

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

static bool ValidateShell(Shell* shell) {
if (!shell) {
return false;
Expand Down Expand Up @@ -3805,6 +3842,39 @@ TEST_F(ShellTest, SpawnWorksWithOnError) {
ASSERT_FALSE(DartVMRef::IsInstanceRunning());
}

TEST_F(ShellTest, ImmutableBufferLoadsAssetOnBackgroundThread) {
Settings settings = CreateSettingsForFixture();
auto task_runner = CreateNewThread();
TaskRunners task_runners("test", task_runner, task_runner, task_runner,
task_runner);
std::unique_ptr<Shell> shell = CreateShell(settings, task_runners);

fml::CountDownLatch latch(1);
AddNativeCallback("NotifyNative",
CREATE_NATIVE_ENTRY([&](auto args) { latch.CountDown(); }));

// Create the surface needed by rasterizer
PlatformViewNotifyCreated(shell.get());

auto configuration = RunConfiguration::InferFromSettings(settings);
configuration.SetEntrypoint("testThatAssetLoadingHappensOnWorkerThread");
auto asset_manager = configuration.GetAssetManager();
auto test_resolver = std::make_unique<ThreadCheckingAssetResolver>(
shell->GetDartVM()->GetConcurrentMessageLoop());
auto leaked_resolver = test_resolver.get();
asset_manager->PushBack(std::move(test_resolver));

RunEngine(shell.get(), std::move(configuration));
PumpOneFrame(shell.get());

latch.Wait();

EXPECT_EQ(leaked_resolver->mapping_requests[0], "DoesNotExist");

PlatformViewNotifyDestroyed(shell.get());
DestroyShell(std::move(shell), task_runners);
}

TEST_F(ShellTest, PictureToImageSync) {
#if !SHELL_ENABLE_GL
// This test uses the GL backend.
Expand Down

0 comments on commit 7684aa0

Please sign in to comment.