laantungir/amethyst
1
0
Fork 0
mirror of https://github.com/vitorpamplona/amethyst.git synced 2026-10-06 11:48:24 +00:00
Code Issues Packages Projects Releases Wiki Activity

perf(images): rate-limit the cache reconciler to once a day

Browse Source 
The reconciler ran on every AppModules.initiate(), which is every process
start — including the WorkManager wake-ups that cold-start the whole graph, and
which the ledger already counts because that churn is a known problem. Even the
healthy pass is a readdir plus a stat per file: on a full 1 GB cache that is
tens of thousands of syscalls, paid on every start, to look for drift that
accrues only when a process dies with unlinks still queued.

reconcileIfDue() gates the walk on the mtime of an empty `.reconciled` marker in
the cache directory, so a start inside the interval costs one stat instead. The
marker lives with the thing it describes: clearing the app's cache from Settings
takes it too, and the next start reconciles a directory whose history we no
longer know.

A marker dated in the future — a clock that jumped back, or a restored backup —
counts as due, so it cannot park the check until real time catches up.

The marker is a plain file in the swept directory, so it joins the journal files
in the preserved set. reconcileIfDue() rewrites it after a pass anyway, which is
exactly why theMarkerSurvivesAWipe drives reconcile() directly — through
reconcileIfDue() the rewrite masks the deletion and the test guards nothing.
Verified by mutation: dropping the marker from the preserved set fails that
test, and short-circuiting isDue() fails both interval tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017RYCgbvhtBCBNVLMxSWoCJ
This commit is contained in:
Claude
2026-09-07 15:41:29 +00:00
parent 8ded3c13c5
commit 0abcf02384
3 changed files with 192 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt
+6 -3
View File
@@ -1141,11 +1141,14 @@ class AppModules(
// queued unlinks — Coil cannot see them, so without this the directory keeps every killed
// process's residue and drifts past its own cap forever. See ImageDiskCacheReconciler.
//
// Rate-limited to once a day: drift accrues over process deaths, not over startups, and this
// runs on every one of them — including the WorkManager wake-ups that cold-start the graph.
//
// Also the one place that forces the `diskCache` lazy, so its build (a statvfs for the size
// budget) and this walk both land on IO rather than on whichever thread loads an image first.
// budget) and the check both land on IO rather than on whichever thread loads an image first.
applicationIOScope.launch {
val result = ImageDiskCacheReconciler.reconcile(diskCache)
if (result.wasOverBudget) {
val result = ImageDiskCacheReconciler.reconcileIfDue(diskCache)
if (result != null && result.wasOverBudget) {
Log.i("AppModules") { "Image cache was over budget: wiped ${result.reclaimedFiles} files (${result.bytesOnDisk} bytes on disk, ${result.budgetBytes} budget)" }
}
}
amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/ImageDiskCacheReconciler.kt
+77 -6
View File
@@ -55,9 +55,15 @@ data class ImageCacheReconciliation(
* empty the journal, then unlink whatever is still sitting in the directory.
*
* This is deliberately blunt. It costs the whole cache, so it only fires once the directory has
* drifted past [ceilingBytes] — well beyond the slack normal operation needs — and on a healthy
* cache it is one directory walk and nothing else. It does not read the journal, so it stays
* independent of Coil's on-disk format.
* drifted past [ceilingBytes] — well beyond the slack normal operation needs. It does not read the
* journal, so it stays independent of Coil's on-disk format.
*
* Drift accrues only when a process dies with unlinks still queued, which is slow, so
* [reconcileIfDue] rate-limits the check to [DEFAULT_INTERVAL_MS]. Even the healthy pass is a
* `readdir` plus a `stat` per file — on a full 1 GB cache, tens of thousands of syscalls — and
* `AppModules.initiate()` runs on every process start, including the WorkManager wake-ups that
* cold-start the whole graph. Gating it on a marker file's mtime costs one `stat` on the starts
* that skip.
*/
object ImageDiskCacheReconciler {
/**
@@ -72,9 +78,22 @@ object ImageDiskCacheReconciler {
*/
private const val DEFAULT_MIN_SLACK_BYTES = 4L * 1024 * 1024
/** How long one pass is good for. Drift accrues over days, so checking daily is ample. */
private const val DEFAULT_INTERVAL_MS = 24L * 60 * 60 * 1000
/**
* Empty file whose mtime is the last pass. It lives inside the cache directory so it travels
* with the thing it describes: clearing the app's cache from Settings takes the marker with it,
* and the next start reconciles a directory whose history we no longer know.
*/
private const val MARKER_FILE = ".reconciled"
/** Coil's own bookkeeping, which is not entry data and must survive a wipe. */
private val JOURNAL_FILES = setOf("journal", "journal.tmp", "journal.bkp")
/** Files in the cache directory that are not entry data and must survive a wipe. */
private val PRESERVED_FILES = JOURNAL_FILES + MARKER_FILE
/** Bytes the directory may hold before [reconcile] wipes it. */
fun ceilingBytes(
budgetBytes: Long,
@@ -83,14 +102,66 @@ object ImageDiskCacheReconciler {
): Long = budgetBytes + maxOf((budgetBytes * slackFraction).toLong(), minSlackBytes)
/**
* Walks the cache directory and, if it holds more than [ceilingBytes], empties it.
* Runs [reconcile] if the last pass is older than [intervalMs], else returns null having done
* one `stat`.
*
* Blocking IO — call it from a background dispatcher.
*/
fun reconcileIfDue(
diskCache: DiskCache,
now: Long = System.currentTimeMillis(),
intervalMs: Long = DEFAULT_INTERVAL_MS,
slackFraction: Double = DEFAULT_SLACK_FRACTION,
minSlackBytes: Long = DEFAULT_MIN_SLACK_BYTES,
): ImageCacheReconciliation? {
if (!isDue(diskCache, now, intervalMs)) return null
return reconcile(diskCache, slackFraction, minSlackBytes).also { markPass(diskCache) }
}
/**
* True when no pass is recorded, or the recorded one is [intervalMs] old.
*
* A marker dated in the future — a clock that jumped back, or a restored backup — would
* otherwise park the check until real time caught up, so that also counts as due.
*/
private fun isDue(
diskCache: DiskCache,
now: Long,
intervalMs: Long,
): Boolean {
val lastPass =
try {
diskCache.fileSystem.metadataOrNull(diskCache.directory / MARKER_FILE)?.lastModifiedAtMillis
} catch (e: IOException) {
Log.d("ImageDiskCache") { "could not stat the marker: ${e.message}" }
null
} ?: return true
return now - lastPass >= intervalMs || lastPass > now
}
/** Records that a pass just happened, by writing the marker's mtime to now. */
private fun markPass(diskCache: DiskCache) {
try {
diskCache.fileSystem.createDirectories(diskCache.directory)
diskCache.fileSystem.write(diskCache.directory / MARKER_FILE) {}
} catch (e: IOException) {
// Losing the marker only costs a redundant walk on the next start.
Log.d("ImageDiskCache") { "could not write the marker: ${e.message}" }
}
}
/**
* Walks the cache directory and, if it holds more than [ceilingBytes], empties it.
*
* Blocking IO — call it from a background dispatcher. Prefer [reconcileIfDue]; this is the
* unconditional pass.
*
* Deliberately does not read `DiskCache.size`: that would force the journal parse on the happy
* path, and the decision does not need it. A request racing the wipe can lose the entry it was
* writing; Coil treats that as a cache miss and re-fetches, which is why this runs at startup
* rather than on a timer.
* rather than while the feed is scrolling.
*/
fun reconcile(
diskCache: DiskCache,
@@ -118,7 +189,7 @@ object ImageDiskCacheReconciler {
var reclaimed = 0
regularFiles(diskCache).forEach { path ->
if (path.name in JOURNAL_FILES) return@forEach
if (path.name in PRESERVED_FILES) return@forEach
try {
diskCache.fileSystem.delete(path, mustExist = false)
reclaimed++
amethyst/src/test/java/com/vitorpamplona/amethyst/service/images/ImageDiskCacheReconcilerTest.kt
+109
View File
@@ -33,6 +33,7 @@ import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNotNull
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
@@ -234,4 +235,112 @@ class ImageDiskCacheReconcilerTest {
// over the journal plus a couple of in-flight writes.
assertEquals(1024L * 1024 + 4L * 1024 * 1024, ImageDiskCacheReconciler.ceilingBytes(1024L * 1024))
}
// ---- cadence ----------------------------------------------------------------------------
// The healthy pass is a readdir plus a stat per file, and AppModules.initiate() runs on every
// process start — including the WorkManager wake-ups that cold-start the whole graph. Drift
// accrues over process deaths, not startups, so the check is rate-limited.
private val markerPath get() = cacheDir / ".reconciled"
@Test
fun aCacheWithNoRecordedPassIsDue() {
val diskCache = newCache(DeferredDeleteFileSystem(FileSystem.SYSTEM, scope), 1024L * 1024)
try {
assertNotNull(ImageDiskCacheReconciler.reconcileIfDue(diskCache))
assertNotNull("the pass must record itself", FileSystem.SYSTEM.metadataOrNull(markerPath))
} finally {
diskCache.shutdown()
}
}
@Test
fun aSecondStartWithinTheIntervalSkipsTheWalk() {
val fs = DeferredDeleteFileSystem(FileSystem.SYSTEM, scope)
val budget = 16L * 1024
val diskCache = newCache(fs, budget)
try {
val now = System.currentTimeMillis()
assertNotNull(ImageDiskCacheReconciler.reconcileIfDue(diskCache, now = now))
// Even a directory well over budget is left alone until the next pass is due — the
// point of the gate is that the common start does no work at all.
repeat(40) { i -> writeEntry(diskCache, "key$i", 1024) }
val overBudget = bytesOnDisk()
val skipped = ImageDiskCacheReconciler.reconcileIfDue(diskCache, now = now + 1000, minSlackBytes = 0)
assertNull("within the interval the pass must not run", skipped)
assertEquals(overBudget, bytesOnDisk())
} finally {
diskCache.shutdown()
}
}
@Test
fun aStartAfterTheIntervalIsDueAgain() {
val fs = DeferredDeleteFileSystem(FileSystem.SYSTEM, scope)
val diskCache = newCache(fs, 1024L * 1024)
try {
val now = System.currentTimeMillis()
val interval = 24L * 60 * 60 * 1000
ImageDiskCacheReconciler.reconcileIfDue(diskCache, now = now, intervalMs = interval)
assertNull(ImageDiskCacheReconciler.reconcileIfDue(diskCache, now = now + interval - 1, intervalMs = interval))
assertNotNull(ImageDiskCacheReconciler.reconcileIfDue(diskCache, now = now + interval, intervalMs = interval))
} finally {
diskCache.shutdown()
}
}
@Test
fun aMarkerDatedInTheFutureDoesNotParkTheCheck() {
// A clock that jumped back, or a restored backup, would otherwise strand the check until
// real time caught up with the marker.
val fs = DeferredDeleteFileSystem(FileSystem.SYSTEM, scope)
val diskCache = newCache(fs, 1024L * 1024)
try {
val now = System.currentTimeMillis()
ImageDiskCacheReconciler.reconcileIfDue(diskCache, now = now)
assertNotNull(ImageDiskCacheReconciler.reconcileIfDue(diskCache, now = now - 365L * 24 * 60 * 60 * 1000))
} finally {
diskCache.shutdown()
}
}
@Test
fun theMarkerSurvivesAWipe() {
// The marker is a plain file in the cache directory, so the sweep would take it unless it is
// preserved explicitly — and a wipe that erases its own record makes every later start look
// due and walk again. Drives reconcile() rather than reconcileIfDue(), because the latter
// rewrites the marker afterwards and would mask the deletion.
val deadProcessFs = DeferredDeleteFileSystem(FileSystem.SYSTEM, scope)
scope.cancel("inert drainer")
val budget = 16L * 1024
val first = newCache(deadProcessFs, budget)
repeat(40) { i -> writeEntry(first, "key$i", 1024) }
val deadline = System.currentTimeMillis() + 5_000
while (first.size > budget && System.currentTimeMillis() < deadline) Thread.sleep(20)
first.shutdown()
val liveScope = CoroutineScope(Dispatchers.IO + SupervisorJob())
val liveFs = DeferredDeleteFileSystem(FileSystem.SYSTEM, liveScope)
val second = newCache(liveFs, budget)
try {
// Lay down a marker the way a previous pass would have.
FileSystem.SYSTEM.write(markerPath) {}
val result = ImageDiskCacheReconciler.reconcile(second, minSlackBytes = 0)
assertTrue("expected a wipe", result.wasOverBudget)
liveFs.drainNow()
assertNotNull("the marker must outlive the wipe", FileSystem.SYSTEM.metadataOrNull(markerPath))
} finally {
second.shutdown()
liveScope.cancel()
}
}
}