Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Closed
Prev Previous commit
Next Next commit
Tweak the code and add some comments
  • Loading branch information
ColdPaleLight committed Mar 23, 2022
commit c5bfc8bb85b4d7fd7b2c42f8a72f5f4892a6c397
2 changes: 1 addition & 1 deletion flow/raster_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -416,8 +416,8 @@ 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) {
entry.unused_count++;
if (entry.unused_count < entry.unused_threshold()) {
entry.unused_count++;
if (entry.image) {
RasterCacheKeyKind kind = it->first.kind();
Copy link
Contributor

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

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) {
Expand Down
4 changes: 3 additions & 1 deletion flow/raster_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,9 @@ class RasterCache {
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; }
// 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; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe move "3" to a class constant (something like kHighPriorityEvictionThreshold) so its meaning is more obvious and the number is more visible for when we want to tweak it.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

};

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