Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[WIP]RasterCache adds a new mechanism for particularly complex pictures or display lists #31925

Closed
ColdPaleLight wants to merge 11 commits into from
Closed

[WIP]RasterCache adds a new mechanism for particularly complex pictures or display lists #31925

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b8f53ff
RasterCache adds a new mechanism for particularly complex pictures or…
ColdPaleLight Mar 9, 2022
e2194eb
fix typo
ColdPaleLight Mar 9, 2022
c5bfc8b
Tweak the code and add some comments
ColdPaleLight Mar 10, 2022
0a4e7c0
re-using the 'is_complex' flag
ColdPaleLight Mar 23, 2022
35b8982
tweak comment
ColdPaleLight Mar 23, 2022
c3401b6
Revert "tweak comment"
ColdPaleLight Mar 24, 2022
5842cc8
Revert "re-using the 'is_complex' flag"
ColdPaleLight Mar 24, 2022
18dc81b
Keep 'is_complex' and 'is_high_priority' orthogonal
ColdPaleLight Mar 24, 2022
c99c52f
Tweak some code in raster cache
ColdPaleLight Mar 25, 2022
36a7f86
Merge branch 'flutter:main' into particularly_complex_cache_entry
ColdPaleLight Mar 31, 2022
7537885
Merge branch 'flutter:main' into particularly_complex_cache_entry
ColdPaleLight Apr 1, 2022
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
RasterCache adds a new mechanism for particularly complex pictures or… 
… display lists
  • Loading branch information
@ColdPaleLight
ColdPaleLight committed Mar 23, 2022
commit b8f53ff475ea3337eb72781bedf71747e90aff53
48 changes: 37 additions & 11 deletions flow/raster_cache.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
Expand All @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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_);
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
Copy link
Contributor

@flar flar Mar 24, 2022

Choose a reason for hiding this comment

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

Should we have entry.is_high_priority be a one-way latch, or a most-recently-specified flag? This might depend on what the recent request for empirical evidence turns up.

if (!is_high_priority && entry.access_count < access_threshold_) {
// Frame threshold has not yet been reached.
return false;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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);
Expand All @@ -405,9 +415,25 @@ void RasterCache::SweepOneCacheAfterFrame(RasterCacheKey::Map<Entry>& 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();
Copy link
Contributor

@flar flar Mar 24, 2022

Choose a reason for hiding this comment

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

Maybe have a function metrics_for(kind) or metrics_for(<type of it.first>) to simplify all 3 cases where we have to choose a metrics to modify?

Copy link
Member Author

@ColdPaleLight ColdPaleLight Mar 25, 2022

Choose a reason for hiding this comment

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

done, added a new method GetMetricsForKind.

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) {
Expand Down
28 changes: 24 additions & 4 deletions flow/raster_cache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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<RasterCacheResult> image;
size_t unused_threshold() const { return is_high_priority ? 3 : 1; }
};

void Touch(const RasterCacheKey& cache_key);
Expand Down
121 changes: 121 additions & 0 deletions flow/raster_cache_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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.
Expand Down