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

Flutter should provide more control over image caching #13493

Closed
cbracken opened this issue Dec 11, 2017 · 17 comments
Closed

Flutter should provide more control over image caching #13493

cbracken opened this issue Dec 11, 2017 · 17 comments
Labels
a: images Loading, displaying, rendering images c: performance Relates to speed or footprint issues (see "perf:" labels) customer: crowd Affects or could affect many people, though not necessarily a specific customer. engine flutter/engine repository. See also e: labels. framework flutter/packages/flutter repository. See also f: labels.

Comments

@cbracken
Copy link
Member

cbracken commented Dec 11, 2017

Background: #13242 covers a specific memory consumption issue (which we'll continue looking into). I'm opening this bug for a more general discussion of the sorts of finer-grained control over image caching and handling that developers would like to see from Flutter.

Today, the only real control the framework provides is the ability to set ImageCache.maximumSize to indicate the maximum number of images to store in Flutter's image cache.

In terms of the really basic things, a few thoughts:

  • finer-grained control over cache eviction
  • finer-grained control over cache strategy (e.g. local storage, store scaled/compressed)
  • de-duplication of identical source images

It would be useful to understand what features developers would like to see from the framework and what priorities people have around image caching.

@cbracken cbracken added c: performance Relates to speed or footprint issues (see "perf:" labels) engine flutter/engine repository. See also e: labels. labels Dec 11, 2017
@Hixie
Copy link
Contributor

Hixie commented Dec 11, 2017

You can override the ImageCache object with whatever object you want.

@xster
Copy link
Member

xster commented Dec 11, 2017

Given that it's likely that every app developer will have to reproduce this work, I think it'd be good to have a deeply opinionated first party plugin or let this functionality be part of the framework.

@Hixie
Copy link
Contributor

Hixie commented Dec 11, 2017

Sure. What should the opinion be? :-)

@xster
Copy link
Member

xster commented Dec 12, 2017

I meant opinionated in terms of the granularity of the API. For the actual default values of those APIs, I think just grabbing them from similar tools like https://github.com/bumptech/glide is a helpful step up since we don't necessarily have additional domain knowledge.

@Hixie
Copy link
Contributor

Hixie commented Dec 14, 2017

Ah, that's the opposite of opinionated. :-)

We can certainly offer more knobs (as per @cbracken's original comment here).

@Hixie Hixie added this to the 5: Make Hixie proud milestone Dec 14, 2017
@rrousselGit
Copy link
Contributor

You can override the ImageCache object with whatever object you want.

@Hixie Can you add a bit more details on this ?
I'm stuck by #13242
imageCache is 'final' and is used by all ImageProvider, which are then used in Image class.

You can create your own ImageCache, true. But you'd want it to be used by Image class.

@Hixie
Copy link
Contributor

Hixie commented Dec 15, 2017

@rrousselGit See the text in #13616, let me know if that's not enough. Thanks!

@zoechi
Copy link
Contributor

zoechi commented Jan 2, 2019

@Hixie is the engine label correct here or should it be replaced by framework?

@Hixie
Copy link
Contributor

Hixie commented Jan 4, 2019

It'll probably involve both.

@zoechi zoechi added the framework flutter/packages/flutter repository. See also f: labels. label Jan 4, 2019
@iapicca iapicca added the customer: crowd Affects or could affect many people, though not necessarily a specific customer. label Nov 11, 2019
@Hixie Hixie modified the milestones: Stretch Goals, New Stretch Goals Jan 7, 2020
@liyuqian
Copy link
Contributor

CC @dnfield who recently has done a lot of great work to improve image cache.

@dnfield
Copy link
Contributor

dnfield commented Feb 25, 2020

I believe the cache api in general has evolved a lot since this issue was created. There are now methods for setting max size bytes, and the cache now knows about all images with listeners rather than just those that fit in the cache regardless of having listeners, and the cache adds tracing information to the timeline to tell you how long it took to decode an image and how many images the cache held at given points in time.

I've opened a separate bug for #47380 to exclude images from the cache, as well as #47378 to come up with a better strategy for caching network based images (and particularly their data). There's also #50022 for coming up with a way to use image dimensions (at least estimated) before deciding whether to actually decode, and #50024 for dealing with memory constraints around decoding.

We can probably do more work to continue to make the image cache better. I wonder if we can close this issue at this point, since it's either covered by more specific issues or resolved. WDYT @cbracken ?

@xster
Copy link
Member

xster commented Feb 25, 2020

Perhaps edit the OP and let this be a easily-searcheable meta bug that links to your other specific features?

@dnfield
Copy link
Contributor

dnfield commented Feb 25, 2020

Meta bugs like that tend to get out of date and go unmaintained - I've seen a bunch where it no longer is clear what's been implemented, what hasn't, and what's obsolete :\ They end up taking a while to ramp up on if you're looking for work to do, and even with this closed those other features are all still linked to it.

I should have added - the store scaled/compressed is possible today, there are additional bugs out about enhancing that, specifically #48885, and deduping source images happens at the ImageProvider.obtainKey level, which most image providers do in a fairly sane way (except memory image, which expects you to have the same instance of typed data, although that probably makes sense performance wise most of the time).

Most of what the bug was originally tracking has been fixed.

@dnfield
Copy link
Contributor

dnfield commented Feb 25, 2020

And in general a: images Loading, displaying, rendering images should be an up to date list of what we're tracking on this.

@liyuqian
Copy link
Contributor

@dnfield : do you think we shall now close this issue in favor of tracking other more specialized issue directly, or should we keep this open, and maybe reference other smaller issues from here?

@dnfield dnfield added the a: images Loading, displaying, rendering images label Mar 16, 2020
@dnfield
Copy link
Contributor

dnfield commented Mar 16, 2020

I think in general using the a: images label is the best way to find out what issues are currently open around images. I'm going to close this one in favor of just using that. Happy to discuss further if someone has a good idea around repurposing this bug, but in general I think that meta-issues tracking lots of other issues for any non-trivial project (which imaging caching is not) tend to get out of date and out of sync.

@dnfield dnfield closed this as completed Mar 16, 2020
@lock
Copy link

lock bot commented Apr 2, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@lock lock bot locked and limited conversation to collaborators Apr 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: images Loading, displaying, rendering images c: performance Relates to speed or footprint issues (see "perf:" labels) customer: crowd Affects or could affect many people, though not necessarily a specific customer. engine flutter/engine repository. See also e: labels. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

No branches or pull requests

8 participants