Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix RasterCache LRU logic + opportunistic simplifications. #16434

Merged
merged 3 commits into from
Feb 6, 2020
Merged

Fix RasterCache LRU logic + opportunistic simplifications. #16434

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9eb589a
Fix RasterCache LRU logic + opportunistic simplifications.
Feb 6, 2020
f0c48ab
Redefine access threshold semantics to reflect the LRU cache semantic…
Feb 6, 2020
24a271e
Restore original semantics: value of access_threshold and that 0 mean…
Feb 6, 2020
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
61 changes: 28 additions & 33 deletions flow/raster_cache.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,6 @@

namespace flutter {

RasterCacheResult::RasterCacheResult() {}

RasterCacheResult::RasterCacheResult(const RasterCacheResult& other) = default;

RasterCacheResult::~RasterCacheResult() = default;

RasterCacheResult::RasterCacheResult(sk_sp<SkImage> image,
const SkRect& logical_rect)
: image_(std::move(image)), logical_rect_(logical_rect) {}
Expand All @@ -41,10 +35,7 @@ RasterCache::RasterCache(size_t access_threshold,
size_t picture_cache_limit_per_frame)
: access_threshold_(access_threshold),
picture_cache_limit_per_frame_(picture_cache_limit_per_frame),
checkerboard_images_(false),
weak_factory_(this) {}

RasterCache::~RasterCache() = default;
checkerboard_images_(false) {}

static bool CanRasterizePicture(SkPicture* picture) {
if (picture == nullptr) {
Expand Down Expand Up @@ -138,24 +129,12 @@ RasterCacheResult RasterizePicture(SkPicture* picture,
[=](SkCanvas* canvas) { canvas->drawPicture(picture); });
}

static inline size_t ClampSize(size_t value, size_t min, size_t max) {
if (value > max) {
return max;
}

if (value < min) {
return min;
}

return value;
}

void RasterCache::Prepare(PrerollContext* context,
Layer* layer,
const SkMatrix& ctm) {
LayerRasterCacheKey cache_key(layer->unique_id(), ctm);
Entry& entry = layer_cache_[cache_key];
entry.access_count = ClampSize(entry.access_count + 1, 0, access_threshold_);
entry.access_count++;
entry.used_this_frame = true;
if (!entry.image.is_valid()) {
entry.image = Rasterize(
Expand Down Expand Up @@ -191,6 +170,10 @@ bool RasterCache::Prepare(GrContext* context,
SkColorSpace* dst_color_space,
bool is_complex,
bool will_change) {
// Disabling caching when access_threshold is zero is historic behavior.
if (access_threshold_ == 0) {
return false;
}
if (picture_cached_this_frame_ >= picture_cache_limit_per_frame_) {
return false;
}
Expand All @@ -210,11 +193,9 @@ bool RasterCache::Prepare(GrContext* context,

PictureRasterCacheKey cache_key(picture->uniqueID(), transformation_matrix);

// Creates an entry, if not present prior.
Entry& entry = picture_cache_[cache_key];
entry.access_count = ClampSize(entry.access_count + 1, 0, access_threshold_);
entry.used_this_frame = true;

if (entry.access_count < access_threshold_ || access_threshold_ == 0) {
if (entry.access_count < access_threshold_) {
ignatz marked this conversation as resolved.
Show resolved Hide resolved
// Frame threshold has not yet been reached.
return false;
}
Expand All @@ -231,20 +212,34 @@ RasterCacheResult RasterCache::Get(const SkPicture& picture,
const SkMatrix& ctm) const {
PictureRasterCacheKey cache_key(picture.uniqueID(), ctm);
auto it = picture_cache_.find(cache_key);
return it == picture_cache_.end() ? RasterCacheResult() : it->second.image;
if (it == picture_cache_.end()) {
return RasterCacheResult();
}

Entry& entry = it->second;
entry.access_count++;
entry.used_this_frame = true;

return entry.image;
}

RasterCacheResult RasterCache::Get(Layer* layer, const SkMatrix& ctm) const {
LayerRasterCacheKey cache_key(layer->unique_id(), ctm);
auto it = layer_cache_.find(cache_key);
return it == layer_cache_.end() ? RasterCacheResult() : it->second.image;
if (it == layer_cache_.end()) {
return RasterCacheResult();
}

Entry& entry = it->second;
entry.access_count++;
entry.used_this_frame = true;

return entry.image;
}

void RasterCache::SweepAfterFrame() {
using PictureCache = PictureRasterCacheKey::Map<Entry>;
using LayerCache = LayerRasterCacheKey::Map<Entry>;
SweepOneCacheAfterFrame<PictureCache, PictureCache::iterator>(picture_cache_);
SweepOneCacheAfterFrame<LayerCache, LayerCache::iterator>(layer_cache_);
SweepOneCacheAfterFrame(picture_cache_);
SweepOneCacheAfterFrame(layer_cache_);
picture_cached_this_frame_ = 0;
TraceStatsToTimeline();
}
Expand Down
17 changes: 6 additions & 11 deletions flow/raster_cache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,9 @@ namespace flutter {

class RasterCacheResult {
public:
RasterCacheResult();
RasterCacheResult() = default;

RasterCacheResult(const RasterCacheResult& other);

~RasterCacheResult();
RasterCacheResult(const RasterCacheResult& other) = default;

RasterCacheResult(sk_sp<SkImage> image, const SkRect& logical_rect);

Expand Down Expand Up @@ -56,8 +54,6 @@ class RasterCache {
size_t access_threshold = 3,
size_t picture_cache_limit_per_frame = kDefaultPictureCacheLimitPerFrame);

~RasterCache();

static SkIRect GetDeviceBounds(const SkRect& rect, const SkMatrix& ctm) {
SkRect device_rect;
ctm.mapRect(&device_rect, rect);
Expand Down Expand Up @@ -109,9 +105,9 @@ class RasterCache {
RasterCacheResult image;
};

template <class Cache, class Iterator>
template <class Cache>
static void SweepOneCacheAfterFrame(Cache& cache) {
std::vector<Iterator> dead;
std::vector<typename Cache::iterator> dead;

for (auto it = cache.begin(); it != cache.end(); ++it) {
Entry& entry = it->second;
Expand All @@ -129,10 +125,9 @@ class RasterCache {
const size_t access_threshold_;
const size_t picture_cache_limit_per_frame_;
size_t picture_cached_this_frame_ = 0;
PictureRasterCacheKey::Map<Entry> picture_cache_;
LayerRasterCacheKey::Map<Entry> layer_cache_;
mutable PictureRasterCacheKey::Map<Entry> picture_cache_;
mutable LayerRasterCacheKey::Map<Entry> layer_cache_;
cbracken marked this conversation as resolved.
Show resolved Hide resolved
bool checkerboard_images_;
fml::WeakPtrFactory<RasterCache> weak_factory_;

void TraceStatsToTimeline() const;

Expand Down
79 changes: 50 additions & 29 deletions flow/raster_cache_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ TEST(RasterCache, SimpleInitialization) {
}

TEST(RasterCache, ThresholdIsRespected) {
size_t threshold = 3;
size_t threshold = 2;
flutter::RasterCache cache(threshold);

SkMatrix matrix = SkMatrix::I();
Expand All @@ -40,18 +40,28 @@ TEST(RasterCache, ThresholdIsRespected) {
sk_sp<SkImage> image;

sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
ASSERT_FALSE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true,
false)); // 1
cache.SweepAfterFrame();
ASSERT_FALSE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true,
false)); // 2
ASSERT_FALSE(
cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false));
// 1st access.
ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());

cache.SweepAfterFrame();
ASSERT_TRUE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true,
false)); // 3

ASSERT_FALSE(
cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false));

// 2st access.
ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());

cache.SweepAfterFrame();

// Now Prepare should cache it.
ASSERT_TRUE(
cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false));
ASSERT_TRUE(cache.Get(*picture, matrix).is_valid());
}

TEST(RasterCache, ThresholdIsRespectedWhenZero) {
TEST(RasterCache, AccessThresholdOfZeroDisablesCaching) {
size_t threshold = 0;
flutter::RasterCache cache(threshold);

Expand All @@ -62,19 +72,31 @@ TEST(RasterCache, ThresholdIsRespectedWhenZero) {
sk_sp<SkImage> image;

sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
ASSERT_FALSE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true,
false)); // 1
cache.SweepAfterFrame();
ASSERT_FALSE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true,
false)); // 2
cache.SweepAfterFrame();
ASSERT_FALSE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true,
false)); // 3
cache.SweepAfterFrame();
ASSERT_FALSE(
cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false));

ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());
}

