DEV/flutter_flutter
1
0
Fork 0
mirror of https://github.com/flutter/flutter.git synced 2026-02-20 02:29:02 +08:00
Code Issues Packages Projects Releases Wiki Activity

Remove tech debt related to image disposal and layer GC (flutter/engine#26870)

Browse Source 
* remove tech debt

* Fix test
This commit is contained in:
Dan Field 2021-06-25 15:33:08 -07:00 committed by GitHub
parent f4e00ac91b
commit 3bc015dc10
17 changed files with 32 additions and 188 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
engine/src/flutter/ci/licenses_golden/licenses_flutter
View File

@ -327,7 +327,6 @@ 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/hooks_unittests.cc
FILE: ../../../flutter/lib/ui/io_manager.h

38
engine/src/flutter/lib/ui/fixtures/ui_test.dart
View File

@ -8,6 +8,10 @@ import 'dart:ui';
void main() {}
/// Mutiple tests use this to signal to the C++ side that they are ready for
/// validation.
void _finish() native 'Finish';
@pragma('vm:entry-point')
void validateSceneBuilderAndScene() {
final SceneBuilder builder = SceneBuilder();
@ -60,7 +64,6 @@ Future<void> createSingleFrameCodec() async {
_finish();
}
void _validateCodec(Codec codec) native 'ValidateCodec';
void _finish() native 'Finish';
@pragma('vm:entry-point')
void createVertices() {
@ -232,8 +235,8 @@ void _validateExternal(Uint8List result) native 'ValidateExternal';
@pragma('vm:entry-point')
Future<void> pumpImage() async {
const int width = 6000;
const int height = 6000;
const int width = 60;
const int height = 60;
final Completer<Image> completer = Completer<Image>();
decodeImageFromPixels(
Uint8List.fromList(List<int>.filled(width * height * 4, 0xFF)),
@ -243,34 +246,37 @@ Future<void> pumpImage() async {
(Image image) => completer.complete(image),
);
final Image image = await completer.future;
late Picture picture;
late OffsetEngineLayer layer;
final FrameCallback renderBlank = (Duration duration) {
void renderBlank(Duration duration) {
image.dispose();
picture.dispose();
layer.dispose();
final PictureRecorder recorder = PictureRecorder();
final Canvas canvas = Canvas(recorder);
canvas.drawRect(Rect.largest, Paint());
final Picture picture = recorder.endRecording();
canvas.drawPaint(Paint());
picture = recorder.endRecording();
final SceneBuilder builder = SceneBuilder();
layer = builder.pushOffset(0, 0);
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) {
_finish();
}
void renderImage(Duration duration) {
final PictureRecorder recorder = PictureRecorder();
final Canvas canvas = Canvas(recorder);
canvas.drawImage(image, Offset.zero, Paint());
final Picture picture = recorder.endRecording();
picture = recorder.endRecording();
final SceneBuilder builder = SceneBuilder();
layer = builder.pushOffset(0, 0);
builder.addPicture(Offset.zero, picture);
_captureImageAndPicture(image, picture);
@ -278,15 +284,15 @@ Future<void> pumpImage() async {
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';
@pragma('vm:entry-point')
void hooksTests() {

24
engine/src/flutter/lib/ui/hint_freed_delegate.h
View File

@ -1,24 +0,0 @@
// 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_

10
engine/src/flutter/lib/ui/painting/engine_layer.cc
View File

@ -25,16 +25,6 @@ EngineLayer::EngineLayer(std::shared_ptr<flutter::ContainerLayer> layer)
EngineLayer::~EngineLayer() = default;
size_t EngineLayer::GetAllocationSize() const {
// TODO(dnfield): Remove this when scene disposal changes land in the
// framework. https://github.com/flutter/flutter/issues/81514
// Provide an approximation of the total memory impact of this object to the
// Dart GC. The ContainerLayer may hold references to a tree of other layers,
// which in turn may contain Skia objects.
return 3000;
};
void EngineLayer::dispose() {
layer_.reset();
ClearDartWrapper();

2
engine/src/flutter/lib/ui/painting/engine_layer.h
View File

@ -22,8 +22,6 @@ class EngineLayer : public RefCountedDartWrappable<EngineLayer> {
public:
~EngineLayer() override;
size_t GetAllocationSize() const override;
static void MakeRetained(Dart_Handle dart_handle,
std::shared_ptr<flutter::ContainerLayer> layer) {
auto engine_layer = fml::MakeRefCounted<EngineLayer>(layer);

6
engine/src/flutter/lib/ui/painting/image.cc
View File

@ -44,12 +44,6 @@ Dart_Handle CanvasImage::toByteData(int format, Dart_Handle callback) {
}
void CanvasImage::dispose() {
// TODO(dnfield): Remove the hint freed delegate once Picture disposal is in
// the framework https://github.com/flutter/flutter/issues/81514
auto hint_freed_delegate = UIDartState::Current()->GetHintFreedDelegate();
if (hint_freed_delegate) {
hint_freed_delegate->HintFreed(GetAllocationSize());
}
image_.reset();
ClearDartWrapper();
}

33
engine/src/flutter/lib/ui/painting/image_dispose_unittests.cc
View File

@ -30,23 +30,11 @@ class ImageDisposeTest : public ShellTest {
// 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,
#if defined(OS_FUCHSIA)
DISABLED_ImageReleasedAfterFrame
#else
ImageReleasedAfterFrame
#endif // defined(OS_FUCHSIA)
) {
TEST_F(ImageDisposeTest, ImageReleasedAfterFrameAndDisposePictureAndLayer) {
auto native_capture_image_and_picture = [&](Dart_NativeArguments args) {
auto image_handle = Dart_GetNativeArgument(args, 0);
auto native_image_handle =
@ -60,12 +48,9 @@ TEST_F(ImageDisposeTest,
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) {
auto native_finish = [&](Dart_NativeArguments args) {
message_latch_.Signal();
};
@ -80,8 +65,7 @@ TEST_F(ImageDisposeTest,
AddNativeCallback("CaptureImageAndPicture",
CREATE_NATIVE_ENTRY(native_capture_image_and_picture));
AddNativeCallback("OnBeginFrameDone",
CREATE_NATIVE_ENTRY(native_on_begin_frame_done));
AddNativeCallback("Finish", CREATE_NATIVE_ENTRY(native_finish));
std::unique_ptr<Shell> shell = CreateShell(std::move(settings), task_runners);
@ -103,15 +87,8 @@ TEST_F(ImageDisposeTest,
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.
// Force a drain the SkiaUnrefQueue. The engine does this normally as frames
// pump, but we force it here to make the test more deterministic.
message_latch_.Reset();
task_runner->PostTask([&, io_manager = shell->GetIOManager()]() {
io_manager->GetSkiaUnrefQueue()->Drain();

6
engine/src/flutter/lib/ui/ui_dart_state.cc
View File

@ -30,7 +30,6 @@ UIDartState::Context::Context(const TaskRunners& task_runners)
UIDartState::Context::Context(
const 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,
@ -40,7 +39,6 @@ UIDartState::Context::Context(
std::shared_ptr<VolatilePathTracker> volatile_path_tracker)
: task_runners(task_runners),
snapshot_delegate(snapshot_delegate),
hint_freed_delegate(hint_freed_delegate),
io_manager(io_manager),
unref_queue(unref_queue),
image_decoder(image_decoder),
@ -169,10 +167,6 @@ fml::WeakPtr<SnapshotDelegate> UIDartState::GetSnapshotDelegate() const {
return context_.snapshot_delegate;
}
fml::WeakPtr<HintFreedDelegate> UIDartState::GetHintFreedDelegate() const {
return context_.hint_freed_delegate;
}
fml::WeakPtr<GrDirectContext> UIDartState::GetResourceContext() const {
if (!context_.io_manager) {
return {};

8
engine/src/flutter/lib/ui/ui_dart_state.h
View File

@ -15,7 +15,6 @@
#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"
@ -46,7 +45,6 @@ class UIDartState : public tonic::DartState {
Context(const 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,
@ -65,10 +63,6 @@ class UIDartState : public tonic::DartState {
/// of Flutter view hierarchies.
fml::WeakPtr<SnapshotDelegate> snapshot_delegate;
/// The delegate used by the isolate to hint the Dart VM when additional
/// memory may be freed if a GC ran at the next NotifyIdle.
fml::WeakPtr<HintFreedDelegate> hint_freed_delegate;
/// The IO manager used by the isolate for asynchronous texture uploads.
fml::WeakPtr<IOManager> io_manager;
@ -126,8 +120,6 @@ 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;

2
engine/src/flutter/runtime/dart_isolate.cc
View File

@ -82,7 +82,6 @@ std::weak_ptr<DartIsolate> DartIsolate::SpawnIsolate(
const Settings& settings,
std::unique_ptr<PlatformConfiguration> platform_configuration,
fml::WeakPtr<SnapshotDelegate> snapshot_delegate,
fml::WeakPtr<HintFreedDelegate> hint_freed_delegate,
std::string advisory_script_uri,
std::string advisory_script_entrypoint,
Flags flags,
@ -103,7 +102,6 @@ std::weak_ptr<DartIsolate> DartIsolate::SpawnIsolate(
std::move(isolate_configration), //
UIDartState::Context{GetTaskRunners(), //
snapshot_delegate, //
hint_freed_delegate, //
GetIOManager(), //
GetSkiaUnrefQueue(), //
GetImageDecoder(), //

2
engine/src/flutter/runtime/dart_isolate.h
View File

@ -15,7 +15,6 @@
#include "flutter/fml/compiler_specific.h"
#include "flutter/fml/macros.h"
#include "flutter/fml/mapping.h"
#include "flutter/lib/ui/hint_freed_delegate.h"
#include "flutter/lib/ui/io_manager.h"
#include "flutter/lib/ui/snapshot_delegate.h"
#include "flutter/lib/ui/ui_dart_state.h"
@ -230,7 +229,6 @@ class DartIsolate : public UIDartState {
const Settings& settings,
std::unique_ptr<PlatformConfiguration> platform_configuration,
fml::WeakPtr<SnapshotDelegate> snapshot_delegate,
fml::WeakPtr<HintFreedDelegate> hint_freed_delegate,
std::string advisory_script_uri,
std::string advisory_script_entrypoint,
Flags flags,

1
engine/src/flutter/runtime/dart_isolate_unittests.cc
View File

@ -114,7 +114,6 @@ TEST_F(DartIsolateTest, SpawnIsolate) {
/*settings=*/vm_data->GetSettings(),
/*platform_configuration=*/nullptr,
/*snapshot_delegate=*/{},
/*hint_freed_delegate=*/{},
/*advisory_script_uri=*/"main.dart",
/*advisory_script_entrypoint=*/"main",
/*flags=*/DartIsolate::Flags{},

5
engine/src/flutter/runtime/runtime_controller.cc
View File

@ -195,7 +195,7 @@ bool RuntimeController::ReportTimings(std::vector<int64_t> timings) {
return false;
}
bool RuntimeController::NotifyIdle(int64_t deadline, size_t freed_hint) {
bool RuntimeController::NotifyIdle(int64_t deadline) {
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
if (!root_isolate) {
return false;
@ -203,9 +203,6 @@ bool RuntimeController::NotifyIdle(int64_t deadline, size_t freed_hint) {
tonic::DartState::Scope scope(root_isolate);
// Dart will use the freed hint at the next idle notification. Make sure to
// Update it with our latest value before calling NotifyIdle.
Dart_HintFreed(freed_hint);
Dart_NotifyIdle(deadline);
// Idle notifications being in isolate scope are part of the contract.

4
engine/src/flutter/runtime/runtime_controller.h
View File

@ -341,12 +341,10 @@ class RuntimeController : public PlatformConfigurationClient {
/// @param[in] deadline The deadline measures in microseconds against the
/// system's monotonic time. The clock can be accessed via
/// `Dart_TimelineGetMicros`.
/// @param[in] freed_hint A hint of the number of bytes potentially freed
/// since the last call to NotifyIdle if a GC were run.
///
/// @return If the idle notification was forwarded to the running isolate.
///
virtual bool NotifyIdle(int64_t deadline, size_t freed_hint);
virtual bool NotifyIdle(int64_t deadline);
//----------------------------------------------------------------------------
/// @brief Returns if the root isolate is running. The isolate must be

19
engine/src/flutter/shell/common/engine.cc
View File

@ -98,7 +98,6 @@ Engine::Engine(Delegate& delegate,
UIDartState::Context{
task_runners_, // task runners
std::move(snapshot_delegate), // snapshot delegate
GetWeakPtr(), // hint freed delegate
std::move(io_manager), // io manager
std::move(unref_queue), // Skia unref queue
image_decoder_.GetWeakPtr(), // image decoder
@ -231,27 +230,11 @@ void Engine::ReportTimings(std::vector<int64_t> timings) {
runtime_controller_->ReportTimings(std::move(timings));
}
void Engine::HintFreed(size_t size) {
hint_freed_bytes_since_last_call_ += size;
}
void Engine::NotifyIdle(int64_t deadline) {
auto trace_event = std::to_string(deadline - Dart_TimelineGetMicros());
TRACE_EVENT1("flutter", "Engine::NotifyIdle", "deadline_now_delta",
trace_event.c_str());
// Avoid asking the RuntimeController to call Dart_HintFreed more than once
// every 5 seconds.
// This is to avoid GCs happening too frequently e.g. when an animated GIF is
// playing and disposing of an image every frame.
fml::TimePoint now = delegate_.GetCurrentTimePoint();
fml::TimeDelta delta = now - last_hint_freed_call_time_;
size_t hint_freed_bytes = 0;
if (delta.ToMilliseconds() > 5000 && hint_freed_bytes_since_last_call_ > 0) {
hint_freed_bytes = hint_freed_bytes_since_last_call_;
hint_freed_bytes_since_last_call_ = 0;
last_hint_freed_call_time_ = now;
}
runtime_controller_->NotifyIdle(deadline, hint_freed_bytes);
runtime_controller_->NotifyIdle(deadline);
}
std::optional<uint32_t> Engine::GetUIIsolateReturnCode() {

10
engine/src/flutter/shell/common/engine.h
View File

@ -13,7 +13,6 @@
#include "flutter/fml/macros.h"
#include "flutter/fml/mapping.h"
#include "flutter/fml/memory/weak_ptr.h"
#include "flutter/lib/ui/hint_freed_delegate.h"
#include "flutter/lib/ui/painting/image_decoder.h"
#include "flutter/lib/ui/painting/image_generator_registry.h"
#include "flutter/lib/ui/semantics/custom_accessibility_action.h"
@ -73,9 +72,7 @@ namespace flutter {
/// name and it does happen to be one of the older classes in the
/// repository.
///
class Engine final : public RuntimeDelegate,
public HintFreedDelegate,
PointerDataDispatcher::Delegate {
class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {
public:
//----------------------------------------------------------------------------
/// @brief Indicates the result of the call to `Engine::Run`.
@ -505,9 +502,6 @@ class Engine final : public RuntimeDelegate,
///
void BeginFrame(fml::TimePoint frame_time);
// |HintFreedDelegate|
void HintFreed(size_t size) override;
//----------------------------------------------------------------------------
/// @brief Notifies the engine that the UI task runner is not expected to
/// undertake a new frame workload till a specified timepoint. The
@ -924,8 +918,6 @@ class Engine final : public RuntimeDelegate,
ImageDecoder image_decoder_;
ImageGeneratorRegistry image_generator_registry_;
TaskRunners task_runners_;
size_t hint_freed_bytes_since_last_call_ = 0;
fml::TimePoint last_hint_freed_call_time_;
fml::WeakPtrFactory<Engine> weak_factory_;
// |RuntimeDelegate|

49
engine/src/flutter/shell/common/engine_unittests.cc
View File

@ -71,7 +71,7 @@ class MockRuntimeController : public RuntimeController {
MOCK_METHOD3(LoadDartDeferredLibraryError,
void(intptr_t, const std::string, bool));
MOCK_CONST_METHOD0(GetDartVM, DartVM*());
MOCK_METHOD2(NotifyIdle, bool(int64_t, size_t));
MOCK_METHOD1(NotifyIdle, bool(int64_t));
};
std::unique_ptr<PlatformMessage> MakePlatformMessage(
@ -302,51 +302,4 @@ TEST_F(EngineTest, PassesLoadDartDeferredLibraryErrorToRuntime) {
});
}
TEST_F(EngineTest, NotifyIdleMoreThanOncePerFiveSeconds) {
PostUITaskSync([this] {
MockRuntimeDelegate client;
auto mock_runtime_controller =
std::make_unique<MockRuntimeController>(client, task_runners_);
// Verify we get an initial call with the bytes, since it's 10 seconds.
EXPECT_CALL(*mock_runtime_controller, NotifyIdle(200, 100)).Times(1);
// Verify that we do not get called again, since it's only been 3 more
// seconds.
EXPECT_CALL(*mock_runtime_controller, NotifyIdle(400, 0)).Times(1);
// Verify that we get called again, since it's now been 6 seconds.
EXPECT_CALL(*mock_runtime_controller, NotifyIdle(600, 300 + 500)).Times(1);
// Set up mocking of the timepoint logic. Calls at 10, 13, and 16 seconds.
EXPECT_CALL(delegate_, GetCurrentTimePoint())
.Times(3)
.WillOnce(::testing::Return(
fml::TimePoint::FromEpochDelta(fml::TimeDelta::FromSeconds(10))))
.WillOnce(::testing::Return(
fml::TimePoint::FromEpochDelta(fml::TimeDelta::FromSeconds(13))))
.WillOnce(::testing::Return(
fml::TimePoint::FromEpochDelta(fml::TimeDelta::FromSeconds(16))));
auto engine = std::make_unique<Engine>(
/*delegate=*/delegate_,
/*dispatcher_maker=*/dispatcher_maker_,
/*image_decoder_task_runner=*/image_decoder_task_runner_,
/*task_runners=*/task_runners_,
/*settings=*/settings_,
/*animator=*/std::move(animator_),
/*io_manager=*/io_manager_,
/*font_collection=*/std::make_shared<FontCollection>(),
/*runtime_controller=*/std::move(mock_runtime_controller));
// Verifications are above, since we std::moved the unique_ptr for the
// mock.
engine->HintFreed(100);
engine->NotifyIdle(200);
engine->HintFreed(300);
engine->NotifyIdle(400);
engine->HintFreed(500);
engine->NotifyIdle(600);
});
}
} // namespace flutter