fix(artwork): restore the shared image cache eviction lost with the Slint frontend
ImageCacheService::evict had no caller since 69ed61207 retired the Slint
app's spawn_evict, so ~/.cache/qbz/images grew without bound in 2.1.0 and
2.1.1. housekeeping() now trims it to the same 200 MB budget once per run,
in 64-row batches so the GUI thread is never parked behind the cache
mutex. evict is batched and covered by unit tests.
cc
This commit is contained in:
4 files changed
+183
-36
No files matched your search
@@ -3,9 +3,9 @@
|
||||
//! LRU disk cache for Qobuz album/artist images.
|
||||
//! - Stores images keyed by MD5 hash of URL
|
||||
//! - Tracks last-access time for LRU eviction
|
||||
//! - Respects a configurable max size
|
||||
//! - Framework-agnostic: shared by the Tauri app and the Slint shell so
|
||||
//! both read and write the same `~/.cache/qbz/images` cache.
|
||||
//! - Respects a configurable max size ([`ImageCacheService::evict`], batched)
|
||||
//! - Framework-agnostic: the Qt shell owns the policy (`artwork_qt.rs`), this
|
||||
//! crate owns the store (`~/.cache/qbz/images`).
|
||||
|
||||
use md5::{Digest, Md5};
|
||||
use rusqlite::{params, Connection};
|
||||
@@ -18,6 +18,13 @@ pub struct ImageCacheStats {
|
||||
pub file_count: u64,
|
||||
}
|
||||
|
||||
/// Outcome of one [`ImageCacheService::evict`] batch.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub struct EvictBatch {
|
||||
pub freed_bytes: u64,
|
||||
pub evicted_entries: usize,
|
||||
}
|
||||
|
||||
pub struct ImageCacheService {
|
||||
cache_dir: PathBuf,
|
||||
conn: Connection,
|
||||
@@ -29,7 +36,12 @@ impl ImageCacheService {
|
||||
.ok_or_else(|| "Could not find cache directory".to_string())?
|
||||
.join("qbz")
|
||||
.join("images");
|
||||
Self::open_at(cache_dir)
|
||||
}
|
||||
|
||||
/// Open (or create) the cache at an explicit directory. `new` is this on
|
||||
/// `~/.cache/qbz/images`; tests use a temp dir.
|
||||
pub fn open_at(cache_dir: PathBuf) -> Result<Self, String> {
|
||||
std::fs::create_dir_all(&cache_dir)
|
||||
.map_err(|e| format!("Failed to create image cache dir: {}", e))?;
|
||||
|
||||
@@ -54,6 +66,18 @@ impl ImageCacheService {
|
||||
Ok(Self { cache_dir, conn })
|
||||
}
|
||||
|
||||
/// Test hook: rewrite an entry's `last_accessed` stamp.
|
||||
#[cfg(test)]
|
||||
fn backdate(&self, url: &str, last_accessed: i64) {
|
||||
let hash = Self::url_hash(url);
|
||||
self.conn
|
||||
.execute(
|
||||
"UPDATE cached_images SET last_accessed = ?1 WHERE hash = ?2",
|
||||
params![last_accessed, hash],
|
||||
)
|
||||
.expect("backdate");
|
||||
}
|
||||
|
||||
fn url_hash(url: &str) -> String {
|
||||
let mut hasher = Md5::new();
|
||||
hasher.update(url.as_bytes());
|
||||
@@ -115,8 +139,14 @@ impl ImageCacheService {
|
||||
Ok(path)
|
||||
}
|
||||
|
||||
/// Evict least-recently-accessed entries until total size is under max_bytes.
|
||||
pub fn evict(&self, max_bytes: u64) -> Result<u64, String> {
|
||||
/// Evict least-recently-accessed entries until the total size is under
|
||||
/// `max_bytes`, deleting at most `max_entries` rows per call.
|
||||
///
|
||||
/// Batched on purpose: the Qt shell holds a process-wide mutex around
|
||||
/// this service and its GUI thread takes that mutex to resolve covers,
|
||||
/// so a caller trims in small batches and releases the lock in between.
|
||||
/// Loop until `evicted_entries == 0`.
|
||||
pub fn evict(&self, max_bytes: u64, max_entries: usize) -> Result<EvictBatch, String> {
|
||||
let total: i64 = self
|
||||
.conn
|
||||
.query_row(
|
||||
@@ -126,21 +156,23 @@ impl ImageCacheService {
|
||||
)
|
||||
.map_err(|e| format!("Failed to query cache size: {}", e))?;
|
||||
|
||||
let mut batch = EvictBatch {
|
||||
freed_bytes: 0,
|
||||
evicted_entries: 0,
|
||||
};
|
||||
if (total as u64) <= max_bytes {
|
||||
return Ok(0);
|
||||
return Ok(batch);
|
||||
}
|
||||
|
||||
let mut to_free = (total as u64) - max_bytes;
|
||||
let mut freed: u64 = 0;
|
||||
|
||||
// Get LRU entries (oldest access first)
|
||||
// LRU entries (oldest access first), bounded to this batch.
|
||||
let mut stmt = self
|
||||
.conn
|
||||
.prepare("SELECT hash, file_size FROM cached_images ORDER BY last_accessed ASC")
|
||||
.prepare("SELECT hash, file_size FROM cached_images ORDER BY last_accessed ASC LIMIT ?1")
|
||||
.map_err(|e| format!("Failed to prepare eviction query: {}", e))?;
|
||||
|
||||
let limit = i64::try_from(max_entries).unwrap_or(i64::MAX);
|
||||
let entries: Vec<(String, i64)> = stmt
|
||||
.query_map([], |row| Ok((row.get(0)?, row.get(1)?)))
|
||||
.query_map(params![limit], |row| Ok((row.get(0)?, row.get(1)?)))
|
||||
.map_err(|e| format!("Failed to query LRU entries: {}", e))?
|
||||
.filter_map(|r| r.ok())
|
||||
.collect();
|
||||
@@ -157,11 +189,12 @@ impl ImageCacheService {
|
||||
.conn
|
||||
.execute("DELETE FROM cached_images WHERE hash = ?1", params![hash]);
|
||||
let size = file_size as u64;
|
||||
freed += size;
|
||||
batch.freed_bytes += size;
|
||||
batch.evicted_entries += 1;
|
||||
to_free = to_free.saturating_sub(size);
|
||||
}
|
||||
|
||||
Ok(freed)
|
||||
Ok(batch)
|
||||
}
|
||||
|
||||
/// Get cache statistics.
|
||||
@@ -203,3 +236,80 @@ impl ImageCacheService {
|
||||
Ok(stats.total_bytes)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
fn fresh_dir(tag: &str) -> PathBuf {
|
||||
let dir = std::env::temp_dir().join(format!("qbz-image-cache-{tag}-{}", std::process::id()));
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
dir
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn evict_drops_least_recently_accessed_first_and_unlinks_files() {
|
||||
let dir = fresh_dir("lru");
|
||||
let cache = ImageCacheService::open_at(dir.clone()).unwrap();
|
||||
let oldest = cache.store("https://img/oldest", &[0u8; 100]).unwrap();
|
||||
let middle = cache.store("https://img/middle", &[0u8; 100]).unwrap();
|
||||
let newest = cache.store("https://img/newest", &[0u8; 100]).unwrap();
|
||||
cache.backdate("https://img/oldest", 1_000);
|
||||
cache.backdate("https://img/middle", 2_000);
|
||||
cache.backdate("https://img/newest", 3_000);
|
||||
|
||||
let batch = cache.evict(150, usize::MAX).unwrap();
|
||||
|
||||
assert_eq!(batch, EvictBatch { freed_bytes: 200, evicted_entries: 2 });
|
||||
assert!(!oldest.exists());
|
||||
assert!(!middle.exists());
|
||||
assert!(newest.exists());
|
||||
let stats = cache.stats().unwrap();
|
||||
assert_eq!((stats.total_bytes, stats.file_count), (100, 1));
|
||||
assert!(cache.get("https://img/oldest").is_none());
|
||||
assert!(cache.get("https://img/newest").is_some());
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn evict_is_a_no_op_at_or_under_budget() {
|
||||
let dir = fresh_dir("noop");
|
||||
let cache = ImageCacheService::open_at(dir.clone()).unwrap();
|
||||
let kept = cache.store("https://img/a", &[0u8; 100]).unwrap();
|
||||
assert_eq!(cache.evict(100, usize::MAX).unwrap(), EvictBatch { freed_bytes: 0, evicted_entries: 0 });
|
||||
assert_eq!(cache.evict(u64::MAX, usize::MAX).unwrap().evicted_entries, 0);
|
||||
assert!(kept.exists());
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn evict_honours_the_batch_size_so_a_caller_can_release_its_lock_between_batches() {
|
||||
let dir = fresh_dir("batch");
|
||||
let cache = ImageCacheService::open_at(dir.clone()).unwrap();
|
||||
for i in 0..3 {
|
||||
cache.store(&format!("https://img/{i}"), &[0u8; 100]).unwrap();
|
||||
cache.backdate(&format!("https://img/{i}"), 1_000 + i);
|
||||
}
|
||||
assert_eq!(cache.evict(0, 1).unwrap(), EvictBatch { freed_bytes: 100, evicted_entries: 1 });
|
||||
assert_eq!(cache.evict(0, 1).unwrap(), EvictBatch { freed_bytes: 100, evicted_entries: 1 });
|
||||
assert_eq!(cache.evict(0, 1).unwrap(), EvictBatch { freed_bytes: 100, evicted_entries: 1 });
|
||||
assert_eq!(cache.evict(0, 1).unwrap(), EvictBatch { freed_bytes: 0, evicted_entries: 0 });
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn get_refreshes_last_accessed_so_a_touched_entry_survives_eviction() {
|
||||
let dir = fresh_dir("touch");
|
||||
let cache = ImageCacheService::open_at(dir.clone()).unwrap();
|
||||
let touched = cache.store("https://img/touched", &[0u8; 100]).unwrap();
|
||||
let untouched = cache.store("https://img/untouched", &[0u8; 100]).unwrap();
|
||||
cache.backdate("https://img/touched", 1_000);
|
||||
cache.backdate("https://img/untouched", 2_000);
|
||||
// A read is an access: it moves `touched` to the newest slot.
|
||||
assert!(cache.get("https://img/touched").is_some());
|
||||
assert_eq!(cache.evict(100, usize::MAX).unwrap().freed_bytes, 100);
|
||||
assert!(touched.exists());
|
||||
assert!(!untouched.exists());
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
}
|
||||
@@ -61,5 +61,5 @@ mod image_cache;
|
||||
mod playback_cache;
|
||||
|
||||
pub use audio_cache::{AudioCache, CacheStats, CachedTrack, CacheAdmission};
|
||||
pub use image_cache::{ImageCacheService, ImageCacheStats};
|
||||
pub use image_cache::{EvictBatch, ImageCacheService, ImageCacheStats};
|
||||
pub use playback_cache::{PlaybackCache, PlaybackCacheStats};
|
||||
@@ -46,14 +46,18 @@
|
||||
//! the documented follow-up. No RGBA decode in Rust: QML `Image` decodes
|
||||
//! `file://` asynchronously and natively.
|
||||
//!
|
||||
//! ## Cache eviction — scaled derivatives only
|
||||
//! ## Cache eviction — both directories, once per run
|
||||
//!
|
||||
//! [`housekeeping`] bounds `~/.cache/qbz/images/scaled` (this module's OWN
|
||||
//! derivative directory) at [`MAX_SCALED_BYTES`], LRU by mtime, and does the
|
||||
//! one-time `.jpg` orphan sweep the `.png` key change left behind. The PARENT
|
||||
//! `~/.cache/qbz/images` is the SHARED cache and keeps its own policy
|
||||
//! (`qbz_cache::ImageCacheService`, SQLite `last_accessed` + the Slint app's
|
||||
//! 200 MB `evict`) — do NOT add a second one for it from here.
|
||||
//! [`housekeeping`] runs on the blocking pool at boot and bounds BOTH caches:
|
||||
//! the SHARED `~/.cache/qbz/images` (the full-size originals `store` writes;
|
||||
//! `qbz_cache::ImageCacheService`, SQLite `last_accessed` LRU, budget
|
||||
//! [`MAX_SHARED_BYTES`], trimmed in [`EVICT_BATCH_ENTRIES`]-row batches so the
|
||||
//! GUI thread is never parked behind a long unlink loop) and this module's
|
||||
//! OWN derivative directory `images/scaled` ([`MAX_SCALED_BYTES`], LRU by
|
||||
//! mtime, plus the one-time `.jpg` orphan sweep). The shared trim was the
|
||||
//! Slint app's `spawn_evict` (v2.0.2); it was retired with that frontend
|
||||
//! (69ed61207) and 2.1.0/2.1.1 shipped with NO caller of `evict` at all — the
|
||||
//! directory grew without bound.
|
||||
|
||||
use std::collections::{HashMap, HashSet};
|
||||
use std::path::{Path, PathBuf};
|
||||
@@ -1229,6 +1233,38 @@ pub fn scaled_path(path: &str, w: u32, h: u32) -> Option<std::path::PathBuf> {
|
||||
/// order of magnitude under the 260 MB the parent image cache already holds.
|
||||
const MAX_SCALED_BYTES: u64 = 64 * 1024 * 1024;
|
||||
|
||||
/// Byte budget for the SHARED cache (`~/.cache/qbz/images`): the Slint app's
|
||||
/// `MAX_CACHE_BYTES` ("matches the Tauri default"), restored unchanged.
|
||||
const MAX_SHARED_BYTES: u64 = 200 * 1024 * 1024;
|
||||
|
||||
/// Rows deleted per `evict` call; the cache mutex is released between calls.
|
||||
const EVICT_BATCH_ENTRIES: usize = 64;
|
||||
|
||||
/// Trim the shared cache to `max_bytes`, LRU by `last_accessed`, in batches.
|
||||
/// Blocking (SQLite + unlinks) — call from `spawn_blocking`. Returns the
|
||||
/// bytes freed, `None` when the cache could not be opened (already logged).
|
||||
fn evict_shared(max_bytes: u64) -> Option<u64> {
|
||||
let cache = cache()?;
|
||||
let mut freed_total = 0u64;
|
||||
loop {
|
||||
let batch = {
|
||||
let guard = cache.lock().ok()?;
|
||||
match guard.evict(max_bytes, EVICT_BATCH_ENTRIES) {
|
||||
Ok(batch) => batch,
|
||||
Err(e) => {
|
||||
log::warn!("[qbz-qt] image cache eviction failed: {e}");
|
||||
return Some(freed_total);
|
||||
}
|
||||
}
|
||||
};
|
||||
freed_total += batch.freed_bytes;
|
||||
if batch.evicted_entries == 0 {
|
||||
return Some(freed_total);
|
||||
}
|
||||
std::thread::yield_now();
|
||||
}
|
||||
}
|
||||
|
||||
/// Keep `scaled/` bounded: drop the oldest derivatives past
|
||||
/// [`MAX_SCALED_BYTES`]. Mtime order, oldest first — the same shape as
|
||||
/// `icon_tint_qt::prune`, with one deliberate difference: `prune` caps on a
|
||||
@@ -1302,16 +1338,18 @@ fn sweep_orphan_scaled(dir: &Path) -> (usize, u64) {
|
||||
(n, freed)
|
||||
}
|
||||
|
||||
/// Boot-time housekeeping for the derivative cache: the one-time `.jpg`
|
||||
/// orphan sweep, then the byte cap. Blocking (`read_dir` + unlinks) — call
|
||||
/// from `spawn_blocking`.
|
||||
///
|
||||
/// SCOPE: `images/scaled` ONLY. The parent `~/.cache/qbz/images` (260 MB) is
|
||||
/// the SHARED cache owned by `qbz_cache::ImageCacheService` (SQLite
|
||||
/// `last_accessed` + the Slint app's 200 MB `evict`); two processes evicting
|
||||
/// the same directory on different policies is exactly what the memo at the
|
||||
/// top of this file had to be written about.
|
||||
/// Boot-time housekeeping for both image caches — see the module doc
|
||||
/// "Cache eviction". Blocking; call from `spawn_blocking`.
|
||||
pub fn housekeeping() {
|
||||
// 1. The SHARED cache (full-size originals): LRU trim to the budget.
|
||||
if let Some(freed) = evict_shared(MAX_SHARED_BYTES) {
|
||||
if freed > 0 {
|
||||
log::info!(
|
||||
"[qbz-qt][perf] image cache evicted {freed} B (budget {MAX_SHARED_BYTES} B)"
|
||||
);
|
||||
}
|
||||
}
|
||||
// 2. This module's OWN derivative directory: orphan sweep, then byte cap.
|
||||
let Some(dir) = dirs::cache_dir().map(|d| d.join("qbz").join("images").join("scaled")) else {
|
||||
return;
|
||||
};
|
||||
|
||||
@@ -721,11 +721,10 @@ pub(crate) fn on_boot() {
|
||||
});
|
||||
offline_fwd::start_ui_forwarder();
|
||||
|
||||
// Derivative-cache housekeeping, off the Qt thread. Once per run: the
|
||||
// `.jpg` orphan sweep is idempotent (FIX 1 moved the scaled derivatives to
|
||||
// `.png`, so every `.jpg` left in `images/scaled/` is dead weight from a
|
||||
// pre-fix build) and the byte cap is cheap — one `read_dir`, then unlink
|
||||
// the oldest until the directory is back under the ceiling.
|
||||
// Image-cache housekeeping, off the Qt thread, once per run: the shared
|
||||
// `~/.cache/qbz/images` LRU trim (200 MB, batched under the cache mutex)
|
||||
// and the `images/scaled` orphan sweep + byte cap. See artwork_qt.rs
|
||||
// "Cache eviction".
|
||||
spawn(async {
|
||||
let _ = tokio::task::spawn_blocking(artwork_qt::housekeeping).await;
|
||||
});
|
||||
|
||||
Reference in new issue
Block a user