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

About bitmap recycle #687

Closed
Caij opened this issue Oct 16, 2015 · 9 comments
Closed

About bitmap recycle #687

Caij opened this issue Oct 16, 2015 · 9 comments

Comments

@Caij
Copy link

Caij commented Oct 16, 2015

when LruMemoryCache full, LruMemoryCache remove some bitmap, these bitmap put BitmapPool , these bitmap reuse for other Bitmap Request。if these bitmap is using in ImageView,Will lead to errors?

@TWiStErRob
Copy link
Collaborator

@Caij Interesting question, did you have any problems or are you just trying to understand how it works?

If a Bitmap is shown an ImageView it's an active resource, which means it's not in any cache or pool.
Look at Engine.getEngineResourceFromCache where it is removed from MemoryCache, then the result of that method is used just above (Engine.loadFromCache) to activate it.
Note that remove and evict are different, see LruCache.remove/trimToSize: only the evicted items are returned to the pool.

@sjudd Which brings me to something which I found by navigating the code:

  • Glide.clearMemory();:

    bitmapPool.clearMemory(); // all bitmaps are removed and recycled
    memoryCache.clearMemory(); // all bitmaps are removed and put to pool?! (see below)
    // end of clearMemory, but pool has stuff in it
  • LruCache.clearMemory -> LruCache.trimToSize

  • trimToSize removes all items and calls onItemEvicted

  • onItemEvicted is overridden in LruResourceCache

  • which calls ResourceRemovedListener.onResourceRemoved() implemented by Engine

  • Engine.onResourceRemoved asserts main thread as well as right inside recycle() it's safe, but is it necessary?

  • Engine.onResourceRemoved calls resourceRecycler.recycle

  • ResourceRecycler.recycle calls Resource.recycle() which returns the Bitmap into the pool if the resource has one.

So there are two things to consider:

public void clearMemory() {
    // ResourceRecycler.recycle asserts this anyway, fail faster and consistently
    // (I think currently clearMemory would work on a background thread if there are no resources)
    Util.assertMainThread();
    // clear first so any pooled Bitmaps will be cleared as well
    memoryCache.clearMemory(); // may return Bitmaps to pool
    bitmapPool.clearMemory();
}

All the above is based on static analysis, of course it would be nice to confirm this via debugging too.

@Caij
Copy link
Author

Caij commented Oct 16, 2015

@TWiStErRob thanks.

private EngineResource<?> getEngineResourceFromCache(Key key) {
    Resource<?> cached = cache.remove(key); 

    final EngineResource result;
    ...
    return result;
}

I am very careless, wrong for cache.get(key).

@TWiStErRob
Copy link
Collaborator

I double-checked it runtime and everything happens as predicted:

Log.i("GLIDE", "Clearing memory cache");
Glide.get(this).clearMemory();
Log.i("GLIDE", "Clearing memory cache finished");
I/GLIDE﹕ Clearing memory cache
D/LruBitmapPool﹕ clearMemory
D/LruBitmapPool﹕ Evicting bitmap=[562800](RGB_565)
V/LruBitmapPool﹕ Hits=28, misses=16, puts=38, evictions=10, currentSize=0, maxSize=33177600
    Strategy=SizeConfigStrategy{groupedMap=GroupedLinkedMap( {[1339200](RGB_565):0}, {[2048000](RGB_565):0} ), sortedSizes=(RGB_565[{}])}
-------------------- memoryCache.clearMemory() starts here ---------------------------
V/LruBitmapPool﹕ Put bitmap in pool=[1555200](RGB_565)
V/LruBitmapPool﹕ Hits=28, misses=16, puts=39, evictions=10, currentSize=1555200, maxSize=33177600
    Strategy=SizeConfigStrategy{groupedMap=GroupedLinkedMap( {[1339200](RGB_565):0}, {[2048000](RGB_565):0}, {[1555200](RGB_565):1} ), sortedSizes=(RGB_565[{1555200=1}])}
V/LruBitmapPool﹕ Put bitmap in pool=[1339200](RGB_565)
V/LruBitmapPool﹕ Hits=28, misses=16, puts=40, evictions=10, currentSize=2894400, maxSize=33177600
    Strategy=SizeConfigStrategy{groupedMap=GroupedLinkedMap( {[1339200](RGB_565):1}, {[2048000](RGB_565):0}, {[1555200](RGB_565):1} ), sortedSizes=(RGB_565[{1339200=1, 1555200=1}])}
V/LruBitmapPool﹕ Put bitmap in pool=[2048000](RGB_565)
V/LruBitmapPool﹕ Hits=28, misses=16, puts=41, evictions=10, currentSize=4942400, maxSize=33177600
    Strategy=SizeConfigStrategy{groupedMap=GroupedLinkedMap( {[1339200](RGB_565):1}, {[2048000](RGB_565):1}, {[1555200](RGB_565):1} ), sortedSizes=(RGB_565[{1339200=1, 1555200=1, 2048000=1}])}
I/GLIDE﹕ Clearing memory cache finished

What do you think @sjudd?

@Caij
Copy link
Author

Caij commented Oct 17, 2015

@TWiStErRob Thanks。

I have understood. You are right. If a Bitmap is shown an ImageView it's an active resource, which means it's not in any cache or pool.

@TWiStErRob
Copy link
Collaborator

@Caij yep, your part is resolved, but what I found can be considered a bug. It's highly related to your question so I'm just dumping related information here for @sjudd to consider.

@Caij
Copy link
Author

Caij commented Oct 17, 2015

@TWiStErRob

What‘s the bug?

@TWiStErRob
Copy link
Collaborator

Glide.clearMemory():
    bitmapPool.clearMemory(); // all bitmaps are removed and recycled
    memoryCache.clearMemory(); // all bitmaps are removed and put to pool?!
    // end of clearMemory, but pool has stuff in it, so clear doesn't work as intended

@Caij
Copy link
Author

Caij commented Oct 17, 2015

yep ,if call method in turn is right ?

memoryCache.clearMemory(); 
bitmapPool.clearMemory(); 

@sjudd
Copy link
Collaborator

sjudd commented Oct 19, 2015

Yup definitely a bug, we have the same issue in Glide 4:

https://github.com/bumptech/glide/blob/master/library/src/main/java/com/bumptech/glide/Glide.java#L332.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants