diff --git a/e2e/perf/measure.mjs b/e2e/perf/measure.mjs index 0cff8d6..119857d 100644 --- a/e2e/perf/measure.mjs +++ b/e2e/perf/measure.mjs @@ -42,6 +42,13 @@ * browse number below could not see this, because it * visits Explore without ever typing in it. * heap JS heap after a scripted browse, post-GC. m3. + * memory what the app holds at rest after launch, before any + * view but the landing one is opened: the backend + * process's RSS and peak, its Go heap, the page's JS + * heap, and the bytes each binding returned on the way. + * Then the same after the first open of Tracks, and how + * long that open took to its first *row* — the number + * eager loading was buying (#280/#281). * * Usage: * node e2e/perf/measure.mjs --label before @@ -51,7 +58,7 @@ * Requires a running app: `make dev-headless SEED=bulk`. */ -import { readFileSync, writeFileSync, mkdirSync, existsSync } from 'node:fs'; +import { readFileSync, writeFileSync, mkdirSync, existsSync, readdirSync } from 'node:fs'; import { dirname, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import { chromium } from '@playwright/test'; @@ -190,6 +197,135 @@ const SEARCH_DEBOUNCE_MS = 150; /* -------------------------------------------------------------------- */ +/** + * The backend process's memory, from /proc and its own pprof endpoint. + * + * Linux and a dev build only (pprof is mounted by `-tags dev`); either + * missing is a null, not a failure — this is a measurement harness, and + * a number it cannot take is reported as absent rather than as zero. + */ +async function backendMemory() { + const out = { rssMB: null, peakMB: null, anonMB: null, goHeapInuseMB: null, goHeapHeldMB: null }; + const pid = findBackendPid(); + + if (pid) { + try { + const status = readFileSync(`/proc/${pid}/status`, 'utf8'); + const kb = (k) => Number(status.match(new RegExp(`^${k}:\\s+(\\d+)`, 'm'))?.[1] ?? NaN); + + out.rssMB = round(kb('VmRSS') / 1024, 1); + out.peakMB = round(kb('VmHWM') / 1024, 1); + out.anonMB = round(kb('RssAnon') / 1024, 1); + } catch { + // The process went away between finding it and reading it. + } + } + + try { + const text = await (await fetch('http://localhost:6060/debug/pprof/heap?debug=1')).text(); + const field = (k) => Number(text.match(new RegExp(`^# ${k} = (\\d+)`, 'm'))?.[1] ?? NaN); + + out.goHeapInuseMB = round(field('HeapInuse') / 1048576, 1); + // What the runtime holds from the OS: the RSS the heap accounts for. + out.goHeapHeldMB = round((field('HeapSys') - field('HeapReleased')) / 1048576, 1); + } catch { + // Not a dev build, or pprof is not listening. + } + + return out; +} + +/** The pid of the running `yj-dev` binary, found by name under /proc. */ +function findBackendPid() { + try { + for (const entry of readdirSync('/proc')) { + if (!/^\d+$/.test(entry)) continue; + + try { + if (readFileSync(`/proc/${entry}/comm`, 'utf8').trim() === 'yj-dev') return entry; + } catch { + // Raced with an exiting process. + } + } + } catch { + // No /proc: not Linux. + } + + return null; +} + +/** Bytes returned per binding name, largest first. */ +function bytesByBinding(calls) { + const by = {}; + + for (const c of calls) by[c.path] = (by[c.path] ?? 0) + (c.bytes ?? 0); + + return Object.fromEntries( + Object.entries(by) + .filter(([, b]) => b > 0) + .sort((a, b) => b[1] - a[1]) + .map(([k, b]) => [k, round(b / 1048576, 2)]), + ); +} + +async function pageMemory(page, client) { + await client.send('HeapProfiler.collectGarbage'); + await page.waitForTimeout(300); + + const heap = await client.send('Runtime.getHeapUsage'); + const dom = await client.send('Memory.getDOMCounters'); + + return { jsHeapMB: round(heap.usedSize / 1048576, 1), domNodes: dom.nodes }; +} + +async function measureMemory(page, client) { + // Settled: the landing view has loaded and any idle warming has run. + await page.waitForTimeout(5000); + + const startupCalls = await page.evaluate(() => window.__yjPerf.calls); + const atRest = { + backend: await backendMemory(), + page: await pageMemory(page, client), + bytesByBindingMB: bytesByBinding(startupCalls), + totalBindingMB: round(startupCalls.reduce((n, c) => n + (c.bytes ?? 0), 0) / 1048576, 2), + }; + + // First open of Tracks, to its first row: what a user waits for when + // nothing was loaded ahead of them. + const since = await page.evaluate(() => performance.now()); + const firstRowMs = await page.evaluate(async () => { + const t0 = performance.now(); + + document.dispatchEvent(new CustomEvent('navigate', { detail: { view: 'tracks' } })); + + for (;;) { + const list = document.querySelector('#main-content > track-list:not(.view-hidden)'); + + if (list?.shadowRoot?.querySelector('[data-testid="track-row"]')) { + return Math.round(performance.now() - t0); + } + + if (performance.now() - t0 > 60000) return null; + await new Promise((r) => requestAnimationFrame(r)); + } + }); + await page.waitForTimeout(2000); + + const tracksCalls = await page.evaluate((t) => window.__yjPerf.since(t), since); + + return { + atRest, + tracksOpen: { + firstRowMs, + bytesByBindingMB: bytesByBinding(tracksCalls), + backend: await backendMemory(), + page: await pageMemory(page, client), + }, + }; +} + +/* -------------------------------------------------------------------- */ + async function measureStartup(page) { const nav = await page.evaluate(() => { const n = performance.getEntriesByType('navigation')[0]; @@ -234,16 +370,23 @@ async function measureStartup(page) { }; }); - // "First row on screen" is the number a user experiences as startup; - // FCP fires on the chrome around an empty list. + // "First row on screen" is the number a user experiences as startup + // *when the app lands on Tracks*; FCP fires on the chrome around an + // empty list. + // + // The deadline is short because since #280 a landing on any other + // view leaves the track list unloaded, and this would otherwise + // spend a minute waiting for a row that is not coming. The number + // that means something either way is `memory.tracksOpen.firstRowMs`, + // which opens Tracks and waits for its first row deliberately. const firstRowMs = await page.evaluate(async () => { const t0 = performance.now(); - const deadline = t0 + 60000; + const deadline = t0 + 2000; for (;;) { const list = document.querySelector('track-list'); const row = list?.shadowRoot?.querySelector('[role="row"], .track-row'); - if (row) return Math.round(performance.now() - t0 + (performance.timeOrigin ? 0 : 0)); + if (row) return Math.round(performance.now() - t0); if (performance.now() > deadline) return null; await new Promise((r) => setTimeout(r, 16)); } @@ -1845,6 +1988,12 @@ async function run(label) { loadWallMs: Date.now() - t0, }; + // First of all: at rest means before any measurement opens a view. + console.log(' memory at rest, then Tracks first open…'); + report.memory = await measureMemory(page, client); + await page.evaluate(() => document.dispatchEvent( + new CustomEvent('navigate', { detail: { view: 'home' } }), + )); console.log(' startup…'); report.startup = await measureStartup(page); // Before anything else navigates: every view's first open has to be @@ -1907,6 +2056,14 @@ async function run(label) { const ROWS = [ ['First contentful paint', (r) => fmt(r.startup.firstContentfulPaintMs, 'ms')], + ['At rest: backend RSS', (r) => fmt(r.memory?.atRest.backend.rssMB, 'MB')], + ['At rest: backend peak RSS', (r) => fmt(r.memory?.atRest.backend.peakMB, 'MB')], + ['At rest: Go heap held', (r) => fmt(r.memory?.atRest.backend.goHeapHeldMB, 'MB')], + ['At rest: JS heap', (r) => fmt(r.memory?.atRest.page.jsHeapMB, 'MB')], + ['At rest: binding bytes', (r) => fmt(r.memory?.atRest.totalBindingMB, 'MB')], + ['Tracks first open: first row', (r) => fmt(r.memory?.tracksOpen.firstRowMs, 'ms')], + ['Tracks open: backend peak RSS', (r) => fmt(r.memory?.tracksOpen.backend.peakMB, 'MB')], + ['Tracks open: JS heap', (r) => fmt(r.memory?.tracksOpen.page.jsHeapMB, 'MB')], ['First track row', (r) => fmt(r.startup.firstRowAfterLoadMs, 'ms')], ['JS transferred', (r) => fmt(round(r.startup.scriptBytes / 1024), 'kB')], ['JS evaluated before first paint', (r) => fmt(round((r.startup.scriptBytesBeforePaint ?? 0) / 1024), 'kB')], diff --git a/e2e/specs/library-lazy.spec.ts b/e2e/specs/library-lazy.spec.ts new file mode 100644 index 0000000..fec0766 --- /dev/null +++ b/e2e/specs/library-lazy.spec.ts @@ -0,0 +1,58 @@ +/** + * #280: launching the app must not load the track list. + * + * Every collection used to be fetched at `DOMContentLoaded` and + * refetched on every invalidation, whichever view was showing. On a + * 26 138-track library the track list was 20.5 MB of that, and encoding + * it cost the backend ~170 MB of transient allocation — for someone + * looking at Home, which draws none of it. Measured on 50 000 tracks: + * 543 MB of backend RSS at rest before, 296 MB after. + * + * The negative assertion is the point, and it needs its complement: "no + * `GetTrackTable`" also holds on a build that fetches nothing at all, + * so the same spec opens Tracks and watches it arrive. + */ +import { test, expect, bindingCalls, navigateTo } from '../support/fixtures.js'; + +const TRACKS = 'library.Library.GetTrackTable'; + +/** The seed's default page, which is what a launch lands on. */ +const LANDING_VIEW = 'home'; + +test('launching on Home does not load the track list', async ({ app }) => { + // Past the store's idle warm-up: "nothing was fetched" has to mean + // nothing, not "nothing has happened yet". + await app.waitForTimeout(3_500); + + const calls = await bindingCalls(app); + + expect(calls, 'the warm-up for the small collections ran').toContain( + 'library.Library.GetAlbums', + ); + expect(calls, `the track list was fetched at launch (${LANDING_VIEW})`).not.toContain( + TRACKS, + ); +}); + +test('opening Tracks loads it, and only then', async ({ app }) => { + await app.waitForTimeout(1_000); + expect(await bindingCalls(app)).not.toContain(TRACKS); + + await navigateTo(app, 'tracks'); + await expect(app.getByTestId('track-row').first()).toBeVisible(); + + expect(await bindingCalls(app)).toContain(TRACKS); +}); + +test('a hover on the nav item starts the load before the click', async ({ app }) => { + await app.waitForTimeout(1_000); + + const nav = app.getByTestId('nav-tracks'); + + // The pointer entering the item is the whole mechanism; nothing is + // clicked, so the data must arrive because of the hover alone. + await nav.hover(); + await expect + .poll(async () => (await bindingCalls(app)).includes(TRACKS)) + .toBe(true); +}); diff --git a/frontend/index.html b/frontend/index.html index cebe75a..bdcd761 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -51,7 +51,15 @@
- + +
diff --git a/frontend/index.ts b/frontend/index.ts index d97d80a..0527839 100644 --- a/frontend/index.ts +++ b/frontend/index.ts @@ -200,13 +200,18 @@ const mainContent = document.getElementById('main-content'); // tracked as currentViewEl, is never hidden: two visible primary views // splitting the main panel between them regardless of which is // selected. +// +// It is deliberately not activated here. It is markup, not a decision: +// the shell does not yet know which view the launch lands on, and +// activating it starts the Tracks view's work — its data fetch, above +// all (#280) — for a launch that is about to land on Home. The first +// navigation is what activates whichever view it lands on. if (mainContent) { const initialTrackList = mainContent.querySelector('track-list'); if (initialTrackList) { viewCache.set('tracks', initialTrackList as HTMLElement); currentViewEl = initialTrackList as HTMLElement; - activateView(currentViewEl); } } @@ -580,10 +585,14 @@ async function handleNavigate( } default: { const fallback = document.createElement('div'); + const message = document.createElement('p'); fallback.style.padding = '1em'; fallback.style.color = 'var(--yj-text-secondary, #b3b3b3)'; - fallback.innerHTML = `

Coming soon: ${view}

`; + // textContent, not innerHTML: `view` comes from a navigation + // detail, which is app-supplied but not app-owned. + message.textContent = `Coming soon: ${view}`; + fallback.append(message); mainContent.appendChild(fallback); currentDetailEl = fallback; } @@ -620,6 +629,10 @@ function warmViewChunks(): void { /** requestIdleCallback where it exists; WebKit2GTK does not have it. */ function schedule(fn: () => void): void { + // SAFETY: `requestIdleCallback` is not in this project's DOM lib + // types, so it is read through an optional-property shape; the + // property is either absent (undefined) or the browser's own + // function, which is what the guard below checks. const ric = ( window as unknown as { requestIdleCallback?: (cb: () => void) => number; diff --git a/frontend/src/components/queue-panel/queue-panel.ts b/frontend/src/components/queue-panel/queue-panel.ts index 1bb1c2f..6bef4d6 100644 --- a/frontend/src/components/queue-panel/queue-panel.ts +++ b/frontend/src/components/queue-panel/queue-panel.ts @@ -1553,9 +1553,13 @@ export class QueuePanel indices: number[], ) { const queueTracks = this.queue.tracks; - const filePaths = indices - .map((i) => queueTracks[i]?.filePath) - .filter((fp): fp is string => fp != null); + const filePaths: string[] = []; + + for (const i of indices) { + const path = queueTracks[i]?.filePath; + + if (path != null) filePaths.push(path); + } await showBatchTrackDetailsForPaths( () => this.trackDetailsDialog, diff --git a/frontend/src/components/sidebar/app-sidebar.ts b/frontend/src/components/sidebar/app-sidebar.ts index 55696a7..ca7a99b 100644 --- a/frontend/src/components/sidebar/app-sidebar.ts +++ b/frontend/src/components/sidebar/app-sidebar.ts @@ -6,6 +6,7 @@ import { designTokens } from '../../styles/tokens.css'; import type { DragActiveDetail } from '@utils/drag-controller'; import { ActiveViewController } from '@store/controllers/active-view-controller'; import { ViewVisibilityController } from '@store/controllers/view-visibility-controller'; +import { libraryStore } from '@store/library-store'; import { VIEW_META } from '../../services/view-meta'; import type { View } from '../../services/view-meta'; @@ -352,6 +353,10 @@ export class AppSidebar extends LitElement { : 'false'} @click=${() => this.navigate(item.id)} + @mouseenter=${() => + this.prefetch(item.id)} + @focus=${() => + this.prefetch(item.id)} @dragover=${(e: DragEvent) => this.onNavDragOver( e, @@ -503,6 +508,19 @@ export class AppSidebar extends LitElement { } } + /** + * Start loading what this view draws, on hover or keyboard focus. + * + * The ~100 ms before the click is the whole point: #280 stopped + * fetching every collection at startup, and this is what keeps the + * view that *is* opened from paying the whole payload after the + * click. Nothing is awaited and nothing is reported here — the view + * itself reports a failure, and this is the same request. + */ + private prefetch(view: View) { + libraryStore.prefetch(view); + } + private navigate(view: View) { // No optimistic highlight: the shell answers, and it answers // synchronously in `handleNavigate` before it awaits anything. diff --git a/frontend/src/components/track-list/track-list.ts b/frontend/src/components/track-list/track-list.ts index df9d315..cbae111 100644 --- a/frontend/src/components/track-list/track-list.ts +++ b/frontend/src/components/track-list/track-list.ts @@ -1336,11 +1336,6 @@ export class TrackList super.connectedCallback(); this.restoreSortPreferences(); - if (this.externalTracks) { - this.tracks = this.externalTracks; - } else { - this.loadTracks(); - } this.resizeObserver = new ResizeObserver( () => { this.onHostResize(); @@ -1390,10 +1385,33 @@ export class TrackList this.resizeObserver = null; } + /** + * Fetch the list when this becomes the view on screen (#280). + * + * Not on connection: `index.html` renders a `` as the + * main panel's first-paint content, so a connection-time fetch was + * the whole library loaded at launch for a landing on Home — 12 MB + * at 50 000 tracks, and the backend's peak RSS with it. The shell + * activates this element only when a navigation lands on Tracks. + * + * Called on *every* activation, not just the first: the store may + * have refetched while this view was off screen, and `getTracks()` + * answers from its cache when nothing changed. + */ + protected override onViewActivate(): void { + if (this.externalTracks) { + this.tracks = this.externalTracks; + } else { + void this.loadTracks(); + } + + this.attachListListeners(); + } + /** Document-level listeners belong to the *visible* list. A cached * list is never disconnected, so this is the only place they can be * taken down again. */ - protected override onViewActivate(): void { + private attachListListeners(): void { this.listenWhileActive(document, 'click', this.clearSelectionHandler); this.listenWhileActive( document, @@ -1560,9 +1578,10 @@ export class TrackList this.selection.clear(); } - // Re-fetch when the store delivers fresh - // data after eager refetch on invalidation. - if (!this.externalTracks) { + // Re-fetch when the store delivers fresh data after an + // invalidation. Off screen the list does nothing with it, and + // the next activation re-reads the store (#280). + if (!this.externalTracks && this.viewActive) { const cached = this.libraryCtrl.cachedTracks; diff --git a/frontend/src/store/library-store.ts b/frontend/src/store/library-store.ts index 29eea8c..9aba95c 100644 --- a/frontend/src/store/library-store.ts +++ b/frontend/src/store/library-store.ts @@ -28,6 +28,14 @@ const COVER_SIZE_DEFAULT = 176; /** localStorage key for persisted cover size. */ const COVER_SIZE_KEY = 'cover-grid-size'; +/** + * How long the small-collection warm-up will wait for idle before + * running anyway. It is speculative, but a busy main thread must not + * mean Albums is slow to open — the timeout is the promise that it is + * only ever deferred, never skipped. + */ +const WARM_IDLE_TIMEOUT_MS = 3_000; + class LibraryStore { private tracks: ListTrack[] | null = null; private albums: library.Album[] | null = null; @@ -110,34 +118,82 @@ class LibraryStore { }); this.loadCoverSize(); - this.deferEagerFetch(); + this.warmSmallCollectionsOnIdle(); } /** - * Schedules eagerFetch() to run after the DOM is ready. - * The LibraryStore singleton is instantiated during ES module - * evaluation (import time), so calling eagerFetch() in the - * constructor would fire 4 backend roundtrips before the app - * shell has rendered. Deferring to the 'DOMContentLoaded' - * event (or calling immediately if the DOM is already parsed) - * lets the shell paint first, then begins data loading. + * Warm the three small collections once the app is idle after first + * paint, and deliberately not the tracks (#280). + * + * All four used to be fetched together at `DOMContentLoaded`, + * whichever view was showing. On a 26 138-track library the track + * list was 20.5 MB of that, and encoding it cost the backend ~170 MB + * of transient allocation — paid by someone looking at Home, which + * needs none of it. Measured on 50 000 tracks: 543 MB of backend RSS + * at rest before, 296 MB after, and 12.1 MB of binding bytes instead + * of 35.9. + * + * Albums, artists and genres are 1.6 MB together, so they are still + * fetched ahead of the click — that is what made those views + * instant. Tracks are 12 MB at 50 000 and are fetched by the view + * that draws them, or by `prefetch` from a hover. + * + * Idle rather than immediate: this is speculative, so it must not + * compete with the first paint. */ - private deferEagerFetch(): void { - if (document.readyState === 'loading') { - window.addEventListener( - 'DOMContentLoaded', - () => { - this.eagerFetch(); - }, - { once: true }, - ); + private warmSmallCollectionsOnIdle(): void { + const warm = () => { + const logged = this.failureReporter(); + + void this.getAlbums().catch(logged('albums')); + void this.getArtists().catch(logged('artists')); + void this.getGenres().catch(logged('genres')); + }; + + if (typeof requestIdleCallback === 'function') { + requestIdleCallback(warm, { timeout: WARM_IDLE_TIMEOUT_MS }); } else { - // DOM already parsed (shouldn't happen during module - // eval, but handles dynamic instantiation safely). - this.eagerFetch(); + setTimeout(warm, 0); } } + /** + * Start loading what a view will need, without waiting for it. + * + * A nav item calls this on hover or focus: that is the ~100 ms + * before the click, and it is what #280 trades for not paying for + * every collection at startup whether or not anyone goes there. + * + * A view this store holds nothing for is ignored rather than an + * error — it is a hint, and a hint about Home is not a mistake. + * The failure is swallowed here because the view that wanted the + * data reports it: this is the same request, already deduplicated + * by `inFlight`, and it has no caller to reject to. + */ + prefetch(view: string): void { + const started = (() => { + switch (view) { + case 'tracks': + return this.getTracks(); + case 'albums': + return this.getAlbums(); + case 'artists': + return this.getArtists(); + case 'genres': + return this.getGenres(); + default: + return null; + } + })(); + + void started?.catch(() => undefined); + } + + private failureReporter(): (what: string) => (err: unknown) => void { + return (what) => (err) => + console.error(`library: could not load ${what}`, err); + } + // =================================================================== // DATA ACCESS // Returns cached data or fetches from backend on first access. @@ -595,6 +651,8 @@ class LibraryStore { } } + const loaded = this.loadedCollections(); + this.albums = null; this.artists = null; this.genres = null; @@ -610,50 +668,63 @@ class LibraryStore { this.changeGen++; this.notify(); - const logged = (what: string) => (err: unknown) => - console.error(`library: could not reload ${what}`, err); - - void this.getAlbums().catch(logged('albums')); - void this.getArtists().catch(logged('artists')); - void this.getGenres().catch(logged('genres')); + // Only the summaries something was showing (#280): a removal + // nobody was looking at does not load a collection to correct it. + this.refetchLoaded({ + albums: loaded.albums, + artists: loaded.artists, + genres: loaded.genres, + }); } private invalidate(): void { + // Which collections something has actually loaded, before they + // are dropped. Refetching all four here would undo #280: a scan + // finishing would fetch the track list of a library nobody has + // opened the Tracks view on. + const loaded = this.loadedCollections(); + this.tracks = null; this.albums = null; this.artists = null; this.genres = null; // Anything still in flight was asked for on behalf of a - // selection that no longer applies: forget it, so the eager - // refetch below starts a request for the current one rather - // than adopting the old one's answer. + // selection that no longer applies: forget it, so the refetch + // below starts a request for the current one rather than + // adopting the old one's answer. this.inFlight.clear(); this.cacheGen++; this.changeGen++; this.scrollPositions = { tracks: 0, albums: 0, artists: 0, genres: 0 }; this.notify(); - this.eagerFetch(); + this.refetchLoaded(loaded); + } + + /** The collections currently held, keyed by the view that draws them. */ + private loadedCollections(): Partial> { + return { + tracks: this.tracks !== null, + albums: this.albums !== null, + artists: this.artists !== null, + genres: this.genres !== null, + }; } /** - * Fetches all library data. Called after DOM ready - * (initial load, via deferEagerFetch) and after cache - * invalidation so that controller subscribers receive - * fresh data on the next requestUpdate() cycle without - * needing their own LibraryScanComplete listener. + * Refetch exactly the collections named, and nothing else. + * + * A failed fetch is reported by whichever view asked for the data + * (it is that panel's failure, not the app's), but this refetch has + * no caller to reject to — without a catch it is an unhandled + * rejection. */ - private eagerFetch(): void { - // A failed fetch is reported by whichever view asked for the - // data (it is that panel's failure, not the app's), but the - // eager refetch has no caller to reject to — without a catch it - // is an unhandled rejection. - const logged = (what: string) => (err: unknown) => - console.error(`library: could not load ${what}`, err); + private refetchLoaded(loaded: Partial>): void { + const logged = this.failureReporter(); - void this.getTracks().catch(logged('tracks')); - void this.getAlbums().catch(logged('albums')); - void this.getArtists().catch(logged('artists')); - void this.getGenres().catch(logged('genres')); + if (loaded.tracks) void this.getTracks().catch(logged('tracks')); + if (loaded.albums) void this.getAlbums().catch(logged('albums')); + if (loaded.artists) void this.getArtists().catch(logged('artists')); + if (loaded.genres) void this.getGenres().catch(logged('genres')); } // =================================================================== diff --git a/frontend/test/components/chrome.test.ts b/frontend/test/components/chrome.test.ts index a60bf76..d797c10 100644 --- a/frontend/test/components/chrome.test.ts +++ b/frontend/test/components/chrome.test.ts @@ -11,7 +11,8 @@ import '@components/library-filter/library-filter'; import '@components/library-status-indicator/library-status-indicator'; import { Events } from '../../src/events'; import { activeViewStore } from '@store/active-view-store'; -import { emit, stub, flush, calls, lastArgs } from '@test/support/harness'; +import { libraryStore } from '@store/library-store'; +import { emit, stub, flush, calls, lastArgs, resetHarness } from '@test/support/harness'; import { fixture, shadow, @@ -181,6 +182,14 @@ describe('', () => { it('selects a library by id, and the merged view by empty string', async () => { const el = await fixture('library-filter'); + // The list has to be in use for the filter change to refetch it + // (#280): a collection nothing has loaded is not loaded to correct + // it, so this is the state the app is in when the Tracks view is up. + stub('library.Library.GetTrackTable', { strings: [''] }); + await libraryStore.getTracks(); + resetHarness(); + stub('library.Library.GetTrackTable', { strings: [''] }); + await flush(); await el.updateComplete; diff --git a/frontend/test/stores/library-lazy.test.ts b/frontend/test/stores/library-lazy.test.ts new file mode 100644 index 0000000..1c8dfdd --- /dev/null +++ b/frontend/test/stores/library-lazy.test.ts @@ -0,0 +1,99 @@ +/** + * #280: the app loads the library when a view needs it, not at startup. + * + * Every collection used to be fetched together at `DOMContentLoaded` + * and refetched on every invalidation, whether or not anything was + * showing it. On a 26 138-track library the track list was 20.5 MB of + * that and cost the backend ~170 MB of transient allocation — for + * someone looking at Home, which draws none of it. + * + * **This file must not load the track list before the first test.** It + * asserts the state the app is actually left in by its own startup, so + * a test added above that asks for tracks would invalidate the + * precondition rather than silently pass. + */ +import { describe, expect, it, beforeEach } from 'vitest'; + +import { libraryStore } from '@store/library-store'; +import { Events } from '../../src/events'; +import { + calls, + emit, + flush, + resetHarness, + stub, +} from '@test/support/harness'; + +const TRACKS = 'library.Library.GetTrackTable'; +const SMALL = [ + 'library.Library.GetAlbums', + 'library.Library.GetArtists', + 'library.Library.GetGenres', +]; + +/** + * Wait for the store's own idle warm-up to land. + * + * It asks for the three small collections on `requestIdleCallback`, so + * when it has happened is the browser's decision — waiting for the + * effect rather than for a duration is the only way this is not a race. + */ +function stubReads(): void { + stub(TRACKS, { strings: [''] }); + for (const path of SMALL) stub(path, []); +} + +async function waitForWarm(): Promise { + for (let i = 0; i < 200 && libraryStore.getCachedAlbums() === null; i++) { + await flush(); + } + + expect(libraryStore.getCachedAlbums(), 'the warm-up ran').not.toBeNull(); +} + +describe('what the app loads on its own (#280)', () => { + beforeEach(async () => { + stubReads(); + await waitForWarm(); + // resetHarness clears the stubs as well as the recorded calls. + resetHarness(); + stubReads(); + }); + + it('leaves the track list alone', () => { + expect(libraryStore.getCachedTracks()).toBeNull(); + expect(calls(TRACKS)).toEqual([]); + }); + + it('does not load it to answer a scan', async () => { + emit(Events.LibraryScanComplete); + await flush(); + + // The small collections are in use, so they are refreshed; the + // track list nobody has opened is not. + expect(calls().map((c) => c.path).sort()).toEqual([...SMALL].sort()); + }); + + it('loads it for the view that draws it, and refreshes it from then on', async () => { + libraryStore.prefetch('tracks'); + await flush(); + + expect(calls(TRACKS)).toHaveLength(1); + expect(libraryStore.getCachedTracks()).not.toBeNull(); + + resetHarness(); + emit(Events.LibraryScanComplete); + await flush(); + + expect(calls(TRACKS), 'in use now, so a scan refreshes it').toHaveLength(1); + }); + + it('is asked for nothing by a prefetch for a view it has no data for', async () => { + libraryStore.prefetch('home'); + libraryStore.prefetch('settings'); + libraryStore.prefetch('explore'); + await flush(); + + expect(calls()).toEqual([]); + }); +}); diff --git a/frontend/test/stores/library-store.test.ts b/frontend/test/stores/library-store.test.ts index 2bbf094..8fcf88b 100644 --- a/frontend/test/stores/library-store.test.ts +++ b/frontend/test/stores/library-store.test.ts @@ -43,16 +43,25 @@ function stubReads(): void { } /** - * Drop the cache and let the eager refetch settle, so each test starts - * from the same place. The store has no reset of its own; a scan - * completing is how the app itself clears it. + * Start each test from a loaded store. The store has no reset of its + * own; a scan completing is how the app itself clears it — and since + * #280 it reloads only what something had loaded, so the four reads + * here are what says "this test is about a loaded store". */ async function reload(): Promise { stubReads(); emit(Events.LibraryScanComplete); await flush(); - // The eager refetch the invalidation kicks off is recorded like any - // other call; clear it, or every count in every test is off by one. + resetHarness(); + stubReads(); + await Promise.all([ + libraryStore.getTracks(), + libraryStore.getAlbums(), + libraryStore.getArtists(), + libraryStore.getGenres(), + ]); + // Those reads are recorded like any other call; clear them, or every + // count in every test is off by one. resetHarness(); stubReads(); } @@ -91,7 +100,7 @@ describe('library store: caching', () => { ]).toEqual([['/a.mp3', '/b.mp3'], ALBUMS, ARTISTS, GENRES]); }); - it('refetches everything when a scan completes', async () => { + it('refetches everything a view had loaded when a scan completes', async () => { emit(Events.LibraryScanComplete); await flush();