From 472367b4ff7ddea8177487275ed0f90986c466bf Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 14 May 2026 11:05:13 +0000 Subject: [PATCH] Revert "perf: drain Coil disk cache off the write path to prevent scroll stalls" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit b221dce8. The KeyAccessLog/TrackingDiskCache/CoilDiskTrimmer approach rebuilt, persistently, the LRU index Coil already keeps in memory (~50 MB heap for the key map), and the trimmer drained the cache in one unbounded delete burst rather than spreading the work — potentially a worse stall than the inline eviction it replaced, just less frequent. Replacing it with a FileSystem-layer delegate that defers the eviction unlink() off the commit thread without any parallel bookkeeping. --- .../com/vitorpamplona/amethyst/AppModules.kt | 48 +--- .../service/images/CoilDiskTrimmer.kt | 111 -------- .../amethyst/service/images/KeyAccessLog.kt | 262 ------------------ .../service/images/TrackingDiskCache.kt | 68 ----- .../service/images/CoilDiskTrimmerTest.kt | 216 --------------- .../service/images/KeyAccessLogTest.kt | 234 ---------------- 6 files changed, 2 insertions(+), 937 deletions(-) delete mode 100644 amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/CoilDiskTrimmer.kt delete mode 100644 amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/KeyAccessLog.kt delete mode 100644 amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/TrackingDiskCache.kt delete mode 100644 amethyst/src/test/java/com/vitorpamplona/amethyst/service/images/CoilDiskTrimmerTest.kt delete mode 100644 amethyst/src/test/java/com/vitorpamplona/amethyst/service/images/KeyAccessLogTest.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt index a6060c052..466779ad6 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt @@ -47,12 +47,9 @@ import com.vitorpamplona.amethyst.service.connectivity.ConnectivityStatus import com.vitorpamplona.amethyst.service.crashreports.CrashReportCache import com.vitorpamplona.amethyst.service.crashreports.UnexpectedCrashSaver import com.vitorpamplona.amethyst.service.eventCache.MemoryTrimmingService -import com.vitorpamplona.amethyst.service.images.CoilDiskTrimmer import com.vitorpamplona.amethyst.service.images.ImageCacheFactory import com.vitorpamplona.amethyst.service.images.ImageLoaderSetup -import com.vitorpamplona.amethyst.service.images.KeyAccessLog import com.vitorpamplona.amethyst.service.images.ThumbnailDiskCache -import com.vitorpamplona.amethyst.service.images.TrackingDiskCache import com.vitorpamplona.amethyst.service.location.LocationState import com.vitorpamplona.amethyst.service.notifications.AlwaysOnNotificationServiceManager import com.vitorpamplona.amethyst.service.notifications.NotificationDispatcher @@ -120,7 +117,6 @@ import kotlinx.coroutines.flow.merge import kotlinx.coroutines.flow.onCompletion import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.transform -import kotlinx.coroutines.isActive import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking import java.io.File @@ -511,33 +507,10 @@ class AppModules( VideoCacheFactory.new(appContext) } - // shadow index of Coil's disk cache keys so the trimmer can evict - // oldest-first via Coil's public remove(key) API. - val coilKeyAccessLog: KeyAccessLog by lazy { - KeyAccessLog( - logFile = File(appContext.filesDir, "coil_key_access.log"), - scope = applicationIOScope, - ) - } - - // image cache in disk for coil. Wrapped in TrackingDiskCache so the - // background trimmer can keep `size < maxSize` and prevent inline - // eviction storms on the write path during feed scroll. + // image cache in disk for coil val diskCache: DiskCache by lazy { Log.d("AppModules", "ImageCacheFactory Init") - TrackingDiskCache( - delegate = ImageCacheFactory.newDisk(appContext), - keyLog = coilKeyAccessLog, - ) - } - - // Background trimmer that drains the Coil disk cache to ~55% of cap - // whenever it crosses ~70%. Runs on a fixed cadence and on onTrimMemory. - val coilDiskTrimmer: CoilDiskTrimmer by lazy { - CoilDiskTrimmer( - diskCache = { diskCache }, - keyLog = coilKeyAccessLog, - ) + ImageCacheFactory.newDisk(appContext) } // image cache in memory for coil @@ -690,19 +663,6 @@ class AppModules( delay(1_500) videoCache } - - // Replay the persisted Coil key-access log into memory so the - // background trimmer can evict oldest-first via remove(key). Then - // run the trim loop while the app is alive. The first pass is - // delayed so it doesn't compete with first-paint IO. - applicationIOScope.launch { - coilKeyAccessLog.load() - delay(30_000) - while (isActive) { - coilDiskTrimmer.trim() - delay(5 * 60 * 1000L) - } - } } fun terminate(appContext: Context) { @@ -724,10 +684,6 @@ class AppModules( dnsStore.save() val loggedIn = accountsCache.accounts.value.values trimmingService.run(loggedIn, LocalPreferences.allSavedAccounts()) - // Drain the Coil disk cache below target on backgrounding too — - // doing it here keeps the next foreground session's write path on - // the no-inline-eviction fast path. - coilDiskTrimmer.trim() } } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/CoilDiskTrimmer.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/CoilDiskTrimmer.kt deleted file mode 100644 index 065d2b685..000000000 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/CoilDiskTrimmer.kt +++ /dev/null @@ -1,111 +0,0 @@ -/* - * Copyright (c) 2025 Vitor Pamplona - * - * Permission is hereby granted, free of charge, to any person obtaining a copy of - * this software and associated documentation files (the "Software"), to deal in - * the Software without restriction, including without limitation the rights to use, - * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the - * Software, and to permit persons to whom the Software is furnished to do so, - * subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in all - * copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS - * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR - * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN - * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION - * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. - */ -package com.vitorpamplona.amethyst.service.images - -import coil3.annotation.ExperimentalCoilApi -import coil3.disk.DiskCache -import com.vitorpamplona.quartz.utils.Log -import kotlinx.coroutines.sync.Mutex - -/** - * Drains Coil's disk cache below [targetFraction] of `maxSize`, by calling - * `DiskCache.remove(key)` for the oldest keys recorded in [keyLog]. Called - * from a background coroutine on a fixed cadence (e.g. every 5 minutes) and - * on `onTrimMemory`. - * - * The point is to keep `size < maxSize` so Coil's internal `trimToSize()` - * never fires during `commit()`. Inline eviction is what stalls IO on a - * saturated cache; moving it out of the write path is the whole reason this - * class exists. - * - * On every run: - * 1. Flush [keyLog]'s pending writes so the snapshot is current. - * 2. If `size < targetFraction * maxSize`, exit (nothing to do). - * 3. Walk the log oldest-first, calling `remove(key)` until - * `size < floorFraction * maxSize`. The hysteresis gap between target and - * floor gives bursty writes some headroom before the next run is needed. - * - * Keys that the trimmer doesn't know about (entries written before this code - * was deployed, or after the in-memory map filled to capacity) stay on disk - * until Coil's own internal trim notices them — that's the safety net. - */ -@OptIn(ExperimentalCoilApi::class) -class CoilDiskTrimmer( - private val diskCache: () -> DiskCache, - private val keyLog: KeyAccessLog, - private val targetFraction: Double = 0.70, - private val floorFraction: Double = 0.55, -) { - companion object { - private const val TAG = "CoilDiskTrimmer" - } - - private val runMutex = Mutex() - - init { - require(floorFraction < targetFraction) { "floor must be < target" } - require(targetFraction in 0.0..1.0) { "target out of range" } - require(floorFraction in 0.0..1.0) { "floor out of range" } - } - - /** - * Run one trim pass. No-op if another pass is already running. - */ - suspend fun trim() { - if (!runMutex.tryLock()) return - try { - keyLog.flushNow() - val cache = diskCache() - val maxSize = cache.maxSize - if (maxSize <= 0L) return - val target = (maxSize * targetFraction).toLong() - val floor = (maxSize * floorFraction).toLong() - val before = cache.size - if (before < target) { - Log.d(TAG) { "size=${before / KB} KB < target=${target / KB} KB; nothing to do" } - return - } - - val candidates = keyLog.snapshotOldestFirst() - if (candidates.isEmpty()) { - Log.d(TAG) { "size=${before / KB} KB ≥ target=${target / KB} KB but no known keys; falling back to Coil's inline trim" } - return - } - - val started = System.currentTimeMillis() - var evicted = 0 - for (entry in candidates) { - if (cache.size <= floor) break - if (cache.remove(entry.key)) evicted++ - } - val after = cache.size - val elapsed = System.currentTimeMillis() - started - Log.d(TAG) { - "trimmed ${(before - after) / KB} KB in ${elapsed}ms " + - "(${before / KB} → ${after / KB} KB, target=${target / KB} KB, evicted=$evicted)" - } - } finally { - runMutex.unlock() - } - } - - private val KB get() = 1024L -} diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/KeyAccessLog.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/KeyAccessLog.kt deleted file mode 100644 index f9819787a..000000000 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/KeyAccessLog.kt +++ /dev/null @@ -1,262 +0,0 @@ -/* - * Copyright (c) 2025 Vitor Pamplona - * - * Permission is hereby granted, free of charge, to any person obtaining a copy of - * this software and associated documentation files (the "Software"), to deal in - * the Software without restriction, including without limitation the rights to use, - * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the - * Software, and to permit persons to whom the Software is furnished to do so, - * subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in all - * copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS - * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR - * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN - * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION - * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. - */ -package com.vitorpamplona.amethyst.service.images - -import com.vitorpamplona.quartz.utils.Log -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.Job -import kotlinx.coroutines.isActive -import kotlinx.coroutines.launch -import kotlinx.coroutines.sync.Mutex -import kotlinx.coroutines.sync.withLock -import kotlinx.coroutines.withContext -import java.io.BufferedWriter -import java.io.File -import java.io.FileWriter -import java.util.concurrent.ConcurrentHashMap -import java.util.concurrent.atomic.AtomicLong - -/** - * Persistent shadow index of Coil's disk cache: maps each Coil key to its - * last-known access timestamp. - * - * Coil 3's `DiskCache` interface does not expose iteration, so we cannot ask - * Coil "which keys exist, in LRU order." This class observes every - * `openEditor` / `openSnapshot` call via [TrackingDiskCache] and writes the - * pair `(timestamp, key)` to an append-only log. On startup the log is - * replayed into [accessTimes] so that the trimmer - * ([CoilDiskTrimmer]) can choose oldest-first eviction candidates without - * touching Coil internals. - * - * Format: - * ``` - * \t\n - * \t\n - * ... - * ``` - * Multiple lines for the same key are tolerated; the latest wins on replay. - * - * Capacity: bounded at [maxEntries]. When exceeded, the oldest entries are - * dropped from the in-memory map. Those keys become invisible to the trimmer - * (the trimmer will not call `remove` on them) but Coil's own internal trim - * remains as a backstop. - * - * Compaction: when the on-disk log grows past [compactionRatio] × in-memory - * size, the log is rewritten as a fresh snapshot. - */ -class KeyAccessLog( - private val logFile: File, - private val scope: CoroutineScope, - private val maxEntries: Int = 200_000, - private val flushIntervalMs: Long = 30_000L, - private val compactionRatio: Int = 3, -) { - companion object { - private const val TAG = "KeyAccessLog" - } - - private val accessTimes = ConcurrentHashMap() - private val pendingWrites = ConcurrentHashMap() - private val ioMutex = Mutex() - private val bytesWritten = AtomicLong(0L) - private val loaded = AtomicLong(0L) - - @Volatile private var flushJob: Job? = null - - /** Reload the in-memory map from the log file. Idempotent. */ - suspend fun load() { - if (!loaded.compareAndSet(0L, 1L)) return - withContext(Dispatchers.IO) { - ioMutex.withLock { - if (!logFile.exists()) { - bytesWritten.set(0L) - return@withLock - } - var size = 0L - try { - logFile.bufferedReader().useLines { lines -> - for (line in lines) { - size += line.length + 1 - val tab = line.indexOf('\t') - if (tab <= 0 || tab >= line.length - 1) continue - val ts = line.substring(0, tab).toLongOrNull() ?: continue - val key = line.substring(tab + 1) - val prev = accessTimes[key] - if (prev == null || ts > prev) accessTimes[key] = ts - } - } - } catch (e: Exception) { - Log.w(TAG, "Failed reading log; resetting", e) - runCatching { logFile.delete() } - accessTimes.clear() - bytesWritten.set(0L) - return@withLock - } - bytesWritten.set(size) - if (accessTimes.size > maxEntries) { - dropOldestLocked(accessTimes.size - maxEntries) - rewriteLocked() - } - Log.d(TAG) { "Loaded ${accessTimes.size} entries (${size}B on disk)" } - } - } - } - - /** Record that [key] was accessed at [timeMs] (defaults to now). */ - fun recordAccess( - key: String, - timeMs: Long = System.currentTimeMillis(), - ) { - val prev = accessTimes.put(key, timeMs) - if (prev == null || timeMs - prev >= flushIntervalMs / 2) { - pendingWrites[key] = timeMs - scheduleFlush() - } - } - - /** Forget a key. Caller is the eviction path (after `remove`). */ - fun forget(key: String) { - accessTimes.remove(key) - pendingWrites.remove(key) - } - - /** Forget every key. Caller is `DiskCache.clear()`. */ - fun forgetAll() { - accessTimes.clear() - pendingWrites.clear() - scope.launch(Dispatchers.IO) { - ioMutex.withLock { - runCatching { logFile.delete() } - bytesWritten.set(0L) - } - } - } - - /** Snapshot of all known (key, lastAccess) pairs, oldest first. */ - fun snapshotOldestFirst(): List = - accessTimes - .entries - .map { Entry(it.key, it.value) } - .sortedBy { it.lastAccessMs } - - fun size(): Int = accessTimes.size - - private fun scheduleFlush() { - if (flushJob?.isActive == true) return - synchronized(this) { - if (flushJob?.isActive == true) return - flushJob = - scope.launch(Dispatchers.IO) { - kotlinx.coroutines.delay(flushIntervalMs) - if (scope.isActive) flushNow() - } - } - } - - /** Flush pending writes immediately. Visible for tests. */ - suspend fun flushNow() { - val toWrite = - ioMutex.withLock { - if (pendingWrites.isEmpty()) return@flushNow - val drained = pendingWrites.toMap() - pendingWrites.clear() - drained - } - appendBatch(toWrite) - maybeCompact() - } - - private suspend fun appendBatch(batch: Map) { - if (batch.isEmpty()) return - ioMutex.withLock { - try { - logFile.parentFile?.mkdirs() - var addedBytes = 0L - BufferedWriter(FileWriter(logFile, true)).use { writer -> - for ((key, ts) in batch) { - if ('\n' in key || '\t' in key) continue - val line = "$ts\t$key\n" - writer.write(line) - addedBytes += line.length.toLong() - } - } - bytesWritten.addAndGet(addedBytes) - } catch (e: Exception) { - Log.w(TAG, "appendBatch failed", e) - } - } - } - - private suspend fun maybeCompact() { - val mapSize = accessTimes.size - if (mapSize == 0) return - val avgLine = 80L - val expectedBytes = mapSize * avgLine - if (bytesWritten.get() < expectedBytes * compactionRatio) return - ioMutex.withLock { - if (accessTimes.size > maxEntries) { - dropOldestLocked(accessTimes.size - maxEntries) - } - rewriteLocked() - } - } - - private fun dropOldestLocked(count: Int) { - if (count <= 0) return - val victims = - accessTimes.entries - .asSequence() - .sortedBy { it.value } - .take(count) - .map { it.key } - .toList() - for (k in victims) accessTimes.remove(k) - } - - private fun rewriteLocked() { - try { - logFile.parentFile?.mkdirs() - val tmp = File(logFile.parentFile, "${logFile.name}.tmp") - var bytes = 0L - BufferedWriter(FileWriter(tmp, false)).use { writer -> - for ((key, ts) in accessTimes) { - if ('\n' in key || '\t' in key) continue - val line = "$ts\t$key\n" - writer.write(line) - bytes += line.length.toLong() - } - } - if (!tmp.renameTo(logFile)) { - tmp.copyTo(logFile, overwrite = true) - tmp.delete() - } - bytesWritten.set(bytes) - } catch (e: Exception) { - Log.w(TAG, "rewrite failed", e) - } - } - - data class Entry( - val key: String, - val lastAccessMs: Long, - ) -} diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/TrackingDiskCache.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/TrackingDiskCache.kt deleted file mode 100644 index 8444aeff2..000000000 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/TrackingDiskCache.kt +++ /dev/null @@ -1,68 +0,0 @@ -/* - * Copyright (c) 2025 Vitor Pamplona - * - * Permission is hereby granted, free of charge, to any person obtaining a copy of - * this software and associated documentation files (the "Software"), to deal in - * the Software without restriction, including without limitation the rights to use, - * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the - * Software, and to permit persons to whom the Software is furnished to do so, - * subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in all - * copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS - * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR - * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN - * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION - * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. - */ -package com.vitorpamplona.amethyst.service.images - -import coil3.annotation.ExperimentalCoilApi -import coil3.disk.DiskCache - -/** - * `DiskCache` wrapper that records every key access into [keyLog]. - * - * Coil's built-in `DiskLruCache` evicts inline on every `commit()` once - * `size` exceeds `maxSize`. That inline eviction is what stalls feed-scroll - * IO on a saturated cache. By recording access timestamps here we give - * [CoilDiskTrimmer] enough information to run an external eviction pass on a - * background coroutine — which keeps `size < maxSize` and turns the inline - * trim into a no-op during normal use. - * - * Delegates every method to the real Coil cache via Kotlin's interface - * delegation. Recording happens before delegation so a key seen via `openEditor` - * always shows up in [keyLog] regardless of whether the editor is later - * aborted — that's fine: the trimmer is robust against `remove(key)` of an - * already-evicted key. - */ -@OptIn(ExperimentalCoilApi::class) -class TrackingDiskCache( - private val delegate: DiskCache, - private val keyLog: KeyAccessLog, -) : DiskCache by delegate { - override fun openEditor(key: String): DiskCache.Editor? { - keyLog.recordAccess(key) - return delegate.openEditor(key) - } - - override fun openSnapshot(key: String): DiskCache.Snapshot? { - val snap = delegate.openSnapshot(key) - if (snap != null) keyLog.recordAccess(key) - return snap - } - - override fun remove(key: String): Boolean { - val removed = delegate.remove(key) - if (removed) keyLog.forget(key) - return removed - } - - override fun clear() { - delegate.clear() - keyLog.forgetAll() - } -} diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/images/CoilDiskTrimmerTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/images/CoilDiskTrimmerTest.kt deleted file mode 100644 index e4593fcf3..000000000 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/images/CoilDiskTrimmerTest.kt +++ /dev/null @@ -1,216 +0,0 @@ -/* - * Copyright (c) 2025 Vitor Pamplona - * - * Permission is hereby granted, free of charge, to any person obtaining a copy of - * this software and associated documentation files (the "Software"), to deal in - * the Software without restriction, including without limitation the rights to use, - * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the - * Software, and to permit persons to whom the Software is furnished to do so, - * subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in all - * copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS - * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR - * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN - * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION - * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. - */ -package com.vitorpamplona.amethyst.service.images - -import coil3.annotation.ExperimentalCoilApi -import coil3.disk.DiskCache -import io.mockk.every -import io.mockk.mockk -import io.mockk.verify -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.SupervisorJob -import kotlinx.coroutines.async -import kotlinx.coroutines.awaitAll -import kotlinx.coroutines.cancel -import kotlinx.coroutines.coroutineScope -import kotlinx.coroutines.test.runTest -import org.junit.After -import org.junit.Assert.assertEquals -import org.junit.Assert.assertTrue -import org.junit.Before -import org.junit.Test -import java.io.File -import java.nio.file.Files -import java.util.concurrent.atomic.AtomicLong - -@OptIn(ExperimentalCoilApi::class) -class CoilDiskTrimmerTest { - private lateinit var tmpDir: File - private lateinit var logFile: File - private val scope = CoroutineScope(Dispatchers.IO + SupervisorJob()) - - @Before - fun setUp() { - tmpDir = Files.createTempDirectory("coil-trimmer-test").toFile() - logFile = File(tmpDir, "log.tsv") - } - - @After - fun tearDown() { - scope.cancel("tearDown") - tmpDir.deleteRecursively() - } - - /** - * Tiny fake of [DiskCache] that only models `size`, `maxSize`, and - * `remove`. Each call to `remove(key)` decrements `size` by the per-entry - * byte budget we hand in. Other methods of `DiskCache` are not relevant - * to the trimmer and are stubbed via mockk so any accidental call would - * blow up loud in tests. - */ - private class FakeDiskCache( - override val maxSize: Long, - private val bytesPerEntry: Long, - ) : DiskCache by mockk(relaxed = false) { - private val current = AtomicLong(0L) - - override val size: Long get() = current.get() - - fun setSize(bytes: Long) { - current.set(bytes) - } - - override fun remove(key: String): Boolean { - current.addAndGet(-bytesPerEntry) - return true - } - } - - @Test - fun trim_belowTarget_isNoop() = - runTest { - val cache = FakeDiskCache(maxSize = 1000L, bytesPerEntry = 10L) - cache.setSize(500L) // 50% — below 70% target - - val log = KeyAccessLog(logFile, scope, flushIntervalMs = 1L) - log.load() - for (i in 1..50) log.recordAccess("k$i", timeMs = i.toLong()) - log.flushNow() - - val trimmer = CoilDiskTrimmer({ cache }, log) - trimmer.trim() - - assertEquals(500L, cache.size) - } - - @Test - fun trim_aboveTarget_drainsToFloor() = - runTest { - val cache = FakeDiskCache(maxSize = 1000L, bytesPerEntry = 10L) - cache.setSize(900L) // 90%, well above 70% target - - val log = KeyAccessLog(logFile, scope, flushIntervalMs = 1L) - log.load() - // 90 known entries so we definitely have enough to evict. - for (i in 1..90) log.recordAccess("k$i", timeMs = i.toLong()) - log.flushNow() - - val trimmer = CoilDiskTrimmer({ cache }, log) - trimmer.trim() - - // Floor is 55% = 550 bytes. Trimmer stops at the first call where - // size has dipped at or below the floor — with 10-byte entries we - // stop at exactly 550 (35 evictions: 900 - 35*10 = 550). - assertTrue("size=${cache.size} should be at/below floor 550", cache.size <= 550L) - assertTrue("size=${cache.size} should not over-evict past floor", cache.size >= 540L) - } - - @Test - fun trim_evictsOldestFirst() = - runTest { - val cache = FakeDiskCache(maxSize = 1000L, bytesPerEntry = 10L) - cache.setSize(800L) // 80%, above target - - val log = KeyAccessLog(logFile, scope, flushIntervalMs = 1L) - log.load() - log.recordAccess("oldest", timeMs = 100) - log.recordAccess("middle", timeMs = 200) - log.recordAccess("newest", timeMs = 300) - // Pad the log so we have plenty of keys to choose from. - for (i in 1..30) log.recordAccess("pad$i", timeMs = 1000L + i) - log.flushNow() - - val evicted = mutableListOf() - val tracked = - object : DiskCache by cache { - override fun remove(key: String): Boolean { - evicted += key - return cache.remove(key) - } - } - - val trimmer = CoilDiskTrimmer({ tracked }, log) - trimmer.trim() - - // First three evictions must be the three oldest in that order. - assertTrue("evicted=$evicted; first 3 must be oldest-first", evicted.size >= 3) - assertEquals("oldest", evicted[0]) - assertEquals("middle", evicted[1]) - assertEquals("newest", evicted[2]) - } - - @Test - fun trim_emptyKeyLog_isNoopEvenWhenOverTarget() = - runTest { - val cache = FakeDiskCache(maxSize = 1000L, bytesPerEntry = 10L) - cache.setSize(950L) // 95%, well above target - - val log = KeyAccessLog(logFile, scope, flushIntervalMs = 1L) - log.load() // no entries - - val trimmer = CoilDiskTrimmer({ cache }, log) - trimmer.trim() - - // No known keys to evict; relies on Coil's own internal trim. - assertEquals(950L, cache.size) - } - - @Test - fun trim_zeroMaxSize_isNoop() = - runTest { - val cache = mockk(relaxed = true) - every { cache.maxSize } returns 0L - every { cache.size } returns 100L - - val log = KeyAccessLog(logFile, scope, flushIntervalMs = 1L) - log.load() - log.recordAccess("a", timeMs = 100) - log.flushNow() - - val trimmer = CoilDiskTrimmer({ cache }, log) - trimmer.trim() - - verify(exactly = 0) { cache.remove(any()) } - } - - @Test - fun trim_concurrentCalls_serialise() = - runTest { - val cache = FakeDiskCache(maxSize = 1000L, bytesPerEntry = 10L) - cache.setSize(900L) - - val log = KeyAccessLog(logFile, scope, flushIntervalMs = 1L) - log.load() - for (i in 1..100) log.recordAccess("k$i", timeMs = i.toLong()) - log.flushNow() - - val trimmer = CoilDiskTrimmer({ cache }, log) - - // Two concurrent trim() calls: the second should tryLock-fail and - // bail out; the first should still drain to floor. - coroutineScope { - listOf(async { trimmer.trim() }, async { trimmer.trim() }).awaitAll() - } - - assertTrue(cache.size <= 550L) - } -} diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/images/KeyAccessLogTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/images/KeyAccessLogTest.kt deleted file mode 100644 index e2a1f1fcd..000000000 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/images/KeyAccessLogTest.kt +++ /dev/null @@ -1,234 +0,0 @@ -/* - * Copyright (c) 2025 Vitor Pamplona - * - * Permission is hereby granted, free of charge, to any person obtaining a copy of - * this software and associated documentation files (the "Software"), to deal in - * the Software without restriction, including without limitation the rights to use, - * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the - * Software, and to permit persons to whom the Software is furnished to do so, - * subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in all - * copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS - * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR - * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN - * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION - * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. - */ -package com.vitorpamplona.amethyst.service.images - -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.SupervisorJob -import kotlinx.coroutines.cancel -import kotlinx.coroutines.test.runTest -import org.junit.After -import org.junit.Assert.assertEquals -import org.junit.Assert.assertNotNull -import org.junit.Assert.assertNull -import org.junit.Assert.assertTrue -import org.junit.Before -import org.junit.Test -import java.io.File -import java.nio.file.Files - -class KeyAccessLogTest { - private lateinit var tmpDir: File - private lateinit var logFile: File - private val scope = CoroutineScope(Dispatchers.IO + SupervisorJob()) - - @Before - fun setUp() { - tmpDir = Files.createTempDirectory("coil-keylog-test").toFile() - logFile = File(tmpDir, "log.tsv") - } - - @After - fun tearDown() { - scope.cancel("tearDown") - tmpDir.deleteRecursively() - } - - @Test - fun recordAccess_persistsAndReplaysOnLoad() = - runTest { - val log = KeyAccessLog(logFile, scope, flushIntervalMs = 1L) - log.load() - - log.recordAccess("a", timeMs = 100) - log.recordAccess("b", timeMs = 200) - log.recordAccess("c", timeMs = 300) - log.flushNow() - - val reloaded = KeyAccessLog(logFile, scope, flushIntervalMs = 1L) - reloaded.load() - val snapshot = reloaded.snapshotOldestFirst() - - assertEquals(3, snapshot.size) - assertEquals(listOf("a", "b", "c"), snapshot.map { it.key }) - assertEquals(listOf(100L, 200L, 300L), snapshot.map { it.lastAccessMs }) - } - - @Test - fun recordAccess_laterTimestampWins() = - runTest { - val log = KeyAccessLog(logFile, scope, flushIntervalMs = 1L) - log.load() - - log.recordAccess("a", timeMs = 100) - log.recordAccess("a", timeMs = 500) - log.recordAccess("a", timeMs = 300) - log.flushNow() - - val reloaded = KeyAccessLog(logFile, scope, flushIntervalMs = 1L) - reloaded.load() - val snapshot = reloaded.snapshotOldestFirst() - - assertEquals(1, snapshot.size) - assertEquals("a", snapshot[0].key) - assertEquals(500L, snapshot[0].lastAccessMs) - } - - @Test - fun forget_removesKey() = - runTest { - val log = KeyAccessLog(logFile, scope, flushIntervalMs = 1L) - log.load() - - log.recordAccess("a", timeMs = 100) - log.recordAccess("b", timeMs = 200) - log.forget("a") - log.flushNow() - - assertEquals(listOf("b"), log.snapshotOldestFirst().map { it.key }) - } - - @Test - fun forgetAll_clearsLogFile() = - runTest { - val log = KeyAccessLog(logFile, scope, flushIntervalMs = 1L) - log.load() - - log.recordAccess("a", timeMs = 100) - log.flushNow() - assertTrue(logFile.exists()) - - log.forgetAll() - // Give the IO-dispatched delete a moment. - kotlinx.coroutines.delay(100) - - assertEquals(0, log.snapshotOldestFirst().size) - } - - @Test - fun snapshotOldestFirst_sortsByTimestamp() = - runTest { - val log = KeyAccessLog(logFile, scope, flushIntervalMs = 1L) - log.load() - - log.recordAccess("newest", timeMs = 500) - log.recordAccess("oldest", timeMs = 100) - log.recordAccess("middle", timeMs = 300) - log.flushNow() - - val snapshot = log.snapshotOldestFirst() - assertEquals(listOf("oldest", "middle", "newest"), snapshot.map { it.key }) - } - - @Test - fun load_isIdempotent() = - runTest { - val log = KeyAccessLog(logFile, scope, flushIntervalMs = 1L) - log.recordAccess("a", timeMs = 100) - log.flushNow() - - log.load() - log.load() // second call should be a no-op - assertEquals(1, log.snapshotOldestFirst().size) - } - - @Test - fun maxEntries_dropsOldestOnOverflow() = - runTest { - val log = - KeyAccessLog( - logFile, - scope, - flushIntervalMs = 1L, - maxEntries = 3, - compactionRatio = 1, - ) - log.load() - - for (i in 1..10) log.recordAccess("k$i", timeMs = i.toLong() * 100) - log.flushNow() - // Force compaction so overflow trimming runs. - for (i in 11..20) log.recordAccess("k$i", timeMs = i.toLong() * 100) - log.flushNow() - - // Reload and verify only the newest survived. - val reloaded = KeyAccessLog(logFile, scope, flushIntervalMs = 1L, maxEntries = 3) - reloaded.load() - val keys = reloaded.snapshotOldestFirst().map { it.key }.toSet() - assertTrue("only newest 3 should survive, got=$keys", keys.size <= 3) - // The newest entries must include k20. - assertTrue("k20 must survive", "k20" in keys) - } - - @Test - fun load_recoversFromCorruptedLines() = - runTest { - logFile.parentFile?.mkdirs() - logFile.writeText( - buildString { - appendLine("100\tgood-a") - appendLine("not a number\tbad") - appendLine("\tno-timestamp") - appendLine("200") - appendLine("300\tgood-b") - }, - ) - - val log = KeyAccessLog(logFile, scope, flushIntervalMs = 1L) - log.load() - val keys = log.snapshotOldestFirst().map { it.key } - assertEquals(listOf("good-a", "good-b"), keys) - } - - @Test - fun keysWithTabOrNewline_areSkippedOnWrite() = - runTest { - val log = KeyAccessLog(logFile, scope, flushIntervalMs = 1L) - log.load() - - log.recordAccess("ok", timeMs = 100) - log.recordAccess("has\ttab", timeMs = 200) - log.recordAccess("has\nnewline", timeMs = 300) - log.flushNow() - - val reloaded = KeyAccessLog(logFile, scope, flushIntervalMs = 1L) - reloaded.load() - // Only the clean key should round-trip. - assertEquals(listOf("ok"), reloaded.snapshotOldestFirst().map { it.key }) - } - - @Test - fun size_reflectsInMemoryCount() = - runTest { - val log = KeyAccessLog(logFile, scope, flushIntervalMs = 1L) - log.load() - - assertEquals(0, log.size()) - log.recordAccess("a", timeMs = 100) - log.recordAccess("b", timeMs = 200) - assertEquals(2, log.size()) - log.forget("a") - assertEquals(1, log.size()) - - assertNotNull(log.snapshotOldestFirst().firstOrNull { it.key == "b" }) - assertNull(log.snapshotOldestFirst().firstOrNull { it.key == "a" }) - } -}