From d7dd122a5e0773dbf735e944437ff8ead3dd2ed2 Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Tue, 10 Apr 2018 12:15:58 -0700 Subject: [PATCH] Avoid copying the contents of large platform message responses (flutter/engine#4947) Assets are loaded via platform messages, and currently asset payloads are being copied into Dart typed data buffers. This change uses external typed data objects that wrap the existing buffer if copying would be expensive. See https://github.com/flutter/flutter/issues/16291 --- .../window/platform_message_response_dart.cc | 43 +++++++++++++------ engine/src/flutter/lib/ui/window/window.cc | 34 +++++++-------- engine/src/flutter/lib/ui/window/window.h | 2 + 3 files changed, 49 insertions(+), 30 deletions(-) diff --git a/engine/src/flutter/lib/ui/window/platform_message_response_dart.cc b/engine/src/flutter/lib/ui/window/platform_message_response_dart.cc index adddae83653..065159b7971 100644 --- a/engine/src/flutter/lib/ui/window/platform_message_response_dart.cc +++ b/engine/src/flutter/lib/ui/window/platform_message_response_dart.cc @@ -7,12 +7,41 @@ #include #include "flutter/common/threads.h" +#include "flutter/lib/ui/window/window.h" #include "lib/fxl/functional/make_copyable.h" #include "lib/tonic/dart_state.h" #include "lib/tonic/logging/dart_invoke.h" namespace blink { +namespace { + +// Avoid copying the contents of messages beyond a certain size. +const int kMessageCopyThreshold = 1000; + +void MessageDataFinalizer(void* isolate_callback_data, + Dart_WeakPersistentHandle handle, + void* peer) { + std::vector* data = reinterpret_cast*>(peer); + delete data; +} + +Dart_Handle WrapByteData(std::vector data) { + if (data.size() < kMessageCopyThreshold) { + return ToByteData(data); + } else { + std::vector* heap_data = new std::vector(std::move(data)); + Dart_Handle data_handle = Dart_NewExternalTypedData( + Dart_TypedData_kByteData, heap_data->data(), heap_data->size()); + DART_CHECK_VALID(data_handle); + Dart_NewWeakPersistentHandle(data_handle, heap_data, heap_data->size(), + MessageDataFinalizer); + return data_handle; + } +} + +} // anonymous namespace + PlatformMessageResponseDart::PlatformMessageResponseDart( tonic::DartPersistentValue callback) : callback_(std::move(callback)) {} @@ -38,19 +67,7 @@ void PlatformMessageResponseDart::Complete(std::vector data) { return; tonic::DartState::Scope scope(dart_state); - Dart_Handle byte_buffer = - Dart_NewTypedData(Dart_TypedData_kByteData, data.size()); - DART_CHECK_VALID(byte_buffer); - - void* buffer; - intptr_t length; - Dart_TypedData_Type type; - DART_CHECK_VALID( - Dart_TypedDataAcquireData(byte_buffer, &type, &buffer, &length)); - FXL_CHECK(type == Dart_TypedData_kByteData); - FXL_CHECK(static_cast(length) == data.size()); - memcpy(buffer, data.data(), length); - Dart_TypedDataReleaseData(byte_buffer); + Dart_Handle byte_buffer = WrapByteData(std::move(data)); tonic::DartInvoke(callback.Release(), {byte_buffer}); })); } diff --git a/engine/src/flutter/lib/ui/window/window.cc b/engine/src/flutter/lib/ui/window/window.cc index 7b4cf2cde12..e12e03f10d0 100644 --- a/engine/src/flutter/lib/ui/window/window.cc +++ b/engine/src/flutter/lib/ui/window/window.cc @@ -22,23 +22,6 @@ using tonic::ToDart; namespace blink { namespace { -Dart_Handle ToByteData(const std::vector& buffer) { - Dart_Handle data_handle = - Dart_NewTypedData(Dart_TypedData_kByteData, buffer.size()); - if (Dart_IsError(data_handle)) - return data_handle; - - Dart_TypedData_Type type; - void* data = nullptr; - intptr_t num_bytes = 0; - FXL_CHECK(!Dart_IsError( - Dart_TypedDataAcquireData(data_handle, &type, &data, &num_bytes))); - - memcpy(data, buffer.data(), num_bytes); - Dart_TypedDataReleaseData(data_handle); - return data_handle; -} - void DefaultRouteName(Dart_NativeArguments args) { std::string routeName = UIDartState::Current()->window()->client()->DefaultRouteName(); @@ -119,6 +102,23 @@ void _RespondToPlatformMessage(Dart_NativeArguments args) { } // namespace +Dart_Handle ToByteData(const std::vector& buffer) { + Dart_Handle data_handle = + Dart_NewTypedData(Dart_TypedData_kByteData, buffer.size()); + if (Dart_IsError(data_handle)) + return data_handle; + + Dart_TypedData_Type type; + void* data = nullptr; + intptr_t num_bytes = 0; + FXL_CHECK(!Dart_IsError( + Dart_TypedDataAcquireData(data_handle, &type, &data, &num_bytes))); + + memcpy(data, buffer.data(), num_bytes); + Dart_TypedDataReleaseData(data_handle); + return data_handle; +} + WindowClient::~WindowClient() {} Window::Window(WindowClient* client) : client_(client) {} diff --git a/engine/src/flutter/lib/ui/window/window.h b/engine/src/flutter/lib/ui/window/window.h index fa8ce41835b..ef0d7ef863a 100644 --- a/engine/src/flutter/lib/ui/window/window.h +++ b/engine/src/flutter/lib/ui/window/window.h @@ -21,6 +21,8 @@ class DartLibraryNatives; namespace blink { class Scene; +Dart_Handle ToByteData(const std::vector& buffer); + class WindowClient { public: virtual std::string DefaultRouteName() = 0;