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

Evict oldest half of cache on TRIM_MEMORY_RUNNING_CRITICAL #2889

Merged
merged 4 commits into from
Feb 13, 2018

Conversation

ygnessin
Copy link
Contributor

Description

Trim half of resource cache allocations when receiving TRIM_MEMORY_RUNNING_CRITICAL in trimMemory()

See #2810

Also, while in the code I updated some outdated comments, and replaced maxSize with getMaxSize() in LruBitmapPool

Motivation and Context

Glide is excellent at trimming memory when backgrounded, but fails to do anything when trimMemory() is received while foregrounded. This fixes that, which may save an app from slowing down or encountering an OOM while foregrounded, especially if the foreground context isn't the one using the images.

@sjudd had some good points about this strategy in #2810, which is why I've elected to be pretty conservative by only evicting half the cache when TRIM_MEMORY_RUNNING_CRITICAL is encountered. I also think this makes sense because the next stage after CRITICAL is TRIM_MEMORY_COMPLETE, which Glide already handles by evicting the entire cache. Thus, this follows the same pattern as the backgrounded behavior: Half of the cache is evicted first, then one level up the whole cache is evicted.

If anybody thinks the strategy should be more or less aggressive I'd be happy to discuss, but hopefully that explains my rationale. I also think that if the community finds this effective, we could always make it more aggressive in a future update.

Thank you for considering my contribution!

@ygnessin ygnessin changed the title Trim memory running critical Evict oldest half of cache on TRIM_MEMORY_RUNNING_CRITICAL Feb 10, 2018
@@ -220,7 +220,9 @@ public void trimMemory(int level) {
if (level >= android.content.ComponentCallbacks2.TRIM_MEMORY_BACKGROUND) {
clearMemory();
} else if (level >= android.content.ComponentCallbacks2.TRIM_MEMORY_UI_HIDDEN) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use || instead of duplicating the size logic

@@ -142,6 +142,8 @@ public synchronized void trimMemory(int level) {
clearMemory();
} else if (level >= android.content.ComponentCallbacks2.TRIM_MEMORY_UI_HIDDEN) {
evictToSize(maxSize / 2);
} else if (level == android.content.ComponentCallbacks2.TRIM_MEMORY_RUNNING_CRITICAL) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use || instead of duplicating the size logic.

@ygnessin
Copy link
Contributor Author

@sjudd thanks for the feedback, I addressed your comments

@sjudd sjudd merged commit d939314 into bumptech:master Feb 13, 2018
@sjudd
Copy link
Collaborator

sjudd commented Feb 13, 2018

Thanks!

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