Skip to content

Commit

Permalink
Use hint freed specifically for image disposal (flutter#20754)
Browse files Browse the repository at this point in the history
* Use hint freed specifically for image disposal
  • Loading branch information
dnfield authored Sep 2, 2020
1 parent f6920da commit 634e499
Show file tree
Hide file tree
Showing 22 changed files with 351 additions and 20 deletions.
2 changes: 2 additions & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ FILE: ../../../flutter/lib/ui/fixtures/hello_loop_2.webp
FILE: ../../../flutter/lib/ui/fixtures/ui_test.dart
FILE: ../../../flutter/lib/ui/geometry.dart
FILE: ../../../flutter/lib/ui/hash_codes.dart
FILE: ../../../flutter/lib/ui/hint_freed_delegate.h
FILE: ../../../flutter/lib/ui/hooks.dart
FILE: ../../../flutter/lib/ui/io_manager.h
FILE: ../../../flutter/lib/ui/isolate_name_server.dart
Expand Down Expand Up @@ -337,6 +338,7 @@ FILE: ../../../flutter/lib/ui/painting/image_decoder.h
FILE: ../../../flutter/lib/ui/painting/image_decoder_unittests.cc
FILE: ../../../flutter/lib/ui/painting/image_descriptor.cc
FILE: ../../../flutter/lib/ui/painting/image_descriptor.h
FILE: ../../../flutter/lib/ui/painting/image_dispose_unittests.cc
FILE: ../../../flutter/lib/ui/painting/image_encoding.cc
FILE: ../../../flutter/lib/ui/painting/image_encoding.h
FILE: ../../../flutter/lib/ui/painting/image_encoding_unittests.cc
Expand Down
1 change: 1 addition & 0 deletions lib/ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ if (enable_unittests) {
public_configs = [ "//flutter:export_dynamic_symbols" ]

sources = [
"painting/image_dispose_unittests.cc",
"painting/image_encoding_unittests.cc",
"painting/vertices_unittests.cc",
"window/platform_configuration_unittests.cc",
Expand Down
59 changes: 59 additions & 0 deletions lib/ui/fixtures/ui_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:async';
import 'dart:typed_data';
import 'dart:ui';

Expand Down Expand Up @@ -73,3 +74,61 @@ Future<void> encodeImageProducesExternalUint8List() async {
void _encodeImage(Image i, int format, void Function(Uint8List result))
native 'EncodeImage';
void _validateExternal(Uint8List result) native 'ValidateExternal';

@pragma('vm:entry-point')
Future<void> pumpImage() async {
const int width = 6000;
const int height = 6000;
final Completer<Image> completer = Completer<Image>();
decodeImageFromPixels(
Uint8List.fromList(List<int>.filled(width * height * 4, 0xFF)),
width,
height,
PixelFormat.rgba8888,
(Image image) => completer.complete(image),
);
final Image image = await completer.future;

final FrameCallback renderBlank = (Duration duration) {
image.dispose();

final PictureRecorder recorder = PictureRecorder();
final Canvas canvas = Canvas(recorder);
canvas.drawRect(Rect.largest, Paint());
final Picture picture = recorder.endRecording();

final SceneBuilder builder = SceneBuilder();
builder.addPicture(Offset.zero, picture);

final Scene scene = builder.build();
window.render(scene);
scene.dispose();
window.onBeginFrame = (Duration duration) {
window.onDrawFrame = _onBeginFrameDone;
};
window.scheduleFrame();
};

final FrameCallback renderImage = (Duration duration) {
final PictureRecorder recorder = PictureRecorder();
final Canvas canvas = Canvas(recorder);
canvas.drawImage(image, Offset.zero, Paint());
final Picture picture = recorder.endRecording();

final SceneBuilder builder = SceneBuilder();
builder.addPicture(Offset.zero, picture);

_captureImageAndPicture(image, picture);

final Scene scene = builder.build();
window.render(scene);
scene.dispose();
window.onBeginFrame = renderBlank;
window.scheduleFrame();
};

window.onBeginFrame = renderImage;
window.scheduleFrame();
}
void _captureImageAndPicture(Image image, Picture picture) native 'CaptureImageAndPicture';
Future<void> _onBeginFrameDone() native 'OnBeginFrameDone';
24 changes: 24 additions & 0 deletions lib/ui/hint_freed_delegate.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef FLUTTER_LIB_UI_HINT_FREED_DELEGATE_H_
#define FLUTTER_LIB_UI_HINT_FREED_DELEGATE_H_

namespace flutter {

class HintFreedDelegate {
public:
//----------------------------------------------------------------------------
/// @brief Notifies the engine that native bytes might be freed if a
/// garbage collection ran at the next NotifyIdle period.
///
/// @param[in] size The number of bytes freed. This size adds to any
/// previously supplied value, rather than replacing.
///
virtual void HintFreed(size_t size) = 0;
};

} // namespace flutter

#endif // FLUTTER_LIB_UI_HINT_FREED_DELEGATE_H_
4 changes: 4 additions & 0 deletions lib/ui/painting/image.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ Dart_Handle CanvasImage::toByteData(int format, Dart_Handle callback) {
}

void CanvasImage::dispose() {
auto hint_freed_delegate = UIDartState::Current()->GetHintFreedDelegate();
if (hint_freed_delegate) {
hint_freed_delegate->HintFreed(GetAllocationSize());
}
image_.reset();
ClearDartWrapper();
}
Expand Down
121 changes: 121 additions & 0 deletions lib/ui/painting/image_dispose_unittests.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#define FML_USED_ON_EMBEDDER

#include "flutter/common/task_runners.h"
#include "flutter/fml/synchronization/waitable_event.h"
#include "flutter/lib/ui/painting/image.h"
#include "flutter/lib/ui/painting/picture.h"
#include "flutter/runtime/dart_vm.h"
#include "flutter/shell/common/shell_test.h"
#include "flutter/shell/common/thread_host.h"
#include "flutter/testing/testing.h"

namespace flutter {
namespace testing {

class ImageDisposeTest : public ShellTest {
public:
template <class T>
T* GetNativePeer(Dart_NativeArguments args, int index) {
auto handle = Dart_GetNativeArgument(args, index);
intptr_t peer = 0;
EXPECT_FALSE(Dart_IsError(Dart_GetNativeInstanceField(
handle, tonic::DartWrappable::kPeerIndex, &peer)));
return reinterpret_cast<T*>(peer);
}

// Used to wait on Dart callbacks or Shell task runner flushing
fml::AutoResetWaitableEvent message_latch_;

fml::AutoResetWaitableEvent picture_finalizer_latch_;
static void picture_finalizer(void* isolate_callback_data, void* peer) {
auto latch = reinterpret_cast<fml::AutoResetWaitableEvent*>(peer);
latch->Signal();
}

sk_sp<SkPicture> current_picture_;
sk_sp<SkImage> current_image_;
};

TEST_F(ImageDisposeTest, ImageReleasedAfterFrame) {
auto native_capture_image_and_picture = [&](Dart_NativeArguments args) {
CanvasImage* image = GetNativePeer<CanvasImage>(args, 0);
Picture* picture = GetNativePeer<Picture>(args, 1);
ASSERT_FALSE(image->image()->unique());
ASSERT_FALSE(picture->picture()->unique());
current_image_ = image->image();
current_picture_ = picture->picture();

Dart_NewFinalizableHandle(Dart_GetNativeArgument(args, 1),
&picture_finalizer_latch_, 0, &picture_finalizer);
};

auto native_on_begin_frame_done = [&](Dart_NativeArguments args) {
message_latch_.Signal();
};

Settings settings = CreateSettingsForFixture();
auto task_runner = CreateNewThread();
TaskRunners task_runners("test", // label
GetCurrentTaskRunner(), // platform
task_runner, // raster
task_runner, // ui
task_runner // io
);

AddNativeCallback("CaptureImageAndPicture",
CREATE_NATIVE_ENTRY(native_capture_image_and_picture));
AddNativeCallback("OnBeginFrameDone",
CREATE_NATIVE_ENTRY(native_on_begin_frame_done));

std::unique_ptr<Shell> shell = CreateShell(std::move(settings), task_runners);

ASSERT_TRUE(shell->IsSetup());

SetViewportMetrics(shell.get(), 800, 600);

shell->GetPlatformView()->NotifyCreated();

auto configuration = RunConfiguration::InferFromSettings(settings);
configuration.SetEntrypoint("pumpImage");

shell->RunEngine(std::move(configuration), [&](auto result) {
ASSERT_EQ(result, Engine::RunStatus::Success);
});

message_latch_.Wait();

ASSERT_TRUE(current_picture_);
ASSERT_TRUE(current_image_);

// Simulate a large notify idle, as the animator would do
// when it has no frames left.
// On slower machines, this is especially important - we capture that
// this happens normally in devicelab bnechmarks like large_image_changer.
NotifyIdle(shell.get(), Dart_TimelineGetMicros() + 100000);

picture_finalizer_latch_.Wait();

// Force a drain the SkiaUnrefQueue.
message_latch_.Reset();
task_runner->PostTask([&, io_manager = shell->GetIOManager()]() {
io_manager->GetSkiaUnrefQueue()->Drain();
message_latch_.Signal();
});
message_latch_.Wait();

EXPECT_TRUE(current_picture_->unique());
current_picture_.reset();

EXPECT_TRUE(current_image_->unique());
current_image_.reset();

shell->GetPlatformView()->NotifyDestroyed();
DestroyShell(std::move(shell), std::move(task_runners));
}

} // namespace testing
} // namespace flutter
6 changes: 6 additions & 0 deletions lib/ui/ui_dart_state.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ UIDartState::UIDartState(
TaskObserverAdd add_callback,
TaskObserverRemove remove_callback,
fml::WeakPtr<SnapshotDelegate> snapshot_delegate,
fml::WeakPtr<HintFreedDelegate> hint_freed_delegate,
fml::WeakPtr<IOManager> io_manager,
fml::RefPtr<SkiaUnrefQueue> skia_unref_queue,
fml::WeakPtr<ImageDecoder> image_decoder,
Expand All @@ -31,6 +32,7 @@ UIDartState::UIDartState(
add_callback_(std::move(add_callback)),
remove_callback_(std::move(remove_callback)),
snapshot_delegate_(std::move(snapshot_delegate)),
hint_freed_delegate_(std::move(hint_freed_delegate)),
io_manager_(std::move(io_manager)),
skia_unref_queue_(std::move(skia_unref_queue)),
image_decoder_(std::move(image_decoder)),
Expand Down Expand Up @@ -136,6 +138,10 @@ fml::WeakPtr<SnapshotDelegate> UIDartState::GetSnapshotDelegate() const {
return snapshot_delegate_;
}

fml::WeakPtr<HintFreedDelegate> UIDartState::GetHintFreedDelegate() const {
return hint_freed_delegate_;
}

fml::WeakPtr<GrDirectContext> UIDartState::GetResourceContext() const {
if (!io_manager_) {
return {};
Expand Down
5 changes: 5 additions & 0 deletions lib/ui/ui_dart_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "flutter/fml/build_config.h"
#include "flutter/fml/memory/weak_ptr.h"
#include "flutter/fml/synchronization/waitable_event.h"
#include "flutter/lib/ui/hint_freed_delegate.h"
#include "flutter/lib/ui/io_manager.h"
#include "flutter/lib/ui/isolate_name_server/isolate_name_server.h"
#include "flutter/lib/ui/painting/image_decoder.h"
Expand Down Expand Up @@ -60,6 +61,8 @@ class UIDartState : public tonic::DartState {

fml::WeakPtr<SnapshotDelegate> GetSnapshotDelegate() const;

fml::WeakPtr<HintFreedDelegate> GetHintFreedDelegate() const;

fml::WeakPtr<GrDirectContext> GetResourceContext() const;

fml::WeakPtr<ImageDecoder> GetImageDecoder() const;
Expand Down Expand Up @@ -87,6 +90,7 @@ class UIDartState : public tonic::DartState {
TaskObserverAdd add_callback,
TaskObserverRemove remove_callback,
fml::WeakPtr<SnapshotDelegate> snapshot_delegate,
fml::WeakPtr<HintFreedDelegate> hint_freed_delegate,
fml::WeakPtr<IOManager> io_manager,
fml::RefPtr<SkiaUnrefQueue> skia_unref_queue,
fml::WeakPtr<ImageDecoder> image_decoder,
Expand All @@ -113,6 +117,7 @@ class UIDartState : public tonic::DartState {
const TaskObserverAdd add_callback_;
const TaskObserverRemove remove_callback_;
fml::WeakPtr<SnapshotDelegate> snapshot_delegate_;
fml::WeakPtr<HintFreedDelegate> hint_freed_delegate_;
fml::WeakPtr<IOManager> io_manager_;
fml::RefPtr<SkiaUnrefQueue> skia_unref_queue_;
fml::WeakPtr<ImageDecoder> image_decoder_;
Expand Down
25 changes: 16 additions & 9 deletions runtime/dart_isolate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(
TaskRunners task_runners,
std::unique_ptr<PlatformConfiguration> platform_configuration,
fml::WeakPtr<SnapshotDelegate> snapshot_delegate,
fml::WeakPtr<HintFreedDelegate> hint_freed_delegate,
fml::WeakPtr<IOManager> io_manager,
fml::RefPtr<SkiaUnrefQueue> unref_queue,
fml::WeakPtr<ImageDecoder> image_decoder,
Expand All @@ -84,15 +85,16 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(

auto isolate_data = std::make_unique<std::shared_ptr<DartIsolate>>(
std::shared_ptr<DartIsolate>(new DartIsolate(
settings, // settings
task_runners, // task runners
std::move(snapshot_delegate), // snapshot delegate
std::move(io_manager), // IO manager
std::move(unref_queue), // Skia unref queue
std::move(image_decoder), // Image Decoder
advisory_script_uri, // advisory URI
advisory_script_entrypoint, // advisory entrypoint
true // is_root_isolate
settings, // settings
task_runners, // task runners
std::move(snapshot_delegate), // snapshot delegate
std::move(hint_freed_delegate), // hint freed delegate
std::move(io_manager), // IO manager
std::move(unref_queue), // Skia unref queue
std::move(image_decoder), // Image Decoder
advisory_script_uri, // advisory URI
advisory_script_entrypoint, // advisory entrypoint
true // is_root_isolate
)));

DartErrorString error;
Expand Down Expand Up @@ -120,6 +122,7 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(
DartIsolate::DartIsolate(const Settings& settings,
TaskRunners task_runners,
fml::WeakPtr<SnapshotDelegate> snapshot_delegate,
fml::WeakPtr<HintFreedDelegate> hint_freed_delegate,
fml::WeakPtr<IOManager> io_manager,
fml::RefPtr<SkiaUnrefQueue> unref_queue,
fml::WeakPtr<ImageDecoder> image_decoder,
Expand All @@ -130,6 +133,7 @@ DartIsolate::DartIsolate(const Settings& settings,
settings.task_observer_add,
settings.task_observer_remove,
std::move(snapshot_delegate),
std::move(hint_freed_delegate),
std::move(io_manager),
std::move(unref_queue),
std::move(image_decoder),
Expand Down Expand Up @@ -603,6 +607,7 @@ Dart_Isolate DartIsolate::DartCreateAndStartServiceIsolate(
null_task_runners, // task runners
nullptr, // platform_configuration
{}, // snapshot delegate
{}, // Hint freed delegate
{}, // IO Manager
{}, // Skia unref queue
{}, // Image Decoder
Expand Down Expand Up @@ -706,6 +711,7 @@ Dart_Isolate DartIsolate::DartIsolateGroupCreateCallback(
(*isolate_group_data)->GetSettings(), // settings
null_task_runners, // task_runners
fml::WeakPtr<SnapshotDelegate>{}, // snapshot_delegate
fml::WeakPtr<HintFreedDelegate>{}, // hint_freed_delegate
fml::WeakPtr<IOManager>{}, // io_manager
fml::RefPtr<SkiaUnrefQueue>{}, // unref_queue
fml::WeakPtr<ImageDecoder>{}, // image_decoder
Expand Down Expand Up @@ -749,6 +755,7 @@ bool DartIsolate::DartIsolateInitializeCallback(void** child_callback_data,
(*isolate_group_data)->GetSettings(), // settings
null_task_runners, // task_runners
fml::WeakPtr<SnapshotDelegate>{}, // snapshot_delegate
fml::WeakPtr<HintFreedDelegate>{}, // hint_freed_delegate
fml::WeakPtr<IOManager>{}, // io_manager
fml::RefPtr<SkiaUnrefQueue>{}, // unref_queue
fml::WeakPtr<ImageDecoder>{}, // image_decoder
Expand Down
Loading

0 comments on commit 634e499

Please sign in to comment.