Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Use hint freed specifically for image disposal
  • Loading branch information
dnfield committed Aug 25, 2020
commit fa3cce82866d7853625712df4c28803b739bd347
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_
6 changes: 0 additions & 6 deletions lib/ui/painting/canvas.cc
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,6 @@ void Canvas::clipPath(const CanvasPath* path, bool doAntiAlias) {
ToDart("Canvas.clipPath called with non-genuine Path."));
return;
}
external_allocation_size_ += path->path().approximateBytesUsed();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will the engine no longer be reporting these sizes as the external allocation size for Dart objects with native fields? Or does this only affect HintFreed? (Same question here and below.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. I can split this into another patch, but basically this seems like it was a mistake. We've already moved the reporting of images for this.

The basic problem is that from scene to scene, we might be reusing these things, and end up reporting vastly inflated numbers to the VM, and then that creates lots of wasteful garbage collections.

canvas_->clipPath(path->path(), doAntiAlias);
}

Expand Down Expand Up @@ -310,7 +309,6 @@ void Canvas::drawPath(const CanvasPath* path,
ToDart("Canvas.drawPath called with non-genuine Path."));
return;
}
external_allocation_size_ += path->path().approximateBytesUsed();
canvas_->drawPath(path->path(), *paint.paint());
}

Expand Down Expand Up @@ -391,7 +389,6 @@ void Canvas::drawPicture(Picture* picture) {
ToDart("Canvas.drawPicture called with non-genuine Picture."));
return;
}
external_allocation_size_ += picture->GetAllocationSize();
canvas_->drawPicture(picture->picture().get());
}

Expand Down Expand Up @@ -424,7 +421,6 @@ void Canvas::drawVertices(const Vertices* vertices,
ToDart("Canvas.drawVertices called with non-genuine Vertices."));
return;
}
external_allocation_size_ += vertices->GetAllocationSize();
canvas_->drawVertices(vertices->vertices(), blend_mode, *paint.paint());
}

