diff --git a/python-backend/musicmouse/library/cache.py b/python-backend/musicmouse/library/cache.py index dae85ad..d7ea9c4 100644 --- a/python-backend/musicmouse/library/cache.py +++ b/python-backend/musicmouse/library/cache.py @@ -47,10 +47,15 @@ __all__ = ["Fingerprint", "LibraryCache"] #: files are untouched, so otherwise a change to that logic is invisible until somebody #: edits their music folder. # 5: every album's `cover` now points into this cache, downscaled, including the ones -# that used to point straight at a `cover.jpg` in the library folder. Bumping this is -# what re-points them: an index from version 4 is discarded and the next scan rebuilds -# it, which is cheap by design - see the module docstring. -_INDEX_VERSION = 5 +# that used to point straight at a `cover.jpg` in the library folder. +# 6: MAX_COVER_PX dropped from 640 to 384. This is what applies it. `shrink_cover` never +# scales art *up*, so a cover already written at some size is only ever rewritten +# smaller - which means a cache holding 640 px files cannot be brought to 384 px by +# re-reading them, only by going back to the original art. Discarding the index does +# exactly that: every album is scanned again and `store_cover` runs on the real art. +# +# Bumping this is cheap by design - see the module docstring. +_INDEX_VERSION = 6 @dataclass(frozen=True, slots=True) @@ -266,9 +271,19 @@ def _album_from_json(data: dict[str, Any]) -> Album: #: cover it painted, to show it in a 185 px card on a Pi with 2 GB of RAM. #: #: 340 is the largest any of this app's screens asks for (the play view; the browse grid -#: asks for 180 and the player bar for 56), so 640 still leaves room for a tablet at -#: devicePixelRatio 2 and cuts the decode by about twenty times. -MAX_COVER_PX: Final = 640 +#: asks for 180 and the player bar for 56), so this only has to beat 340 to be lossless +#: where it shows. +#: +#: It was 640 first, for headroom on a tablet at devicePixelRatio 2, and that headroom +#: turned out to be expensive on the device that actually runs this. Measured on +#: musicdolphin, typing "conni" over a 343-album library, keydown to painted, worst +#: keystroke of three runs: 3.4-9.1 s at 640 px against 0.2 s at 384 px. Decode cost +#: goes with pixel count, and a browse grid paints a card 132 px wide. +#: +#: A tablet at devicePixelRatio 2 therefore gets a slightly soft cover on the play view +#: and nowhere else. That is the trade: a visible sharpness margin nobody asked for, for +#: a search box that answers a keystroke in a frame. +MAX_COVER_PX: Final = 384 #: Re-encode quality. At these dimensions the difference from 95 is invisible and the #: file is a third of the size. diff --git a/web/public/sw.js b/web/public/sw.js index 09a69d8..22a4f77 100644 --- a/web/public/sw.js +++ b/web/public/sw.js @@ -4,18 +4,9 @@ // cached answer for any of those would be a lie. const SHELL = "musikdelphin-shell-v1"; -// v2: the backend now downscales cover art to a 640 px long edge on its way into the -// cache (MAX_COVER_PX in library/cache.py). The URL of a cover does not change when its -// contents do, and the handler below is cache-first, so every client that had ever -// loaded a cover went on serving the old 3000 px one from disk - a 9-megapixel decode -// per card, which a trace on musicdolphin showed costing ~1.5 s each and dwarfing -// everything else the page did. Renaming the cache is what retires them: `activate` -// deletes every cache that is not one of these two. -// -// So: change how covers are produced, bump this name. "Immutable per album id" is true -// of which album a cover belongs to, not of the bytes. -const COVERS = "musikdelphin-covers-v2"; +// There is no cover cache any more; `activate` below deletes every cache that is not +// SHELL, which retires the two generations of cover cache this worker used to keep. self.addEventListener("install", (event) => { event.waitUntil( caches @@ -39,7 +30,7 @@ self.addEventListener("activate", (event) => { caches .keys() .then((names) => - Promise.all(names.filter((n) => n !== SHELL && n !== COVERS).map((n) => caches.delete(n))), + Promise.all(names.filter((n) => n !== SHELL).map((n) => caches.delete(n))), ) .then(() => self.clients.claim()), ); @@ -52,31 +43,22 @@ self.addEventListener("fetch", (event) => { const url = new URL(request.url); if (url.origin !== self.location.origin) return; - // Cover art: stale-while-revalidate. The cached copy is served straight away, which - // is what keeps a grid of album art off the network entirely, and a fresh copy is - // fetched behind the page and put back for next time. + // Cover art is NOT cached here, and that is deliberate. This worker only ever + // registers on localhost or over HTTPS (see the README), which on this setup means + // the kiosk on the device itself - where the backend is the same machine, and a + // Cache Storage lookup plus a revalidating fetch plus a cache write is strictly more + // work than just asking for the file. // - // It was plain cache-first, which is subtly wrong in a way that cost a long afternoon: - // the bytes behind a cover URL do change (the backend reprocesses art), and - // cache-first on a URL that never changes means a client can serve a stale cover for - // ever. Revalidating in the background is the cheap way to be both fast and eventually - // right, and it needs no version bump when art is reprocessed. - if (url.pathname.startsWith("/api/albums/")) { - event.respondWith( - caches.open(COVERS).then(async (cache) => { - const hit = await cache.match(request); - const fetching = fetch(request) - .then((response) => { - if (response.ok) cache.put(request, response.clone()); - return response; - }) - // Offline, or the mouse is off: a cached cover is still better than none. - .catch(() => hit); - return hit ?? fetching; - }), - ); - return; - } + // It was measured, after a stale-while-revalidate version of this handler made + // searching visibly worse. Typing "conni" over a 343-album library, keydown to + // painted, worst keystroke: 5580 ms through this worker against 461 ms with + // Network.setBypassServiceWorker on. The main thread was not running any JavaScript + // during those stalls - it sat waiting while ImageDecodeTask blocked on bytes this + // worker was serialising through a single thread. + // + // If the player ever moves behind a TLS proxy for the tablet, revisit: caching covers + // is worth something over wifi, and would want a budget rather than every cover on + // every load. // Everything else under /api is live state. Never cache it, never serve it stale. if (url.pathname.startsWith("/api/")) return;