The Wayback Machine - https://web.archive.org/web/20210820051213/https://github.com/bumptech/glide/issues/2810
Skip to content

/ glide

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

Jump to bottom

Glide's trimMemory() implementation doesn't trim when foregrounded #2810

Open
ygnessin opened this issue Jan 13, 2018 · 5 comments
Open

Glide's trimMemory() implementation doesn't trim when foregrounded #2810

ygnessin opened this issue Jan 13, 2018 · 5 comments
Labels
enhancement good first issue help wanted

Comments

@ygnessin
Copy link
Contributor

@ygnessin ygnessin commented Jan 13, 2018

I am profiling memory on my app usage and was looking at the built-in tremMemory(int level) methods in LruResourceCache and LruBitmapPool to see what their behavior is when I invoke them from my application's onTrimMemory method. The trim memory levels are documented here.

Here is Glide's implementation of this, for both of the aforementioned classes (and probably others):

    public void trimMemory(int level) {
        if (level >= android.content.ComponentCallbacks2.TRIM_MEMORY_MODERATE) {
            // Nearing middle of list of cached background apps
            // Evict our entire bitmap cache
            clearMemory();
        } else if (level >= android.content.ComponentCallbacks2.TRIM_MEMORY_BACKGROUND) {
            // Entering list of cached background apps
            // Evict oldest half of our bitmap cache
            trimToSize(getMaxSize() / 2);
        }
    }

The problem with this is that Glide is treating the level as a sequential number. It assumes that higher is worse. But that is not actually the case, if you look at the constant value of the levels in the linked documentation.

In particular, notice that unless level is at least 40, Glide won't trim any memory. And all levels 40 and above are only triggered if the app is backgrounded. But if the device is running critically low on memory while the app is foregrounded, Glide will never trim memory. Take, for instance, the most severe level, TRIM_MEMORY_RUNNING_CRITICAL which gets called when the app is foregrounded and the device has no remaining memory. This level's constant value is 15, which won't trigger a memory trim. And unless the app is backgrounded, none of the levels before this will trigger a trim either, because all of them are under 40.

I'd like to propose modifying this default trimMemory() behavior to take foreground events into account. If the community agrees this is a good idea, I'd also be happy to open a pull request myself when I have the time, but wanted to propose the idea first. Thanks for taking the time to read this.

Glide Version: 3.8.0 (but checked the source and this is still in issue on 4.5.0)
@sjudd
Copy link
Member

@sjudd sjudd commented Jan 16, 2018

At the moment this is intended. If there's a good reason why we should expect those error messages in the foreground and why clearing the memory cache or bitmap pool will help I'd love to hear it.

At the moment I'd expect the the low memory callbacks to occur either due to other activity on the device outside of the applications control or excessive allocations inside the application. Neither of those are fixed by temporarily clearing the caches. In addition, normal usage of most apps will pretty quickly fill up the memory cache and bitmap pool. Clearing those caches isn't much of a solution to anything.

In the background trimming the cache and pool makes more sense because it reduces the size of the backstack and the caches won't be re-filled unless the app is opened again (for the most part).
@ygnessin
Copy link
Contributor Author

@ygnessin ygnessin commented Jan 16, 2018

@sjudd you make some good points. My original thought was that clearing the cache when memory is critical may prevent the app from being killed or encountering an OOM. I can understand the argument that clearing the cache won't make much of a difference, as it'll simply fill up quickly again. But what if the trim memory message is received while the user is on an activity with few or no images, and the bitmap pool and memory cache contain images from paused or stopped activities? That's the sort of scenario I was envisioning when I proposed this.

Thanks again for taking the time to consider this idea
@sjudd
Copy link
Member

@sjudd sjudd commented Jan 19, 2018

I don't have super strong feelings one way or another. You can implement this in an application by calling Glide.clearMemory, but you can't clear only a percentage of the caches.

If you're interested in sending a pull request, I'd be ok with handling the foreground cases.
@ygnessin ygnessin mentioned this issue Feb 10, 2018
Merged
@dalinaum
Copy link
Contributor

@dalinaum dalinaum commented Nov 27, 2019

@sjudd I think this issue should be closed. Since #2889 was already merged.
@picopa88
Copy link

@picopa88 picopa88 commented Jun 21, 2021

I also mistakenly thought that clearing the cache could be done but it is true that it is not possible to clear it even one percent, for some sites when having some issues related to crashes or picos school crashes. unresponsive caches can be cleared, but for some sites with large storage it may not be possible.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement good first issue help wanted
4 participants
@dalinaum @ygnessin @sjudd @picopa88