Expand Down Expand Up @@ -453,7 +449,6 @@ void Canvas::drawAtlas(const Paint& paint,
static_assert(sizeof(SkRect) == sizeof(float) * 4,
"SkRect doesn't use floats.");

external_allocation_size_ += atlas->GetAllocationSize();
canvas_->drawAtlas(
skImage.get(), reinterpret_cast<const SkRSXform*>(transforms.data()),
reinterpret_cast<const SkRect*>(rects.data()),
Expand All @@ -477,7 +472,6 @@ void Canvas::drawShadow(const CanvasPath* path,
->window()
->viewport_metrics()
.device_pixel_ratio;
external_allocation_size_ += path->path().approximateBytesUsed();
flutter::PhysicalShapeLayer::DrawShadow(canvas_, path->path(), color,
elevation, transparentOccluder, dpr);
}
Expand Down
3 changes: 0 additions & 3 deletions lib/ui/painting/canvas.h
Original file line number Diff line number Diff line change
Expand Up @@ -169,16 +169,13 @@ class Canvas : public RefCountedDartWrappable<Canvas> {

static void RegisterNatives(tonic::DartLibraryNatives* natives);

size_t external_allocation_size() const { return external_allocation_size_; }

private:
explicit Canvas(SkCanvas* canvas);

// The SkCanvas is supplied by a call to SkPictureRecorder::beginRecording,
// which does not transfer ownership. For this reason, we hold a raw
// pointer and manually set to null in Clear.
SkCanvas* canvas_;
size_t external_allocation_size_ = 0;
};

} // namespace flutter
Expand Down
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
18 changes: 7 additions & 11 deletions lib/ui/painting/picture.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,17 @@ IMPLEMENT_WRAPPERTYPEINFO(ui, Picture);

DART_BIND_ALL(Picture, FOR_EACH_BINDING)

fml::RefPtr<Picture> Picture::Create(Dart_Handle dart_handle,
flutter::SkiaGPUObject<SkPicture> picture,
size_t external_allocation_size) {
auto canvas_picture = fml::MakeRefCounted<Picture>(std::move(picture),
external_allocation_size);
fml::RefPtr<Picture> Picture::Create(
Dart_Handle dart_handle,
flutter::SkiaGPUObject<SkPicture> picture) {
auto canvas_picture = fml::MakeRefCounted<Picture>(std::move(picture));

canvas_picture->AssociateWithDartWrapper(dart_handle);
return canvas_picture;
}

Picture::Picture(flutter::SkiaGPUObject<SkPicture> picture,
size_t external_allocation_size)
: picture_(std::move(picture)),
external_allocation_size_(external_allocation_size) {}
Picture::Picture(flutter::SkiaGPUObject<SkPicture> picture)
: picture_(std::move(picture)) {}

Picture::~Picture() = default;

Expand All @@ -62,8 +59,7 @@ void Picture::dispose() {

size_t Picture::GetAllocationSize() const {
if (auto picture = picture_.get()) {
return picture->approximateBytesUsed() + sizeof(Picture) +
external_allocation_size_;
return picture->approximateBytesUsed() + sizeof(Picture);
} else {
return sizeof(Picture);
}
Expand Down
9 changes: 2 additions & 7 deletions lib/ui/painting/picture.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ class Picture : public RefCountedDartWrappable<Picture> {
public:
~Picture() override;
static fml::RefPtr<Picture> Create(Dart_Handle dart_handle,
flutter::SkiaGPUObject<SkPicture> picture,
size_t external_allocation_size);
flutter::SkiaGPUObject<SkPicture> picture);

sk_sp<SkPicture> picture() const { return picture_.get(); }

Expand All @@ -45,14 +44,10 @@ class Picture : public RefCountedDartWrappable<Picture> {
uint32_t height,
Dart_Handle raw_image_callback);

size_t external_allocation_size() const { return external_allocation_size_; }

private:
Picture(flutter::SkiaGPUObject<SkPicture> picture,
size_t external_allocation_size_);
Picture(flutter::SkiaGPUObject<SkPicture> picture);

flutter::SkiaGPUObject<SkPicture> picture_;
size_t external_allocation_size_;
};

} // namespace flutter
Expand Down
8 changes: 3 additions & 5 deletions lib/ui/painting/picture_recorder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,9 @@ fml::RefPtr<Picture> PictureRecorder::endRecording(Dart_Handle dart_picture) {
return nullptr;
}

fml::RefPtr<Picture> picture =
Picture::Create(dart_picture,
UIDartState::CreateGPUObject(
picture_recorder_.finishRecordingAsPicture()),
canvas_->external_allocation_size());
fml::RefPtr<Picture> picture = Picture::Create(
dart_picture, UIDartState::CreateGPUObject(
picture_recorder_.finishRecordingAsPicture()));

canvas_->Invalidate();
canvas_ = nullptr;
Expand Down
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
3 changes: 3 additions & 0 deletions runtime/dart_isolate.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#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"
Expand Down Expand Up @@ -194,6 +195,7 @@ class DartIsolate : public UIDartState {
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> skia_unref_queue,
fml::WeakPtr<ImageDecoder> image_decoder,
Expand Down Expand Up @@ -404,6 +406,7 @@ class DartIsolate : public UIDartState {
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 Down
2 changes: 2 additions & 0 deletions runtime/dart_isolate_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ TEST_F(DartIsolateTest, RootIsolateCreationAndShutdown) {
std::move(task_runners), // task runners
nullptr, // window
{}, // snapshot delegate
{}, // hint freed delegate
{}, // io manager
{}, // unref queue
{}, // image decoder
Expand Down Expand Up @@ -85,6 +86,7 @@ TEST_F(DartIsolateTest, IsolateShutdownCallbackIsInIsolateScope) {
std::move(task_runners), // task runners
nullptr, // window
{}, // snapshot delegate
{}, // hint freed delegate
{}, // io manager
{}, // unref queue
{}, // image decoder
Expand Down
1 change: 1 addition & 0 deletions runtime/dart_lifecycle_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ static std::shared_ptr<DartIsolate> CreateAndRunRootIsolate(
runners, // task_runners
{}, // window
{}, // snapshot_delegate
{}, // hint_freed_delegate
{}, // io_manager
{}, // unref_queue
{}, // image_decoder
Expand Down
Loading