Evicting unused cache entries before RasterCache actions #34627

ColdPaleLight · 2022-07-13T03:45:06Z

Delete the unused images before generating new cached images.
fix: flutter/flutter#94511

Add new method 'RasterCache::EvictUnusedCacheEntries', called before the LayerTree::TryToRasterCache method to remove cached images that are no longer used.
Tweak total_count and total_bytes of the RasterCacheMetrics.

- size_t total_count() const { return in_use_count + eviction_count; }
+ size_t total_count() const { return in_use_count; }

- size_t total_bytes() const { return in_use_bytes + eviction_bytes; }
+ size_t total_bytes() const { return in_use_bytes; }

Rename some methods.

Pre-launch Checklist

I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
I read the [Tree Hygiene] wiki page, which explains my responsibilities.
I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides].
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See [testing the engine] for instructions on
writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the [CLA].
All existing and new tests are passing.

flutter-dashboard · 2022-07-14T05:10:48Z

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

ColdPaleLight · 2022-07-14T21:50:02Z

cc @flar @JsouLiang

flar

This was one of my most anticipated follow-on opts for the refactor. It's looking good. Some minor nits that I'd like to hear back about before I approve.

flow/raster_cache.cc

flow/testing/mock_raster_cache.cc

JsouLiang · 2022-07-16T07:27:19Z

Could we provide some benchmarks data?

ColdPaleLight · 2022-07-19T07:24:05Z

Benchmark

I changed the following code, ColorFiltered ensures that the children are cached, and at the same time, its children are not stable in this scene.

