Implement the WeakReference memory cache in common code. (#2243)

coil-core/src/commonMain/kotlin/coil3/memory/WeakMemoryCache.kt

@@ -1,8 +1,16 @@
 package coil3.memory
 
 import coil3.Image
+import coil3.annotation.VisibleForTesting
 import coil3.memory.MemoryCache.Key
 import coil3.memory.MemoryCache.Value
+import coil3.util.WeakReference
+import coil3.util.firstNotNullOfOrNullIndices
+import coil3.util.identityHashCode
+import coil3.util.removeIfIndices
+import kotlin.experimental.ExperimentalNativeApi
+import kotlinx.atomicfu.locks.SynchronizedObject
+import kotlinx.atomicfu.locks.synchronized
 
 /**
  * An in-memory cache that holds weak references.
@@ -35,4 +43,108 @@ internal class EmptyWeakMemoryCache : WeakMemoryCache {
     override fun clear() {}
 }
 
-internal expect fun RealWeakMemoryCache(): WeakMemoryCache
+/** A [WeakMemoryCache] implementation backed by a [LinkedHashMap]. */
+@OptIn(ExperimentalNativeApi::class)
+internal class RealWeakMemoryCache : WeakMemoryCache {
+
+    private val lock = SynchronizedObject()
+    internal val cache = LinkedHashMap<Key, ArrayList<InternalValue>>()
+    private var operationsSinceCleanUp = 0
+
+    override val keys: Set<Key>
+        get() = synchronized(lock) { cache.keys.toSet() }
+
+    override fun get(key: Key): Value? = synchronized(lock) {
+        val values = cache[key] ?: return null
+
+        // Find the first image that hasn't been collected.
+        val value = values.firstNotNullOfOrNullIndices { value ->
+            value.image.get()?.let { Value(it, value.extras) }
+        }
+
+        cleanUpIfNecessary()
+        return value
+    }
+
+    override fun set(
+        key: Key,
+        image: Image,
+        extras: Map<String, Any>,
+        size: Long,
+    ) = synchronized(lock) {
+        val values = cache.getOrPut(key) { arrayListOf() }
+
+        // Insert the value into the list sorted descending by size.
+        run {
+            val identityHashCode = image.identityHashCode()
+            val newValue = InternalValue(identityHashCode, WeakReference(image), extras, size)
+            for (index in values.indices) {
+                val value = values[index]
+                if (size >= value.size) {
+                    if (value.identityHashCode == identityHashCode && value.image.get() === image) {
+                        values[index] = newValue
+                    } else {
+                        values.add(index, newValue)
+                    }
+                    return@run
+                }
+            }
+            values += newValue
+        }
+
+        cleanUpIfNecessary()
+    }
+
+    override fun remove(key: Key): Boolean = synchronized(lock) {
+        return cache.remove(key) != null
+    }
+
+    override fun clear() = synchronized(lock) {
+        operationsSinceCleanUp = 0
+        cache.clear()
+    }
+
+    private fun cleanUpIfNecessary() {
+        if (operationsSinceCleanUp++ >= CLEAN_UP_INTERVAL) {
+            cleanUp()
+        }
+    }
+
+    /** Remove any dereferenced images from the cache. */
+    @VisibleForTesting
+    internal fun cleanUp() {
+        operationsSinceCleanUp = 0
+
+        // Remove all the values whose references have been collected.
+        val iterator = cache.values.iterator()
+        while (iterator.hasNext()) {
+            val list = iterator.next()
+
+            if (list.size <= 1) {
+                // Typically, the list will only contain 1 item. Handle this case in an optimal way here.
+                if (list.firstOrNull()?.image?.get() == null) {
+                    iterator.remove()
+                }
+            } else {
+                // Iterate over the list of values and delete individual entries that have been collected.
+                list.removeIfIndices { it.image.get() == null }
+
+                if (list.isEmpty()) {
+                    iterator.remove()
+                }
+            }
+        }
+    }
+
+    @VisibleForTesting
+    internal class InternalValue(
+        val identityHashCode: Int,
+        val image: WeakReference<Image>,
+        val extras: Map<String, Any>,
+        val size: Long,
+    )
+
+    companion object {
+        private const val CLEAN_UP_INTERVAL = 10
+    }
+}

coil-core/src/commonMain/kotlin/coil3/util/utils.common.kt

@@ -16,6 +16,7 @@ import coil3.size.Scale
 import coil3.size.Size
 import coil3.size.isOriginal
 import coil3.size.pxOrElse
+import kotlin.experimental.ExperimentalNativeApi
 import kotlin.reflect.KClass
 import okio.Closeable
 
@@ -109,3 +110,11 @@ internal fun isFileUri(uri: Uri): Boolean {
 }
 
 internal expect fun isAssetUri(uri: Uri): Boolean
+
+@ExperimentalNativeApi // This must be propagated from the underlying native implementation.
+internal expect class WeakReference<T : Any>(referred: T) {
+    fun get(): T?
+    fun clear()
+}
+
+internal expect fun Any.identityHashCode(): Int

coil-core/src/nonJsCommonTest/kotlin/coil3/memory/RealMemoryCacheTest.kt → coil-core/src/commonTest/kotlin/coil3/memory/RealMemoryCacheTest.kt

@@ -16,7 +16,7 @@ class RealMemoryCacheTest {
     private val cache = RealMemoryCache(strongCache, weakCache)
 
     @Test
-    fun `can retrieve strong cached value`() {
+    fun canRetrieveStrongCachedValue() {
         val key = Key("strong")
         val image = FakeImage()
 
@@ -28,7 +28,7 @@ class RealMemoryCacheTest {
     }
 
     @Test
-    fun `can retrieve weak cached value`() {
+    fun canRetrieveWeakCachedValue() {
         val key = Key("weak")
         val image = FakeImage()
 
@@ -40,7 +40,7 @@ class RealMemoryCacheTest {
     }
 
     @Test
-    fun `remove removes from both caches`() {
+    fun removeRemovesFromBothCaches() {
         val key = Key("key")
         val image = FakeImage()
 
@@ -55,7 +55,7 @@ class RealMemoryCacheTest {
     }
 
     @Test
-    fun `clear clears all values`() {
+    fun clearClearsAllValues() {
         assertEquals(0, cache.size)
 
         strongCache.set(Key("a"), FakeImage(), emptyMap(), 100)
@@ -75,7 +75,7 @@ class RealMemoryCacheTest {
     }
 
     @Test
-    fun `set can be retrieved with get`() {
+    fun setCanBeRetrievedWithGet() {
         val key = Key("a")
         val image = FakeImage()
         cache[key] = Value(image)
@@ -84,7 +84,7 @@ class RealMemoryCacheTest {
     }
 
     @Test
-    fun `setting the same bitmap multiple times can only be removed once`() {
+    fun settingTheSameImageMultipleTimesCanOnlyBeRemovedOnce() {
         val key = Key("a")
         val image = FakeImage()
 

coil-core/src/nonJsCommonTest/kotlin/coil3/memory/WeakMemoryCacheTest.kt → coil-core/src/commonTest/kotlin/coil3/memory/WeakMemoryCacheTest.kt

@@ -13,11 +13,11 @@ import kotlin.test.assertNull
 @OptIn(ExperimentalNativeApi::class)
 class WeakMemoryCacheTest {
 
-    private val weakMemoryCache = WeakReferenceMemoryCache()
+    private val weakMemoryCache = RealWeakMemoryCache()
     private val references = mutableSetOf<Image>()
 
     @Test
-    fun `can retrieve cached value`() {
+    fun canRetrieveCachedValue() {
         val key = Key("key")
         val image = reference(FakeImage())
         val extras = mapOf("test" to 4)
@@ -32,7 +32,7 @@ class WeakMemoryCacheTest {
     }
 
     @Test
-    fun `can hold multiple values`() {
+    fun canHoldMultipleValues() {
         val image1 = reference(FakeImage())
         weakMemoryCache.set(Key("key1"), image1, emptyMap(), 100)
 
@@ -54,7 +54,7 @@ class WeakMemoryCacheTest {
     }
 
     @Test
-    fun `empty references are removed from cache`() {
+    fun emptyReferencesAreRemovedFromCache() {
         val key = Key("key")
         val image = reference(FakeImage())
 
@@ -65,7 +65,7 @@ class WeakMemoryCacheTest {
     }
 
     @Test
-    fun `bitmaps with same key are retrieved by size descending`() {
+    fun imagesWithSameKeyAreRetrievedBySizeDescending() {
         val image1 = reference(FakeImage())
         val image2 = reference(FakeImage())
         val image3 = reference(FakeImage())
@@ -113,7 +113,7 @@ class WeakMemoryCacheTest {
     }
 
     @Test
-    fun `cleanUp clears all collected values`() {
+    fun cleanUpClearsAllCollectedValues() {
         val image1 = reference(FakeImage())
         weakMemoryCache.set(Key("key1"), image1, emptyMap(), 100)
 
@@ -141,7 +141,7 @@ class WeakMemoryCacheTest {
     }
 
     @Test
-    fun `value is removed after invalidate is called`() {
+    fun valueIsRemovedAfterInvalidateIsCalled() {
         val key = Key("1")
         val image = FakeImage()
         weakMemoryCache.set(key, image, emptyMap(), image.size)
@@ -153,7 +153,7 @@ class WeakMemoryCacheTest {
     /**
      * Clears [image]'s weak reference without removing its entry from the cache.
      */
-    private fun WeakReferenceMemoryCache.garbageCollect(image: Image) {
+    private fun RealWeakMemoryCache.garbageCollect(image: Image) {
         cache.values.forEach { values ->
             values.forEachIndices { value ->
                 if (value.image.get() === image) {

coil-core/src/jsMain/kotlin/coil3/util/utils.js.kt

@@ -0,0 +1,21 @@
+package coil3.util
+
+internal actual class WeakReference<T : Any> actual constructor(referred: T) {
+    private var reference: WeakRef<T>? = WeakRef(referred)
+
+    actual fun get(): T? {
+        return reference?.deref()
+    }
+
+    actual fun clear() {
+        reference = null
+    }
+}
+
+// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakRef
+private external class WeakRef<T>(value: T) {
+    fun deref(): T?
+}
+
+// TODO: This can be incorrect if the object implements a custom hashCode.
+internal actual fun Any.identityHashCode(): Int = hashCode()