From b7b791b3fa05fe06d778c56d8ad53897f8ef05cc Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Wed, 17 Jul 2019 13:34:40 -0700 Subject: [PATCH] In a single frame codec, release the compressed image buffer after giving it to the decoder (#9825) The codec will retain a reference to the decoded frame so it can be returned in repeated calls to getNextFrame. Also change the compressed image buffer to use an anonymous mapping on Android. This is a workaround for high consumption of native heap pages by Android's malloc when buffers are allocated on one thread and freed on another thread. See https://github.com/flutter/flutter/issues/36079 --- lib/ui/painting/codec.cc | 53 ++++++++++++++++++++++- lib/ui/painting/single_frame_codec.cc | 60 +++++++++++++++++++++------ lib/ui/painting/single_frame_codec.h | 5 +++ 3 files changed, 104 insertions(+), 14 deletions(-) diff --git a/lib/ui/painting/codec.cc b/lib/ui/painting/codec.cc index b181837e3f51f..1677ddc590492 100644 --- a/lib/ui/painting/codec.cc +++ b/lib/ui/painting/codec.cc @@ -21,12 +21,18 @@ #include "third_party/tonic/logging/dart_invoke.h" #include "third_party/tonic/typed_data/typed_list.h" +#if OS_ANDROID +#include +#endif + using tonic::DartInvoke; using tonic::DartPersistentValue; using tonic::ToDart; namespace flutter { +namespace { + // This needs to be kept in sync with _kDoNotResizeDimension in painting.dart const int kDoNotResizeDimension = -1; @@ -36,6 +42,51 @@ enum PixelFormat { kBGRA8888, }; +#if OS_ANDROID + +// Compressed image buffers are allocated on the UI thread but are deleted on a +// decoder worker thread. Android's implementation of malloc appears to +// continue growing the native heap size when the allocating thread is +// different from the freeing thread. To work around this, create an SkData +// backed by an anonymous mapping. +sk_sp MakeSkDataWithCopy(const void* data, size_t length) { + if (length == 0) { + return SkData::MakeEmpty(); + } + + size_t mapping_length = length + sizeof(size_t); + void* mapping = ::mmap(nullptr, mapping_length, PROT_READ | PROT_WRITE, + MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); + + if (mapping == MAP_FAILED) { + return SkData::MakeEmpty(); + } + + *reinterpret_cast(mapping) = mapping_length; + void* mapping_data = reinterpret_cast(mapping) + sizeof(size_t); + ::memcpy(mapping_data, data, length); + + SkData::ReleaseProc proc = [](const void* ptr, void* context) { + size_t* size_ptr = reinterpret_cast(context); + FML_DCHECK(ptr == size_ptr + 1); + if (::munmap(const_cast(context), *size_ptr) == -1) { + FML_LOG(ERROR) << "munmap of codec SkData failed"; + } + }; + + return SkData::MakeWithProc(mapping_data, length, proc, mapping); +} + +#else + +sk_sp MakeSkDataWithCopy(const void* data, size_t length) { + return SkData::MakeWithCopy(data, length); +} + +#endif // OS_ANDROID + +} // anonymous namespace + static std::variant ConvertImageInfo( Dart_Handle image_info_handle, Dart_NativeArguments args) { @@ -129,7 +180,7 @@ static void InstantiateImageCodec(Dart_NativeArguments args) { Dart_SetReturnValue(args, exception); return; } - buffer = SkData::MakeWithCopy(list.data(), list.num_elements()); + buffer = MakeSkDataWithCopy(list.data(), list.num_elements()); } if (image_info) { diff --git a/lib/ui/painting/single_frame_codec.cc b/lib/ui/painting/single_frame_codec.cc index 8036f767ead7b..b992a95e520e9 100644 --- a/lib/ui/painting/single_frame_codec.cc +++ b/lib/ui/painting/single_frame_codec.cc @@ -11,7 +11,7 @@ namespace flutter { SingleFrameCodec::SingleFrameCodec(ImageDecoder::ImageDescriptor descriptor) - : descriptor_(std::move(descriptor)) {} + : status_(Status::kNew), descriptor_(std::move(descriptor)) {} SingleFrameCodec::~SingleFrameCodec() = default; @@ -28,26 +28,40 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle callback_handle) { return tonic::ToDart("Callback must be a function"); } + if (status_ == Status::kComplete) { + tonic::DartInvoke(callback_handle, {tonic::ToDart(cached_frame_)}); + return Dart_Null(); + } + // This has to be valid because this method is called from Dart. auto dart_state = UIDartState::Current(); + pending_callbacks_.emplace_back(dart_state, callback_handle); + + if (status_ == Status::kInProgress) { + // Another call to getNextFrame is in progress and will invoke the + // pending callbacks when decoding completes. + return Dart_Null(); + } + auto decoder = dart_state->GetImageDecoder(); if (!decoder) { return tonic::ToDart("Image decoder not available."); } - auto raw_callback = new DartPersistentValue(dart_state, callback_handle); + auto raw_codec_wrapper = new DartPersistentValue( + dart_state, Dart_HandleFromWeakPersistent(dart_wrapper())); - // We dont want to to put the raw callback in a lambda capture because we have + // We dont want to to put the raw codec in a lambda capture because we have // to mutate (i.e destroy) it in the callback. Using MakeCopyable will create // a shared pointer for the captures which can be destroyed on any thread. But // we have to ensure that the DartPersistentValue is only destroyed on the UI // thread. - decoder->Decode(descriptor_, [raw_callback](auto image) { - std::unique_ptr callback(raw_callback); + decoder->Decode(descriptor_, [raw_codec_wrapper](auto image) { + std::unique_ptr codec_wrapper(raw_codec_wrapper); - auto state = callback->dart_state().lock(); + auto state = codec_wrapper->dart_state().lock(); if (!state) { // This is probably because the isolate has been terminated before the @@ -58,22 +72,42 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle callback_handle) { tonic::DartState::Scope scope(state.get()); - auto canvas_image = fml::MakeRefCounted(); - canvas_image->set_image(std::move(image)); + SingleFrameCodec* codec = tonic::DartConverter::FromDart( + codec_wrapper->value()); - auto frame_info = fml::MakeRefCounted(std::move(canvas_image), - 0 /* duration */); + if (image.get()) { + auto canvas_image = fml::MakeRefCounted(); + canvas_image->set_image(std::move(image)); - tonic::DartInvoke(callback->value(), {tonic::ToDart(frame_info)}); + codec->cached_frame_ = fml::MakeRefCounted( + std::move(canvas_image), 0 /* duration */); + } + + Dart_Handle frame = tonic::ToDart(codec->cached_frame_); + for (const DartPersistentValue& callback : codec->pending_callbacks_) { + tonic::DartInvoke(callback.value(), {frame}); + } + codec->pending_callbacks_.clear(); + + codec->status_ = Status::kComplete; }); + // The encoded data is no longer needed now that it has been handed off + // to the decoder. + descriptor_.data.reset(); + + status_ = Status::kInProgress; + return Dart_Null(); } size_t SingleFrameCodec::GetAllocationSize() { const auto& data = descriptor_.data; - auto data_byte_size = data ? data->size() : 0; - return data_byte_size + sizeof(this); + const auto data_byte_size = data ? data->size() : 0; + const auto frame_byte_size = (cached_frame_ && cached_frame_->image()) + ? cached_frame_->image()->GetAllocationSize() + : 0; + return data_byte_size + frame_byte_size + sizeof(this); } } // namespace flutter diff --git a/lib/ui/painting/single_frame_codec.h b/lib/ui/painting/single_frame_codec.h index 9684eef11219b..b01638c71ce63 100644 --- a/lib/ui/painting/single_frame_codec.h +++ b/lib/ui/painting/single_frame_codec.h @@ -7,6 +7,7 @@ #include "flutter/fml/macros.h" #include "flutter/lib/ui/painting/codec.h" +#include "flutter/lib/ui/painting/frame_info.h" #include "flutter/lib/ui/painting/image_decoder.h" namespace flutter { @@ -30,7 +31,11 @@ class SingleFrameCodec : public Codec { size_t GetAllocationSize() override; private: + enum class Status { kNew, kInProgress, kComplete }; + Status status_; ImageDecoder::ImageDescriptor descriptor_; + fml::RefPtr cached_frame_; + std::vector pending_callbacks_; FML_FRIEND_MAKE_REF_COUNTED(SingleFrameCodec); FML_FRIEND_REF_COUNTED_THREAD_SAFE(SingleFrameCodec);