diff --git a/dev/benchmarks/macrobenchmarks/lib/src/cull_opacity.dart b/dev/benchmarks/macrobenchmarks/lib/src/cull_opacity.dart
index 7eb04aedf2..355b95cca9 100644
--- a/dev/benchmarks/macrobenchmarks/lib/src/cull_opacity.dart
+++ b/dev/benchmarks/macrobenchmarks/lib/src/cull_opacity.dart
@@ -41,8 +41,8 @@ class _CullOpacityPageState extends State<CullOpacityPage> with SingleTickerProv
     return Stack(children: List<Widget>.generate(50, (int i) => Positioned(
       left: 0,
       top: (200 * i).toDouble() + _offsetY.value,
-      child: Opacity(
-        opacity: 0.5,
+      child: ColorFiltered(
+        colorFilter: ColorFilter.mode(Colors.green[300]!, BlendMode.luminosity),
         child: RepaintBoundary(
           child: Container(
             // Slightly change width to invalidate raster cache.

And then ran the benchmark
../../bin/cache/dart-sdk/bin/dart bin/run.dart --local-engine-src-path=/Users/zzj/Develop/upstream/engine/src --local-engine=android_profile -t cull_opacity_perf__timeline_summary --ab=4

result

═════════════════════════╡ ••• Final A/B results ••• ╞══════════════════════════

Score Average A (noise) Average B (noise) Speed-up
average_frame_build_time_millis 3.15 (1.59%) 3.65 (0.34%) 0.86x
worst_frame_build_time_millis 7.69 (8.98%) 8.97 (33.46%) 0.86x
90th_percentile_frame_build_time_millis 3.92 (0.22%) 4.56 (0.46%) 0.86x
99th_percentile_frame_build_time_millis 4.68 (8.03%) 5.19 (4.48%) 0.90x
average_frame_rasterizer_time_millis 6.44 (1.53%) 6.17 (0.55%) 1.04x
worst_frame_rasterizer_time_millis 19.14 (17.46%) 16.75 (1.28%) 1.14x
90th_percentile_frame_rasterizer_time_millis 10.70 (1.71%) 12.49 (0.80%) 0.86x
99th_percentile_frame_rasterizer_time_millis 15.14 (0.76%) 15.79 (0.61%) 0.96x
average_layer_cache_count 9.98 (0.03%) 4.99 (0.01%) 2.00x
90th_percentile_layer_cache_count 10.00 (0.00%) 5.00 (0.00%) 2.00x
99th_percentile_layer_cache_count 10.00 (0.00%) 5.00 (0.00%) 2.00x
worst_layer_cache_count 10.00 (0.00%) 5.00 (0.00%) 2.00x
average_layer_cache_memory 46.91 (0.03%) 23.45 (0.01%) 2.00x
90th_percentile_layer_cache_memory 47.17 (0.01%) 23.59 (0.00%) 2.00x
99th_percentile_layer_cache_memory 47.21 (0.01%) 23.61 (0.01%) 2.00x
worst_layer_cache_memory 47.23 (0.01%) 23.62 (0.01%) 2.00x
average_picture_cache_count 0.00 (0.00%) 0.00 (0.00%) NaNx
90th_percentile_picture_cache_count 0.00 (0.00%) 0.00 (0.00%) NaNx
99th_percentile_picture_cache_count 0.00 (0.00%) 0.00 (0.00%) NaNx
worst_picture_cache_count 0.00 (0.00%) 0.00 (0.00%) NaNx
average_picture_cache_memory 0.00 (0.00%) 0.00 (0.00%) NaNx
90th_percentile_picture_cache_memory 0.00 (0.00%) 0.00 (0.00%) NaNx
99th_percentile_picture_cache_memory 0.00 (0.00%) 0.00 (0.00%) NaNx
worst_picture_cache_memory 0.00 (0.00%) 0.00 (0.00%) NaNx
new_gen_gc_count 86.50 (1.00%) 152.00 (0.00%) 0.57x
old_gen_gc_count 0.00 (0.00%) 0.00 (0.00%) NaNx
average_vsync_transitions_missed 1.00 (0.00%) 1.00 (0.00%) 1.00x
90th_percentile_vsync_transitions_missed 1.00 (0.00%) 1.00 (0.00%) 1.00x
99th_percentile_vsync_transitions_missed 1.00 (0.00%) 1.00 (0.00%) 1.00x
30hz_frame_percentage 0.00 (0.00%) 0.00 (0.00%) NaNx
60hz_frame_percentage 100.00 (0.00%) 100.00 (0.00%) 1.00x
80hz_frame_percentage 0.00 (0.00%) 0.00 (0.00%) NaNx
90hz_frame_percentage 0.00 (0.00%) 0.00 (0.00%) NaNx
120hz_frame_percentage 0.00 (0.00%) 0.00 (0.00%) NaNx
illegal_refresh_rate_frame_count 0.00 (0.00%) 0.00 (0.00%) NaNx

ColdPaleLight · 2022-07-19T09:54:25Z

I also filed a PR for adding the benchmark flutter/flutter#107918

flar · 2022-07-19T22:08:34Z

Hey, as an FYI - this utility provided in the framework repo can be used to format the A/B results in a more readable format:

https://github.com/flutter/flutter/blob/7148cba6bbe113eecc8b7fdce9b970cdc83edeb8/dev/devicelab/bin/summarize.dart

flow/raster_cache.h

flow/testing/mock_raster_cache.cc

flar

LGTM

flar · 2022-07-20T19:15:14Z

I added the autosubmit label to the benchmark to get it into the working tree ASAP to collect baseline data, but I'll leave it to you to tag this PR at your discretion. During the time that this takes to just roll into the main tree we'll likely get a few samples of baseline, but it would be nice to have a couple dozen so that the regression measuring code has some statistically relevant data to compare against.

…tter/engine#34627)

flar · 2022-07-23T18:40:17Z

Benchmark results look good here: https://flutter-flutter-perf.skia.org/e/?begin=1658253277&end=1658571306&queries=test%3Dcolor_filter_with_unstable_child_perf__e2e_summary&requestType=0

zanderso · 2022-07-25T17:54:37Z

More results from SkiaPerf: https://flutter-flutter-perf.skia.org/e/?begin=1658344386&end=1658770538&keys=Xbdb54ced983e1377840e8781177ff1b2&requestType=0&xbaroffset=30039

ColdPaleLight marked this pull request as draft July 13, 2022 03:45

Evicting unused cache entries before RasterCache actions

9ef007a

ColdPaleLight force-pushed the evict_unused_cache branch from af17d91 to 9ef007a Compare July 13, 2022 06:23

ColdPaleLight added 3 commits July 14, 2022 12:27

Tweak code and add unittests

ac4a16b

remove obsolete unittests

70755f2

update document comments

c31f717

ColdPaleLight marked this pull request as ready for review July 14, 2022 04:39

ColdPaleLight requested review from flar and JsouLiang and removed request for JsouLiang and flar July 14, 2022 04:53

ColdPaleLight marked this pull request as draft July 14, 2022 05:06

ColdPaleLight requested review from JsouLiang and flar July 14, 2022 06:21

ColdPaleLight marked this pull request as ready for review July 14, 2022 06:21

flar suggested changes Jul 15, 2022

View reviewed changes

flow/raster_cache.cc Show resolved Hide resolved

flow/raster_cache.cc Outdated Show resolved Hide resolved

flow/testing/mock_raster_cache.cc Outdated Show resolved Hide resolved

Merge branch 'flutter:main' into evict_unused_cache

5d6eaf4

Add comments about life cycle

a3f1878

ColdPaleLight mentioned this pull request Jul 19, 2022

Added benchmark for early removal of unused raster cache entris mecha… flutter/flutter#107918

Merged

8 tasks

ColdPaleLight requested a review from flar July 19, 2022 09:47

flar suggested changes Jul 19, 2022

View reviewed changes

Clean code

5d605fb

ColdPaleLight requested a review from flar July 20, 2022 08:52

flar approved these changes Jul 20, 2022

View reviewed changes

ColdPaleLight added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 23, 2022

auto-submit bot merged commit 780a1eb into flutter:main Jul 23, 2022

engine-flutter-autoroll mentioned this pull request Jul 23, 2022

Roll Flutter Engine from cb137c5d5db1 to 780a1ebee5ed (1 revision) flutter/flutter#108211

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 23, 2022

780a1eb Evicting unused cache entries before RasterCache actions (flu…

4fab635

…tter/engine#34627)

flar mentioned this pull request Dec 9, 2022

RasterCache populate should be broken out into its own pass flutter/flutter#91923

Closed

Evicting unused cache entries before RasterCache actions #34627

Evicting unused cache entries before RasterCache actions #34627

Uh oh!

Conversation

ColdPaleLight commented Jul 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

flutter-dashboard bot commented Jul 14, 2022

Uh oh!

ColdPaleLight commented Jul 14, 2022

Uh oh!

flar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JsouLiang commented Jul 16, 2022

Uh oh!

ColdPaleLight commented Jul 19, 2022

Benchmark

result

Uh oh!

ColdPaleLight commented Jul 19, 2022

Uh oh!

flar commented Jul 19, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

flar left a comment

Choose a reason for hiding this comment

Uh oh!

flar commented Jul 20, 2022

Uh oh!

flar commented Jul 23, 2022

Uh oh!

zanderso commented Jul 25, 2022

Uh oh!

Uh oh!

ColdPaleLight commented Jul 13, 2022 •

edited

Loading