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.

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

Add missing raster cache LRU accounting to Get() #16431

Closed
wants to merge 1 commit into from

Conversation

cbracken
Copy link
Member

@cbracken cbracken commented Feb 5, 2020

RasterCache::Get() methods were not updating the RasterCache::Entry
access_count and used_this_frame fields, as is done in
RasterCache::Prepare(). This can result in onscreen images being evicted
from the cache as new entries are created (e.g. as new elements scroll
onscreen).

RasterCache::Get() methods were not updating the RasterCache::Entry
access_count and used_this_frame fields, as is done in
RasterCache::Prepare(). This can result in onscreen images being evicted
from the cache as new entries are created (e.g. as new elements scroll
onscreen).
@cbracken
Copy link
Member Author

cbracken commented Feb 5, 2020

See flutter/flutter#50226

Copy link
Member

@chinmaygarde chinmaygarde left a comment

Choose a reason for hiding this comment

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

I expect this is high priority for the linked b/ issue. But can we add a flow_unittests test case for this please? If urgent, feel free to land and I'll add a test.

if (it == picture_cache_.end()) {
return RasterCacheResult();
}
Entry& entry = const_cast<Entry&>(it->second);
Copy link
Member

Choose a reason for hiding this comment

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

Can we dry this up just a little bit by creating a method that takes a key and returns a cache result whose entry has its access counted bumped already?

@cbracken
Copy link
Member Author

cbracken commented Feb 6, 2020

Fixes flutter/flutter#50226

@cbracken
Copy link
Member Author

cbracken commented Feb 6, 2020

Closed in favour of #16434.

@cbracken cbracken closed this Feb 6, 2020
@cbracken cbracken deleted the raster-cache-accounting branch February 6, 2020 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants