From b8f53ff475ea3337eb72781bedf71747e90aff53 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Wed, 9 Mar 2022 22:27:52 +0800 Subject: [PATCH 1/9] RasterCache adds a new mechanism for particularly complex pictures or display lists --- flow/raster_cache.cc | 48 ++++++++++--- flow/raster_cache.h | 28 ++++++-- flow/raster_cache_unittests.cc | 121 +++++++++++++++++++++++++++++++++ 3 files changed, 182 insertions(+), 15 deletions(-) diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index 647567b74dc72..625f79b81d252 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -62,7 +62,8 @@ static bool CanRasterizeRect(const SkRect& cull_rect) { static bool IsPictureWorthRasterizing(SkPicture* picture, bool will_change, - bool is_complex) { + bool is_complex, + bool is_high_priority) { if (will_change) { // If the picture is going to change in the future, there is no point in // doing to extra work to rasterize. @@ -75,7 +76,7 @@ static bool IsPictureWorthRasterizing(SkPicture* picture, return false; } - if (is_complex) { + if (is_complex || is_high_priority) { // The caller seems to have extra information about the picture and thinks // the picture is always worth rasterizing. return true; @@ -90,6 +91,7 @@ static bool IsDisplayListWorthRasterizing( DisplayList* display_list, bool will_change, bool is_complex, + bool is_high_priority, DisplayListComplexityCalculator* complexity_calculator) { if (will_change) { // If the display list is going to change in the future, there is no point @@ -103,7 +105,7 @@ static bool IsDisplayListWorthRasterizing( return false; } - if (is_complex) { + if (is_complex || is_high_priority) { // The caller seems to have extra information about the display list and // thinks the display list is always worth rasterizing. return true; @@ -180,6 +182,7 @@ void RasterCache::Prepare(PrerollContext* context, Entry& entry = cache_[cache_key]; entry.access_count++; entry.used_this_frame = true; + entry.unused_count = 0; if (!entry.image) { entry.image = RasterizeLayer(context, layer, ctm, checkerboard_images_); } @@ -222,12 +225,14 @@ bool RasterCache::Prepare(PrerollContext* context, bool is_complex, bool will_change, const SkMatrix& untranslated_matrix, - const SkPoint& offset) { + const SkPoint& offset, + bool is_high_priority) { if (!GenerateNewCacheInThisFrame()) { return false; } - if (!IsPictureWorthRasterizing(picture, will_change, is_complex)) { + if (!IsPictureWorthRasterizing(picture, will_change, is_complex, + is_high_priority)) { // We only deal with pictures that are worthy of rasterization. return false; } @@ -245,7 +250,8 @@ bool RasterCache::Prepare(PrerollContext* context, // Creates an entry, if not present prior. Entry& entry = cache_[cache_key]; - if (entry.access_count < access_threshold_) { + entry.is_high_priority = is_high_priority; + if (!is_high_priority && entry.access_count < access_threshold_) { // Frame threshold has not yet been reached. return false; } @@ -270,7 +276,8 @@ bool RasterCache::Prepare(PrerollContext* context, bool is_complex, bool will_change, const SkMatrix& untranslated_matrix, - const SkPoint& offset) { + const SkPoint& offset, + bool is_high_priority) { if (!GenerateNewCacheInThisFrame()) { return false; } @@ -281,7 +288,7 @@ bool RasterCache::Prepare(PrerollContext* context, : DisplayListComplexityCalculator::GetForSoftware(); if (!IsDisplayListWorthRasterizing(display_list, will_change, is_complex, - complexity_calculator)) { + is_high_priority, complexity_calculator)) { // We only deal with display lists that are worthy of rasterization. return false; } @@ -300,7 +307,8 @@ bool RasterCache::Prepare(PrerollContext* context, // Creates an entry, if not present prior. Entry& entry = cache_[cache_key]; - if (entry.access_count < access_threshold_) { + entry.is_high_priority = is_high_priority; + if (!is_high_priority && entry.access_count < access_threshold_) { // Frame threshold has not yet been reached. return false; } @@ -345,6 +353,7 @@ void RasterCache::Touch(const RasterCacheKey& cache_key) { if (it != cache_.end()) { it->second.used_this_frame = true; it->second.access_count++; + it->second.unused_count = 0; } } @@ -384,6 +393,7 @@ bool RasterCache::Draw(const RasterCacheKey& cache_key, Entry& entry = it->second; entry.access_count++; entry.used_this_frame = true; + entry.unused_count = 0; if (entry.image) { entry.image->draw(canvas, paint); @@ -405,9 +415,25 @@ void RasterCache::SweepOneCacheAfterFrame(RasterCacheKey::Map& cache, for (auto it = cache.begin(); it != cache.end(); ++it) { Entry& entry = it->second; - if (!entry.used_this_frame) { - dead.push_back(it); + entry.unused_count++; + if (entry.unused_count < entry.unused_threshold()) { + if (entry.image) { + RasterCacheKeyKind kind = it->first.kind(); + switch (kind) { + case RasterCacheKeyKind::kPictureMetrics: + picture_metrics.unused_count++; + picture_metrics.unused_bytes += entry.image->image_bytes(); + break; + case RasterCacheKeyKind::kLayerMetrics: + layer_metrics.unused_count++; + layer_metrics.unused_bytes += entry.image->image_bytes(); + break; + } + } + } else { + dead.push_back(it); + } } else if (entry.image) { RasterCacheKeyKind kind = it->first.kind(); switch (kind) { diff --git a/flow/raster_cache.h b/flow/raster_cache.h index bf5b39e92a751..8a23cff8e7e87 100644 --- a/flow/raster_cache.h +++ b/flow/raster_cache.h @@ -67,19 +67,32 @@ struct RasterCacheMetrics { */ size_t in_use_bytes = 0; + /** + * The number of cache entries with images unused but keeped in this frame. + */ + size_t unused_count = 0; + + /** + * The size of all of the images unused but keeped in this frame. + */ + size_t unused_bytes = 0; /** * The total cache entries that had images during this frame whether * they were used in the frame or held memory during the frame and then * were evicted after it ended. */ - size_t total_count() const { return in_use_count + eviction_count; } + size_t total_count() const { + return in_use_count + unused_count + eviction_count; + } /** * The size of all of the cached images during this frame whether * they were used in the frame or held memory during the frame and then * were evicted after it ended. */ - size_t total_bytes() const { return in_use_bytes + eviction_bytes; } + size_t total_bytes() const { + return in_use_bytes + unused_bytes + eviction_bytes; + } }; class RasterCache { @@ -186,13 +199,15 @@ class RasterCache { bool is_complex, bool will_change, const SkMatrix& untranslated_matrix, - const SkPoint& offset = SkPoint()); + const SkPoint& offset = SkPoint(), + bool is_high_priority = false); bool Prepare(PrerollContext* context, DisplayList* display_list, bool is_complex, bool will_change, const SkMatrix& untranslated_matrix, - const SkPoint& offset = SkPoint()); + const SkPoint& offset = SkPoint(), + bool is_high_priority = false); // If there is cache entry for this picture, display list or layer, mark it as // used for this frame in order to not get evicted. This is needed during @@ -286,9 +301,14 @@ class RasterCache { private: struct Entry { + // If the entry is high priority, it will always cache on first usage and + // survive 3 frames without usage. + bool is_high_priority = false; bool used_this_frame = false; size_t access_count = 0; + size_t unused_count = 0; std::unique_ptr image; + size_t unused_threshold() const { return is_high_priority ? 3 : 1; } }; void Touch(const RasterCacheKey& cache_key); diff --git a/flow/raster_cache_unittests.cc b/flow/raster_cache_unittests.cc index b62cfe5d08cd3..f7b5fe99bbf9a 100644 --- a/flow/raster_cache_unittests.cc +++ b/flow/raster_cache_unittests.cc @@ -58,6 +58,26 @@ TEST(RasterCache, ThresholdIsRespectedForSkPicture) { ASSERT_TRUE(cache.Draw(*picture, dummy_canvas)); } +TEST(RasterCache, HighPriorityIsRespectedForSkPicture) { + flutter::RasterCache cache; + + SkMatrix matrix = SkMatrix::I(); + + auto picture = GetSamplePicture(); + + SkCanvas dummy_canvas; + + PrerollContextHolder preroll_context_holder = GetSamplePrerollContextHolder(); + + cache.PrepareNewFrame(); + + // Prepare should cache it when 1st access. + ASSERT_TRUE(cache.Prepare(&preroll_context_holder.preroll_context, + picture.get(), true, false, matrix, SkPoint(), + /**is_high_priority=*/true)); + ASSERT_TRUE(cache.Draw(*picture, dummy_canvas)); +} + TEST(RasterCache, MetricsOmitUnpopulatedEntries) { size_t threshold = 2; flutter::RasterCache cache(threshold); @@ -141,6 +161,26 @@ TEST(RasterCache, ThresholdIsRespectedForDisplayList) { ASSERT_TRUE(cache.Draw(*display_list, dummy_canvas)); } +TEST(RasterCache, HighPriorityIsRespectedForDisplayList) { + flutter::RasterCache cache; + + SkMatrix matrix = SkMatrix::I(); + + auto display_list = GetSampleDisplayList(); + + SkCanvas dummy_canvas; + + PrerollContextHolder preroll_context_holder = GetSamplePrerollContextHolder(); + + cache.PrepareNewFrame(); + + // Prepare should cache it when 1st access. + ASSERT_TRUE(cache.Prepare(&preroll_context_holder.preroll_context, + display_list.get(), true, false, matrix, SkPoint(), + /**is_high_priority=*/true)); + ASSERT_TRUE(cache.Draw(*display_list, dummy_canvas)); +} + TEST(RasterCache, AccessThresholdOfZeroDisablesCachingForSkPicture) { size_t threshold = 0; flutter::RasterCache cache(threshold); @@ -291,6 +331,87 @@ TEST(RasterCache, SweepsRemoveUnusedDisplayLists) { ASSERT_FALSE(cache.Draw(*display_list, dummy_canvas)); } +void PrepareAndCleanupEmptyFrame(flutter::RasterCache& cache, size_t times) { + for (size_t i = 0; i < times; i++) { + cache.PrepareNewFrame(); + cache.CleanupAfterFrame(); // Extra frame without a Get image access. + } +} + +TEST(RasterCache, KeepUnusedSkPicturesIfIsHighPriority) { + size_t threshold = 1; + flutter::RasterCache cache(threshold); + + SkMatrix matrix = SkMatrix::I(); + + auto picture = GetSamplePicture(); + + SkCanvas dummy_canvas; + + PrerollContextHolder preroll_context_holder = GetSamplePrerollContextHolder(); + + cache.PrepareNewFrame(); + + ASSERT_TRUE(cache.Prepare(&preroll_context_holder.preroll_context, + picture.get(), true, false, matrix, SkPoint(), + /**is_high_priority=*/true)); + ASSERT_TRUE(cache.Draw(*picture, dummy_canvas)); + + cache.CleanupAfterFrame(); + + PrepareAndCleanupEmptyFrame(cache, 1); + cache.PrepareNewFrame(); + ASSERT_TRUE(cache.Draw(*picture, dummy_canvas)); + cache.CleanupAfterFrame(); + + PrepareAndCleanupEmptyFrame(cache, 2); + cache.PrepareNewFrame(); + ASSERT_TRUE(cache.Draw(*picture, dummy_canvas)); + cache.CleanupAfterFrame(); + + PrepareAndCleanupEmptyFrame(cache, 3); + cache.PrepareNewFrame(); + ASSERT_FALSE(cache.Draw(*picture, dummy_canvas)); + cache.CleanupAfterFrame(); +} + +TEST(RasterCache, KeepUnusedDisplayListsIfIsHighPriority) { + size_t threshold = 1; + flutter::RasterCache cache(threshold); + + SkMatrix matrix = SkMatrix::I(); + + auto display_list = GetSampleDisplayList(); + + SkCanvas dummy_canvas; + + PrerollContextHolder preroll_context_holder = GetSamplePrerollContextHolder(); + + cache.PrepareNewFrame(); + + ASSERT_TRUE(cache.Prepare(&preroll_context_holder.preroll_context, + display_list.get(), true, false, matrix, SkPoint(), + /**is_high_priority=*/true)); + ASSERT_TRUE(cache.Draw(*display_list, dummy_canvas)); + + cache.CleanupAfterFrame(); + + PrepareAndCleanupEmptyFrame(cache, 1); + cache.PrepareNewFrame(); + ASSERT_TRUE(cache.Draw(*display_list, dummy_canvas)); + cache.CleanupAfterFrame(); + + PrepareAndCleanupEmptyFrame(cache, 2); + cache.PrepareNewFrame(); + ASSERT_TRUE(cache.Draw(*display_list, dummy_canvas)); + cache.CleanupAfterFrame(); + + PrepareAndCleanupEmptyFrame(cache, 3); + cache.PrepareNewFrame(); + ASSERT_FALSE(cache.Draw(*display_list, dummy_canvas)); + cache.CleanupAfterFrame(); +} + // Construct a cache result whose device target rectangle rounds out to be one // pixel wider than the cached image. Verify that it can be drawn without // triggering any assertions. From e2194eb5df94e5fa5b873288a94aa8b64960a2db Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Wed, 9 Mar 2022 23:04:01 +0800 Subject: [PATCH 2/9] fix typo --- flow/raster_cache.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/flow/raster_cache.h b/flow/raster_cache.h index 8a23cff8e7e87..5b4c6ab98dc70 100644 --- a/flow/raster_cache.h +++ b/flow/raster_cache.h @@ -68,12 +68,12 @@ struct RasterCacheMetrics { size_t in_use_bytes = 0; /** - * The number of cache entries with images unused but keeped in this frame. + * The number of cache entries with images unused but kept in this frame. */ size_t unused_count = 0; /** - * The size of all of the images unused but keeped in this frame. + * The size of all of the images unused but kept in this frame. */ size_t unused_bytes = 0; /** From c5bfc8bb85b4d7fd7b2c42f8a72f5f4892a6c397 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Thu, 10 Mar 2022 10:44:57 +0800 Subject: [PATCH 3/9] Tweak the code and add some comments --- flow/raster_cache.cc | 2 +- flow/raster_cache.h | 4 +++- flow/raster_cache_unittests.cc | 12 ++++++++++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index 625f79b81d252..101a83180e847 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -416,8 +416,8 @@ void RasterCache::SweepOneCacheAfterFrame(RasterCacheKey::Map& cache, for (auto it = cache.begin(); it != cache.end(); ++it) { Entry& entry = it->second; if (!entry.used_this_frame) { - entry.unused_count++; if (entry.unused_count < entry.unused_threshold()) { + entry.unused_count++; if (entry.image) { RasterCacheKeyKind kind = it->first.kind(); switch (kind) { diff --git a/flow/raster_cache.h b/flow/raster_cache.h index 5b4c6ab98dc70..f1331d638ebfe 100644 --- a/flow/raster_cache.h +++ b/flow/raster_cache.h @@ -308,7 +308,9 @@ class RasterCache { size_t access_count = 0; size_t unused_count = 0; std::unique_ptr image; - size_t unused_threshold() const { return is_high_priority ? 3 : 1; } + // Return the number of frames the entry survives if it is not used. If the + // number is 0, then it will be evicted when not in use. + size_t unused_threshold() const { return is_high_priority ? 3 : 0; } }; void Touch(const RasterCacheKey& cache_key); diff --git a/flow/raster_cache_unittests.cc b/flow/raster_cache_unittests.cc index f7b5fe99bbf9a..c078026e56f35 100644 --- a/flow/raster_cache_unittests.cc +++ b/flow/raster_cache_unittests.cc @@ -371,6 +371,12 @@ TEST(RasterCache, KeepUnusedSkPicturesIfIsHighPriority) { PrepareAndCleanupEmptyFrame(cache, 3); cache.PrepareNewFrame(); + ASSERT_TRUE(cache.Draw(*picture, dummy_canvas)); + cache.CleanupAfterFrame(); + + // The entry will be evicted when it is not used 4 times in a row. + PrepareAndCleanupEmptyFrame(cache, 4); + cache.PrepareNewFrame(); ASSERT_FALSE(cache.Draw(*picture, dummy_canvas)); cache.CleanupAfterFrame(); } @@ -408,6 +414,12 @@ TEST(RasterCache, KeepUnusedDisplayListsIfIsHighPriority) { PrepareAndCleanupEmptyFrame(cache, 3); cache.PrepareNewFrame(); + ASSERT_TRUE(cache.Draw(*display_list, dummy_canvas)); + cache.CleanupAfterFrame(); + + // The entry will be evicted when it is not used 4 times in a row. + PrepareAndCleanupEmptyFrame(cache, 4); + cache.PrepareNewFrame(); ASSERT_FALSE(cache.Draw(*display_list, dummy_canvas)); cache.CleanupAfterFrame(); } From 0a4e7c02486a68af8bf660790d858fcd6e391288 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Wed, 23 Mar 2022 18:02:23 +0800 Subject: [PATCH 4/9] re-using the 'is_complex' flag --- flow/layers/display_list_layer.cc | 4 ++-- flow/layers/picture_layer.cc | 4 ++-- flow/raster_cache.cc | 28 ++++++++++++++-------------- flow/raster_cache.h | 12 ++++++------ 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/flow/layers/display_list_layer.cc b/flow/layers/display_list_layer.cc index f360602cf8376..c6cc038082b1b 100644 --- a/flow/layers/display_list_layer.cc +++ b/flow/layers/display_list_layer.cc @@ -99,8 +99,8 @@ void DisplayListLayer::Preroll(PrerollContext* context, if (auto* cache = context->raster_cache) { TRACE_EVENT0("flutter", "DisplayListLayer::RasterCache (Preroll)"); if (context->cull_rect.intersects(bounds)) { - if (cache->Prepare(context, disp_list, is_complex_, will_change_, matrix, - offset_)) { + if (cache->Prepare(context, disp_list, /*for_testing=*/false, + will_change_, matrix, offset_, is_complex_)) { context->subtree_can_inherit_opacity = true; } } else { diff --git a/flow/layers/picture_layer.cc b/flow/layers/picture_layer.cc index f93437d993bdf..ba3f7c9dc9d51 100644 --- a/flow/layers/picture_layer.cc +++ b/flow/layers/picture_layer.cc @@ -117,8 +117,8 @@ void PictureLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { if (auto* cache = context->raster_cache) { TRACE_EVENT0("flutter", "PictureLayer::RasterCache (Preroll)"); if (context->cull_rect.intersects(bounds)) { - if (cache->Prepare(context, sk_picture, is_complex_, will_change_, matrix, - offset_)) { + if (cache->Prepare(context, sk_picture, /*for_testing=*/false, + will_change_, matrix, offset_, is_complex_)) { context->subtree_can_inherit_opacity = true; } } else { diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index 101a83180e847..47f87e3455cc3 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -63,7 +63,7 @@ static bool CanRasterizeRect(const SkRect& cull_rect) { static bool IsPictureWorthRasterizing(SkPicture* picture, bool will_change, bool is_complex, - bool is_high_priority) { + bool for_testing) { if (will_change) { // If the picture is going to change in the future, there is no point in // doing to extra work to rasterize. @@ -76,7 +76,7 @@ static bool IsPictureWorthRasterizing(SkPicture* picture, return false; } - if (is_complex || is_high_priority) { + if (is_complex || for_testing) { // The caller seems to have extra information about the picture and thinks // the picture is always worth rasterizing. return true; @@ -91,7 +91,7 @@ static bool IsDisplayListWorthRasterizing( DisplayList* display_list, bool will_change, bool is_complex, - bool is_high_priority, + bool for_testing, DisplayListComplexityCalculator* complexity_calculator) { if (will_change) { // If the display list is going to change in the future, there is no point @@ -105,7 +105,7 @@ static bool IsDisplayListWorthRasterizing( return false; } - if (is_complex || is_high_priority) { + if (is_complex || for_testing) { // The caller seems to have extra information about the display list and // thinks the display list is always worth rasterizing. return true; @@ -222,17 +222,17 @@ std::unique_ptr RasterCache::RasterizeLayer( bool RasterCache::Prepare(PrerollContext* context, SkPicture* picture, - bool is_complex, + bool for_testing, bool will_change, const SkMatrix& untranslated_matrix, const SkPoint& offset, - bool is_high_priority) { + bool is_complex) { if (!GenerateNewCacheInThisFrame()) { return false; } if (!IsPictureWorthRasterizing(picture, will_change, is_complex, - is_high_priority)) { + for_testing)) { // We only deal with pictures that are worthy of rasterization. return false; } @@ -250,8 +250,8 @@ bool RasterCache::Prepare(PrerollContext* context, // Creates an entry, if not present prior. Entry& entry = cache_[cache_key]; - entry.is_high_priority = is_high_priority; - if (!is_high_priority && entry.access_count < access_threshold_) { + entry.is_complex = is_complex; + if (!is_complex && entry.access_count < access_threshold_) { // Frame threshold has not yet been reached. return false; } @@ -273,11 +273,11 @@ bool RasterCache::Prepare(PrerollContext* context, bool RasterCache::Prepare(PrerollContext* context, DisplayList* display_list, - bool is_complex, + bool for_testing, bool will_change, const SkMatrix& untranslated_matrix, const SkPoint& offset, - bool is_high_priority) { + bool is_complex) { if (!GenerateNewCacheInThisFrame()) { return false; } @@ -288,7 +288,7 @@ bool RasterCache::Prepare(PrerollContext* context, : DisplayListComplexityCalculator::GetForSoftware(); if (!IsDisplayListWorthRasterizing(display_list, will_change, is_complex, - is_high_priority, complexity_calculator)) { + for_testing, complexity_calculator)) { // We only deal with display lists that are worthy of rasterization. return false; } @@ -307,8 +307,8 @@ bool RasterCache::Prepare(PrerollContext* context, // Creates an entry, if not present prior. Entry& entry = cache_[cache_key]; - entry.is_high_priority = is_high_priority; - if (!is_high_priority && entry.access_count < access_threshold_) { + entry.is_complex = is_complex; + if (!is_complex && entry.access_count < access_threshold_) { // Frame threshold has not yet been reached. return false; } diff --git a/flow/raster_cache.h b/flow/raster_cache.h index f1331d638ebfe..a1a84dea04dcd 100644 --- a/flow/raster_cache.h +++ b/flow/raster_cache.h @@ -196,18 +196,18 @@ class RasterCache { // 4. The picture is accessed too few times bool Prepare(PrerollContext* context, SkPicture* picture, - bool is_complex, + bool for_testing, bool will_change, const SkMatrix& untranslated_matrix, const SkPoint& offset = SkPoint(), - bool is_high_priority = false); + bool is_complex = false); bool Prepare(PrerollContext* context, DisplayList* display_list, - bool is_complex, + bool for_testing, bool will_change, const SkMatrix& untranslated_matrix, const SkPoint& offset = SkPoint(), - bool is_high_priority = false); + bool is_complex = false); // If there is cache entry for this picture, display list or layer, mark it as // used for this frame in order to not get evicted. This is needed during @@ -303,14 +303,14 @@ class RasterCache { struct Entry { // If the entry is high priority, it will always cache on first usage and // survive 3 frames without usage. - bool is_high_priority = false; + bool is_complex = false; bool used_this_frame = false; size_t access_count = 0; size_t unused_count = 0; std::unique_ptr image; // Return the number of frames the entry survives if it is not used. If the // number is 0, then it will be evicted when not in use. - size_t unused_threshold() const { return is_high_priority ? 3 : 0; } + size_t unused_threshold() const { return is_complex ? 3 : 0; } }; void Touch(const RasterCacheKey& cache_key); From 35b898274f5588c65e19a3e95ab15edfc0f61563 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Wed, 23 Mar 2022 22:01:04 +0800 Subject: [PATCH 5/9] tweak comment --- flow/raster_cache_unittests.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/flow/raster_cache_unittests.cc b/flow/raster_cache_unittests.cc index c078026e56f35..cf823b4f27611 100644 --- a/flow/raster_cache_unittests.cc +++ b/flow/raster_cache_unittests.cc @@ -74,7 +74,7 @@ TEST(RasterCache, HighPriorityIsRespectedForSkPicture) { // Prepare should cache it when 1st access. ASSERT_TRUE(cache.Prepare(&preroll_context_holder.preroll_context, picture.get(), true, false, matrix, SkPoint(), - /**is_high_priority=*/true)); + /**is_complex=*/true)); ASSERT_TRUE(cache.Draw(*picture, dummy_canvas)); } @@ -177,7 +177,7 @@ TEST(RasterCache, HighPriorityIsRespectedForDisplayList) { // Prepare should cache it when 1st access. ASSERT_TRUE(cache.Prepare(&preroll_context_holder.preroll_context, display_list.get(), true, false, matrix, SkPoint(), - /**is_high_priority=*/true)); + /**is_complex=*/true)); ASSERT_TRUE(cache.Draw(*display_list, dummy_canvas)); } @@ -354,7 +354,7 @@ TEST(RasterCache, KeepUnusedSkPicturesIfIsHighPriority) { ASSERT_TRUE(cache.Prepare(&preroll_context_holder.preroll_context, picture.get(), true, false, matrix, SkPoint(), - /**is_high_priority=*/true)); + /**is_complex=*/true)); ASSERT_TRUE(cache.Draw(*picture, dummy_canvas)); cache.CleanupAfterFrame(); @@ -397,7 +397,7 @@ TEST(RasterCache, KeepUnusedDisplayListsIfIsHighPriority) { ASSERT_TRUE(cache.Prepare(&preroll_context_holder.preroll_context, display_list.get(), true, false, matrix, SkPoint(), - /**is_high_priority=*/true)); + /**is_complex=*/true)); ASSERT_TRUE(cache.Draw(*display_list, dummy_canvas)); cache.CleanupAfterFrame(); From c3401b649973dfe517541d109f4c12912c79ed6e Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Thu, 24 Mar 2022 16:57:29 +0800 Subject: [PATCH 6/9] Revert "tweak comment" This reverts commit 35b898274f5588c65e19a3e95ab15edfc0f61563. --- flow/raster_cache_unittests.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/flow/raster_cache_unittests.cc b/flow/raster_cache_unittests.cc index cf823b4f27611..c078026e56f35 100644 --- a/flow/raster_cache_unittests.cc +++ b/flow/raster_cache_unittests.cc @@ -74,7 +74,7 @@ TEST(RasterCache, HighPriorityIsRespectedForSkPicture) { // Prepare should cache it when 1st access. ASSERT_TRUE(cache.Prepare(&preroll_context_holder.preroll_context, picture.get(), true, false, matrix, SkPoint(), - /**is_complex=*/true)); + /**is_high_priority=*/true)); ASSERT_TRUE(cache.Draw(*picture, dummy_canvas)); } @@ -177,7 +177,7 @@ TEST(RasterCache, HighPriorityIsRespectedForDisplayList) { // Prepare should cache it when 1st access. ASSERT_TRUE(cache.Prepare(&preroll_context_holder.preroll_context, display_list.get(), true, false, matrix, SkPoint(), - /**is_complex=*/true)); + /**is_high_priority=*/true)); ASSERT_TRUE(cache.Draw(*display_list, dummy_canvas)); } @@ -354,7 +354,7 @@ TEST(RasterCache, KeepUnusedSkPicturesIfIsHighPriority) { ASSERT_TRUE(cache.Prepare(&preroll_context_holder.preroll_context, picture.get(), true, false, matrix, SkPoint(), - /**is_complex=*/true)); + /**is_high_priority=*/true)); ASSERT_TRUE(cache.Draw(*picture, dummy_canvas)); cache.CleanupAfterFrame(); @@ -397,7 +397,7 @@ TEST(RasterCache, KeepUnusedDisplayListsIfIsHighPriority) { ASSERT_TRUE(cache.Prepare(&preroll_context_holder.preroll_context, display_list.get(), true, false, matrix, SkPoint(), - /**is_complex=*/true)); + /**is_high_priority=*/true)); ASSERT_TRUE(cache.Draw(*display_list, dummy_canvas)); cache.CleanupAfterFrame(); From 5842cc870359272dfd558b20ef0d40612857a57c Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Thu, 24 Mar 2022 16:57:39 +0800 Subject: [PATCH 7/9] Revert "re-using the 'is_complex' flag" This reverts commit 0a4e7c02486a68af8bf660790d858fcd6e391288. --- flow/layers/display_list_layer.cc | 4 ++-- flow/layers/picture_layer.cc | 4 ++-- flow/raster_cache.cc | 28 ++++++++++++++-------------- flow/raster_cache.h | 12 ++++++------ 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/flow/layers/display_list_layer.cc b/flow/layers/display_list_layer.cc index c6cc038082b1b..f360602cf8376 100644 --- a/flow/layers/display_list_layer.cc +++ b/flow/layers/display_list_layer.cc @@ -99,8 +99,8 @@ void DisplayListLayer::Preroll(PrerollContext* context, if (auto* cache = context->raster_cache) { TRACE_EVENT0("flutter", "DisplayListLayer::RasterCache (Preroll)"); if (context->cull_rect.intersects(bounds)) { - if (cache->Prepare(context, disp_list, /*for_testing=*/false, - will_change_, matrix, offset_, is_complex_)) { + if (cache->Prepare(context, disp_list, is_complex_, will_change_, matrix, + offset_)) { context->subtree_can_inherit_opacity = true; } } else { diff --git a/flow/layers/picture_layer.cc b/flow/layers/picture_layer.cc index ba3f7c9dc9d51..f93437d993bdf 100644 --- a/flow/layers/picture_layer.cc +++ b/flow/layers/picture_layer.cc @@ -117,8 +117,8 @@ void PictureLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { if (auto* cache = context->raster_cache) { TRACE_EVENT0("flutter", "PictureLayer::RasterCache (Preroll)"); if (context->cull_rect.intersects(bounds)) { - if (cache->Prepare(context, sk_picture, /*for_testing=*/false, - will_change_, matrix, offset_, is_complex_)) { + if (cache->Prepare(context, sk_picture, is_complex_, will_change_, matrix, + offset_)) { context->subtree_can_inherit_opacity = true; } } else { diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index 47f87e3455cc3..101a83180e847 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -63,7 +63,7 @@ static bool CanRasterizeRect(const SkRect& cull_rect) { static bool IsPictureWorthRasterizing(SkPicture* picture, bool will_change, bool is_complex, - bool for_testing) { + bool is_high_priority) { if (will_change) { // If the picture is going to change in the future, there is no point in // doing to extra work to rasterize. @@ -76,7 +76,7 @@ static bool IsPictureWorthRasterizing(SkPicture* picture, return false; } - if (is_complex || for_testing) { + if (is_complex || is_high_priority) { // The caller seems to have extra information about the picture and thinks // the picture is always worth rasterizing. return true; @@ -91,7 +91,7 @@ static bool IsDisplayListWorthRasterizing( DisplayList* display_list, bool will_change, bool is_complex, - bool for_testing, + bool is_high_priority, DisplayListComplexityCalculator* complexity_calculator) { if (will_change) { // If the display list is going to change in the future, there is no point @@ -105,7 +105,7 @@ static bool IsDisplayListWorthRasterizing( return false; } - if (is_complex || for_testing) { + if (is_complex || is_high_priority) { // The caller seems to have extra information about the display list and // thinks the display list is always worth rasterizing. return true; @@ -222,17 +222,17 @@ std::unique_ptr RasterCache::RasterizeLayer( bool RasterCache::Prepare(PrerollContext* context, SkPicture* picture, - bool for_testing, + bool is_complex, bool will_change, const SkMatrix& untranslated_matrix, const SkPoint& offset, - bool is_complex) { + bool is_high_priority) { if (!GenerateNewCacheInThisFrame()) { return false; } if (!IsPictureWorthRasterizing(picture, will_change, is_complex, - for_testing)) { + is_high_priority)) { // We only deal with pictures that are worthy of rasterization. return false; } @@ -250,8 +250,8 @@ bool RasterCache::Prepare(PrerollContext* context, // Creates an entry, if not present prior. Entry& entry = cache_[cache_key]; - entry.is_complex = is_complex; - if (!is_complex && entry.access_count < access_threshold_) { + entry.is_high_priority = is_high_priority; + if (!is_high_priority && entry.access_count < access_threshold_) { // Frame threshold has not yet been reached. return false; } @@ -273,11 +273,11 @@ bool RasterCache::Prepare(PrerollContext* context, bool RasterCache::Prepare(PrerollContext* context, DisplayList* display_list, - bool for_testing, + bool is_complex, bool will_change, const SkMatrix& untranslated_matrix, const SkPoint& offset, - bool is_complex) { + bool is_high_priority) { if (!GenerateNewCacheInThisFrame()) { return false; } @@ -288,7 +288,7 @@ bool RasterCache::Prepare(PrerollContext* context, : DisplayListComplexityCalculator::GetForSoftware(); if (!IsDisplayListWorthRasterizing(display_list, will_change, is_complex, - for_testing, complexity_calculator)) { + is_high_priority, complexity_calculator)) { // We only deal with display lists that are worthy of rasterization. return false; } @@ -307,8 +307,8 @@ bool RasterCache::Prepare(PrerollContext* context, // Creates an entry, if not present prior. Entry& entry = cache_[cache_key]; - entry.is_complex = is_complex; - if (!is_complex && entry.access_count < access_threshold_) { + entry.is_high_priority = is_high_priority; + if (!is_high_priority && entry.access_count < access_threshold_) { // Frame threshold has not yet been reached. return false; } diff --git a/flow/raster_cache.h b/flow/raster_cache.h index a1a84dea04dcd..f1331d638ebfe 100644 --- a/flow/raster_cache.h +++ b/flow/raster_cache.h @@ -196,18 +196,18 @@ class RasterCache { // 4. The picture is accessed too few times bool Prepare(PrerollContext* context, SkPicture* picture, - bool for_testing, + bool is_complex, bool will_change, const SkMatrix& untranslated_matrix, const SkPoint& offset = SkPoint(), - bool is_complex = false); + bool is_high_priority = false); bool Prepare(PrerollContext* context, DisplayList* display_list, - bool for_testing, + bool is_complex, bool will_change, const SkMatrix& untranslated_matrix, const SkPoint& offset = SkPoint(), - bool is_complex = false); + bool is_high_priority = false); // If there is cache entry for this picture, display list or layer, mark it as // used for this frame in order to not get evicted. This is needed during @@ -303,14 +303,14 @@ class RasterCache { struct Entry { // If the entry is high priority, it will always cache on first usage and // survive 3 frames without usage. - bool is_complex = false; + bool is_high_priority = false; bool used_this_frame = false; size_t access_count = 0; size_t unused_count = 0; std::unique_ptr image; // Return the number of frames the entry survives if it is not used. If the // number is 0, then it will be evicted when not in use. - size_t unused_threshold() const { return is_complex ? 3 : 0; } + size_t unused_threshold() const { return is_high_priority ? 3 : 0; } }; void Touch(const RasterCacheKey& cache_key); From 18dc81bd0766da9220378fb3d60182d6f8aa6112 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Thu, 24 Mar 2022 17:21:59 +0800 Subject: [PATCH 8/9] Keep 'is_complex' and 'is_high_priority' orthogonal --- flow/raster_cache.cc | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index 101a83180e847..7b7a1b674166b 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -62,8 +62,7 @@ static bool CanRasterizeRect(const SkRect& cull_rect) { static bool IsPictureWorthRasterizing(SkPicture* picture, bool will_change, - bool is_complex, - bool is_high_priority) { + bool is_complex) { if (will_change) { // If the picture is going to change in the future, there is no point in // doing to extra work to rasterize. @@ -76,7 +75,7 @@ static bool IsPictureWorthRasterizing(SkPicture* picture, return false; } - if (is_complex || is_high_priority) { + if (is_complex) { // The caller seems to have extra information about the picture and thinks // the picture is always worth rasterizing. return true; @@ -91,7 +90,6 @@ static bool IsDisplayListWorthRasterizing( DisplayList* display_list, bool will_change, bool is_complex, - bool is_high_priority, DisplayListComplexityCalculator* complexity_calculator) { if (will_change) { // If the display list is going to change in the future, there is no point @@ -105,7 +103,7 @@ static bool IsDisplayListWorthRasterizing( return false; } - if (is_complex || is_high_priority) { + if (is_complex) { // The caller seems to have extra information about the display list and // thinks the display list is always worth rasterizing. return true; @@ -231,8 +229,7 @@ bool RasterCache::Prepare(PrerollContext* context, return false; } - if (!IsPictureWorthRasterizing(picture, will_change, is_complex, - is_high_priority)) { + if (!IsPictureWorthRasterizing(picture, will_change, is_complex)) { // We only deal with pictures that are worthy of rasterization. return false; } @@ -288,7 +285,7 @@ bool RasterCache::Prepare(PrerollContext* context, : DisplayListComplexityCalculator::GetForSoftware(); if (!IsDisplayListWorthRasterizing(display_list, will_change, is_complex, - is_high_priority, complexity_calculator)) { + complexity_calculator)) { // We only deal with display lists that are worthy of rasterization. return false; } From c99c52f9a86abb5cefb6e64a7c6732b6c5d8423e Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Fri, 25 Mar 2022 11:45:00 +0800 Subject: [PATCH 9/9] Tweak some code in raster cache --- flow/raster_cache.cc | 61 ++++++++++++++++---------------------------- flow/raster_cache.h | 13 +++++++--- 2 files changed, 31 insertions(+), 43 deletions(-) diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index 7b7a1b674166b..f3d075594b05b 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -405,63 +405,37 @@ void RasterCache::PrepareNewFrame() { display_list_cached_this_frame_ = 0; } -void RasterCache::SweepOneCacheAfterFrame(RasterCacheKey::Map& cache, - RasterCacheMetrics& picture_metrics, - RasterCacheMetrics& layer_metrics) { +void RasterCache::SweepCacheAfterFrame() { std::vector::iterator> dead; - for (auto it = cache.begin(); it != cache.end(); ++it) { + for (auto it = cache_.begin(); it != cache_.end(); ++it) { Entry& entry = it->second; if (!entry.used_this_frame) { if (entry.unused_count < entry.unused_threshold()) { entry.unused_count++; if (entry.image) { - RasterCacheKeyKind kind = it->first.kind(); - switch (kind) { - case RasterCacheKeyKind::kPictureMetrics: - picture_metrics.unused_count++; - picture_metrics.unused_bytes += entry.image->image_bytes(); - break; - case RasterCacheKeyKind::kLayerMetrics: - layer_metrics.unused_count++; - layer_metrics.unused_bytes += entry.image->image_bytes(); - break; - } + RasterCacheMetrics& metrics = GetMetricsForKind(it->first.kind()); + metrics.unused_count++; + metrics.unused_bytes += entry.image->image_bytes(); } } else { dead.push_back(it); } } else if (entry.image) { - RasterCacheKeyKind kind = it->first.kind(); - switch (kind) { - case RasterCacheKeyKind::kPictureMetrics: - picture_metrics.in_use_count++; - picture_metrics.in_use_bytes += entry.image->image_bytes(); - break; - case RasterCacheKeyKind::kLayerMetrics: - layer_metrics.in_use_count++; - layer_metrics.in_use_bytes += entry.image->image_bytes(); - break; - } + RasterCacheMetrics& metrics = GetMetricsForKind(it->first.kind()); + metrics.in_use_count++; + metrics.in_use_bytes += entry.image->image_bytes(); } entry.used_this_frame = false; } for (auto it : dead) { if (it->second.image) { - RasterCacheKeyKind kind = it->first.kind(); - switch (kind) { - case RasterCacheKeyKind::kPictureMetrics: - picture_metrics.eviction_count++; - picture_metrics.eviction_bytes += it->second.image->image_bytes(); - break; - case RasterCacheKeyKind::kLayerMetrics: - layer_metrics.eviction_count++; - layer_metrics.eviction_bytes += it->second.image->image_bytes(); - break; - } + RasterCacheMetrics& metrics = GetMetricsForKind(it->first.kind()); + metrics.eviction_count++; + metrics.eviction_bytes += it->second.image->image_bytes(); } - cache.erase(it); + cache_.erase(it); } } @@ -470,7 +444,7 @@ void RasterCache::CleanupAfterFrame() { layer_metrics_ = {}; { TRACE_EVENT0("flutter", "RasterCache::SweepCaches"); - SweepOneCacheAfterFrame(cache_, picture_metrics_, layer_metrics_); + SweepCacheAfterFrame(); } TraceStatsToTimeline(); } @@ -481,6 +455,15 @@ void RasterCache::Clear() { layer_metrics_ = {}; } +RasterCacheMetrics& RasterCache::GetMetricsForKind(RasterCacheKeyKind kind) { + switch (kind) { + case RasterCacheKeyKind::kPictureMetrics: + return picture_metrics_; + case RasterCacheKeyKind::kLayerMetrics: + return layer_metrics_; + } +} + size_t RasterCache::GetCachedEntriesCount() const { return cache_.size(); } diff --git a/flow/raster_cache.h b/flow/raster_cache.h index f1331d638ebfe..fb52d7c948ca3 100644 --- a/flow/raster_cache.h +++ b/flow/raster_cache.h @@ -102,6 +102,9 @@ class RasterCache { // on that frame. This limit allows us to throttle the cache and distribute // the work across multiple frames. static constexpr int kDefaultPictureAndDispLayListCacheLimitPerFrame = 3; + // The default number of frames the high-priority entry which is high priority + // survives if it is not used. + static constexpr int kHighPriorityEvictionThreshold = 3; explicit RasterCache(size_t access_threshold = 3, size_t picture_and_display_list_cache_limit_per_frame = @@ -310,7 +313,9 @@ class RasterCache { std::unique_ptr image; // Return the number of frames the entry survives if it is not used. If the // number is 0, then it will be evicted when not in use. - size_t unused_threshold() const { return is_high_priority ? 3 : 0; } + size_t unused_threshold() const { + return is_high_priority ? kHighPriorityEvictionThreshold : 0; + } }; void Touch(const RasterCacheKey& cache_key); @@ -319,9 +324,9 @@ class RasterCache { SkCanvas& canvas, const SkPaint* paint) const; - void SweepOneCacheAfterFrame(RasterCacheKey::Map& cache, - RasterCacheMetrics& picture_metrics, - RasterCacheMetrics& layer_metrics); + void SweepCacheAfterFrame(); + + RasterCacheMetrics& GetMetricsForKind(RasterCacheKeyKind kind); bool GenerateNewCacheInThisFrame() const { // Disabling caching when access_threshold is zero is historic behavior.