TEST(RasterCache, PictureCacheLimitPerFrameIsRespectedWhenZero) {
size_t picture_cache_limit_per_frame = 0;
flutter::RasterCache cache(3, picture_cache_limit_per_frame);

SkMatrix matrix = SkMatrix::I();

auto picture = GetSamplePicture();

sk_sp<SkImage> image;

sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
ASSERT_FALSE(
cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true, false));

ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());
}

TEST(RasterCache, SweepsRemoveUnusedFrames) {
size_t threshold = 3;
size_t threshold = 1;
flutter::RasterCache cache(threshold);

SkMatrix matrix = SkMatrix::I();
Expand All @@ -86,19 +108,18 @@ TEST(RasterCache, SweepsRemoveUnusedFrames) {
sk_sp<SkColorSpace> srgb = SkColorSpace::MakeSRGB();
ASSERT_FALSE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true,
false)); // 1
ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());

cache.SweepAfterFrame();
ASSERT_FALSE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true,
false)); // 2
cache.SweepAfterFrame();
ASSERT_TRUE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true,
false)); // 3
cache.SweepAfterFrame();

ASSERT_TRUE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true,
false)); // 4
false)); // 2
ASSERT_TRUE(cache.Get(*picture, matrix).is_valid());

cache.SweepAfterFrame();
cache.SweepAfterFrame(); // Extra frame without a preroll image access.
ASSERT_FALSE(cache.Prepare(NULL, picture.get(), matrix, srgb.get(), true,
false)); // 5
cache.SweepAfterFrame(); // Extra frame without a Get image access.

ASSERT_FALSE(cache.Get(*picture, matrix).is_valid());
}

} // namespace testing
Expand Down