From fa36e5100f10efe7a22223c60d8dbf450e4b6c34 Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Thu, 4 Jun 2020 16:21:33 -0700 Subject: [PATCH] Avoid creating a vector when constructing Dart typed data objects for platform messages PlatformMessageResponseDart creates an external typed data object to hold the contents of large platform messages such as loaded assets. The implementation was using a std::vector to hold the message data extracted from an fml::Mapping. The vector is initialized during construction, which is unnecessary given that the data will be immediately overwritten. This change centralizes creation of these typed data objects into Tonic's DartByteData::Create. DartByteData::Create will allocate an external typed data based on a malloc buffer if the size exceeds a threshold. Fixes https://github.com/flutter/flutter/issues/58572 --- .../window/platform_message_response_dart.cc | 36 ++----------------- lib/ui/window/window.cc | 19 ++-------- lib/ui/window/window.h | 2 -- .../tonic/typed_data/dart_byte_data.cc | 28 +++++++++++++-- 4 files changed, 31 insertions(+), 54 deletions(-) diff --git a/lib/ui/window/platform_message_response_dart.cc b/lib/ui/window/platform_message_response_dart.cc index dca1ab74f9d58..91681d074bce0 100644 --- a/lib/ui/window/platform_message_response_dart.cc +++ b/lib/ui/window/platform_message_response_dart.cc @@ -8,43 +8,12 @@ #include "flutter/common/task_runners.h" #include "flutter/fml/make_copyable.h" -#include "flutter/lib/ui/window/window.h" #include "third_party/tonic/dart_state.h" #include "third_party/tonic/logging/dart_invoke.h" +#include "third_party/tonic/typed_data/dart_byte_data.h" namespace flutter { -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)); - return Dart_NewExternalTypedDataWithFinalizer( - Dart_TypedData_kByteData, heap_data->data(), heap_data->size(), - heap_data, heap_data->size(), MessageDataFinalizer); - } -} - -Dart_Handle WrapByteData(std::unique_ptr mapping) { - std::vector data(mapping->GetSize()); - memcpy(data.data(), mapping->GetMapping(), mapping->GetSize()); - return WrapByteData(std::move(data)); -} - -} // anonymous namespace - PlatformMessageResponseDart::PlatformMessageResponseDart( tonic::DartPersistentValue callback, fml::RefPtr ui_task_runner) @@ -71,7 +40,8 @@ void PlatformMessageResponseDart::Complete(std::unique_ptr data) { return; tonic::DartState::Scope scope(dart_state); - Dart_Handle byte_buffer = WrapByteData(std::move(data)); + Dart_Handle byte_buffer = + tonic::DartByteData::Create(data->GetMapping(), data->GetSize()); tonic::DartInvoke(callback.Release(), {byte_buffer}); })); } diff --git a/lib/ui/window/window.cc b/lib/ui/window/window.cc index 8e44378a6f3fa..c71e08e677277 100644 --- a/lib/ui/window/window.cc +++ b/lib/ui/window/window.cc @@ -166,25 +166,12 @@ void GetPersistentIsolateData(Dart_NativeArguments args) { persistent_isolate_data->GetSize())); } -} // 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; - FML_CHECK(!Dart_IsError( - Dart_TypedDataAcquireData(data_handle, &type, &data, &num_bytes))); - - memcpy(data, buffer.data(), num_bytes); - Dart_TypedDataReleaseData(data_handle); - return data_handle; + return tonic::DartByteData::Create(buffer.data(), buffer.size()); } +} // namespace + WindowClient::~WindowClient() {} Window::Window(WindowClient* client) : client_(client) {} diff --git a/lib/ui/window/window.h b/lib/ui/window/window.h index 3d7dc52574653..2549a4676c761 100644 --- a/lib/ui/window/window.h +++ b/lib/ui/window/window.h @@ -35,8 +35,6 @@ namespace flutter { class FontCollection; class Scene; -Dart_Handle ToByteData(const std::vector& buffer); - // Must match the AccessibilityFeatureFlag enum in window.dart. enum class AccessibilityFeatureFlag : int32_t { kAccessibleNavigation = 1 << 0, diff --git a/third_party/tonic/typed_data/dart_byte_data.cc b/third_party/tonic/typed_data/dart_byte_data.cc index 5556e37a06d46..b548b0a9c4f4c 100644 --- a/third_party/tonic/typed_data/dart_byte_data.cc +++ b/third_party/tonic/typed_data/dart_byte_data.cc @@ -8,10 +8,32 @@ namespace tonic { +namespace { + +// For large objects it is more efficient to use an external typed data object +// with a buffer allocated outside the Dart heap. +const int kExternalSizeThreshold = 1000; + +void FreeFinalizer(void* isolate_callback_data, + Dart_WeakPersistentHandle handle, + void* peer) { + free(peer); +} + +} // anonymous namespace + Dart_Handle DartByteData::Create(const void* data, size_t length) { - auto handle = DartByteData{data, length}.dart_handle(); - // The destructor should release the typed data. - return handle; + if (length < kExternalSizeThreshold) { + auto handle = DartByteData{data, length}.dart_handle(); + // The destructor should release the typed data. + return handle; + } else { + void* buf = ::malloc(length); + TONIC_DCHECK(buf); + ::memcpy(buf, data, length); + return Dart_NewExternalTypedDataWithFinalizer( + Dart_TypedData_kByteData, buf, length, buf, length, FreeFinalizer); + } } DartByteData::DartByteData()