From 83d16e00581bf5d7111707a5876b52a031f090dd Mon Sep 17 00:00:00 2001 From: Martin Bauer Date: Sun, 20 Sep 2026 18:31:57 +0200 Subject: [PATCH] Take the service worker off the cover path, and cut cover art to 384px Search was still stalling for seconds a keystroke. A trace across five keystrokes said the renderer main thread was blocked for 9.4 s in a single task with *zero* V8 samples inside it - no JavaScript ran at all. What ran instead, on the worker pool during those same 9.4 s: ImageDecodeTask 7.6 s, RasterTask 1.7 s, and only 229 ms of actual "Decode Image". The main thread was waiting on cover bytes, not computing anything. Two causes, and the first one was mine. The stale-while-revalidate handler added earlier today made every cover do a Cache Storage read *plus* a network fetch *plus* a cache write, all serialised through one worker thread. Measured with Network.setBypassServiceWorker: worst keystroke 5580 ms through the worker against 461 ms without it. So covers no longer go through the worker at all. That costs nothing here: this worker only registers on localhost or over HTTPS, which on this setup is the kiosk on the device itself, where the backend is the same machine and a cache lookup is strictly more work than asking for the file. The shell caching, which is what makes it installable, stays. The second is that decode cost goes with pixel count, and 640 px was headroom for a tablet at devicePixelRatio 2 that nobody had asked for. A browse grid paints a card 132 px wide; the largest any screen asks for is 340. At 384 px, typing "conni" over 343 albums, keydown to painted, three runs: before 3735, 270, 5580, 88, 57 ms after 873, 75, 17, 24, 26 ms `shrink_cover` never scales art up, so dropping the limit cannot be applied by re-reading the cache - only by going back to the original art. _INDEX_VERSION 6 does that: the index is discarded and every album is scanned again through store_cover. Co-Authored-By: Claude Opus 5 --- python-backend/musicmouse/library/cache.py | 29 +++++++++--- web/public/sw.js | 54 ++++++++-------------- 2 files changed, 40 insertions(+), 43 deletions(-) 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;