Skip to content

Add WeakMemoryCache implementation.#295

Merged
colinrtwhite merged 30 commits into
masterfrom
colin/weak_memory_cache
Mar 9, 2020
Merged

Add WeakMemoryCache implementation.#295
colinrtwhite merged 30 commits into
masterfrom
colin/weak_memory_cache

Conversation

@colinrtwhite
Copy link
Copy Markdown
Member

@colinrtwhite colinrtwhite commented Feb 16, 2020

This adds a WeakMemoryCache implementation, which holds weak references to bitmaps.

The plan is MemoryCache holds strong references and when they're evicted, pass the bitmap to the WeakMemoryCache where it will hold a weak reference until either the bitmap is no longer referenced or it is added to the BitmapPool.

The use case for this is now you can rely on an ImageLoader having a value in its memory cache if the image still has a strong reference outside of it (i.e. an ImageView). I'd argue this is much more intuitive - basically if it's on screen, it's cached. This should make shared element transitions easier to implement and will increase the overall cache hit rate of the memory cache (at the slight cost of extra overhead to manage the weak references).

The behaviour can be enabled/disabled with ImageLoaderBuilder.trackWeakReferences. Planning to enable this by default since it should be a fairly safe optimization.

@colinrtwhite colinrtwhite force-pushed the colin/weak_memory_cache branch 4 times, most recently from 33066f5 to 2f7ee78 Compare February 28, 2020 21:57
* Completely clear this image loader's memory cache and bitmap pool.
*/
@MainThread
fun clearMemory()
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

These functions have always called through to data structures that aren't thread safe, however now I'm making this assumption explicit. We could support calling these from any thread, however it would slightly slow down every image request due to synchronization so let's optimize for the common case.

You can technically still call these methods off the main thread, however it's not 100% safe due to race conditions.

*
* Default: true
*/
fun trackWeakReferences(enable: Boolean) = apply {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Definitely open to better name suggestions for this.

private var size: Size? = null

@MainThread
suspend inline fun size(cached: BitmapDrawable? = null): Size = scope.run {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Pretty sure this inline isn't actually being inlined because var size is private.

@colinrtwhite colinrtwhite changed the title [WIP] Add WeakMemoryCache implementation. Add WeakMemoryCache implementation. Mar 2, 2020
@colinrtwhite colinrtwhite marked this pull request as ready for review March 2, 2020 00:47
@colinrtwhite colinrtwhite force-pushed the colin/weak_memory_cache branch from 23f09b2 to 24e5be4 Compare March 6, 2020 06:09
@colinrtwhite colinrtwhite merged commit 1cc96f2 into master Mar 9, 2020
@colinrtwhite colinrtwhite deleted the colin/weak_memory_cache branch March 9, 2020 18:01
colinrtwhite added a commit that referenced this pull request Oct 5, 2022
* Add WeakMemoryCache implementation.

* Extensions.

* Implicit return types.

* Rename method.

* Test.

* Undo.

* First pass implementation.

* Optimize + document.

* Add a basic test.

* Add more tests.

* Move test functions out of main binary.

* Support enabling/disabling the weak memory cache.

* Rename values.

* Fix ktlint.

* Specify return type.

* Refactor.

* Test multiple API levels.

* Fix tests.

* Fix tests.

* Really fix tests.

* Add MemoryCache tests.

* Fix naming clash.

* Fix tests.

* Edit tests.

* Not null.

* More coverage.

* Remove unused function.

* Formatting.

* Documentation.

* Documentation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant