perf(frontend): add the bound, the lookup and the lazy dialog
Four small modules the views below adopt: - `lru-map.ts` — a Map re-inserted on read and trimmed from the front. `explore-view` never unmounts and its two art caches were plain Maps: twenty-four searches retained 20.58 MB and were still accelerating, a cover thumbnail being ~27 kB of base64 and an artist photo ~128 kB. - `cache-stats.ts` — a bound has to stay checkable, so caches register and `window.__yjCacheStats()` reports entries, retained chars and cap in one eval, rather than the next session having to rebuild the twenty-four-search reproduction first. - `track-index.ts` — a WeakMap from the tracks array's identity to a Map<FilePath, Track>. Five components turned selected file paths back into tracks with `filePaths.map(fp => tracks.find(...))`, so "Select all -> Edit tags" at 50 000 tracks blocked the main thread for 3.0 to 6.3 s. 68 ms after. Keying on the array's identity is safe for the same reason the memoized filter caches are, and it is collected for free when the store drops the array. - `lazy-track-details.ts` — one memoised dynamic import, because `track-details` (42 kB) was imported for side effect by all five components that open it and so was evaluated before first paint however the routes were split.
This commit is contained in:
@@ -0,0 +1,69 @@
|
||||
/**
|
||||
* A registry of the app's in-memory caches, for measurement.
|
||||
*
|
||||
* `perf.M7`/`M8` are retention findings, and retention is only ever
|
||||
* credible as a number: the two Explore caches did not show as heap
|
||||
* growth in two separate sessions of a ten-view browse script, because
|
||||
* that script *visits* Explore and never types in it, so the caches
|
||||
* stayed empty. What made them real was a session that searched twelve
|
||||
* times and watched the heap climb 9 MB monotonically.
|
||||
*
|
||||
* The lesson is that a bound needs a way to be *checked*, not just
|
||||
* written — otherwise the next session has to rediscover the same
|
||||
* reproduction before it can tell whether the LRU still holds. A cache
|
||||
* registers itself here and `window.__yjCacheStats()` reports every one
|
||||
* in a single eval, which is what `e2e/perf/measure.mjs` reads.
|
||||
*
|
||||
* This follows `src/icons/index.ts`'s `__yjIconMisses`: an unconditional
|
||||
* measurement surface, costing one function and one Map, rather than a
|
||||
* dev-only build branch that is therefore never exercised in the build
|
||||
* that ships.
|
||||
*/
|
||||
|
||||
/** What one cache reports about itself. */
|
||||
export interface CacheStat {
|
||||
/** Number of live entries. */
|
||||
entries: number;
|
||||
/** Total length of the strings retained, where the cache holds strings. */
|
||||
chars: number;
|
||||
/** The cap, so a reading can be read against its bound. */
|
||||
limit: number;
|
||||
}
|
||||
|
||||
const probes = new Map<string, () => CacheStat>();
|
||||
|
||||
declare global {
|
||||
interface Window {
|
||||
__yjCacheStats?: () => Record<string, CacheStat>;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Register a cache under a stable name. Re-registering the same name
|
||||
* replaces the probe, which is what a cached view remounting in a test
|
||||
* needs.
|
||||
*/
|
||||
export function registerCacheProbe(name: string, probe: () => CacheStat): void {
|
||||
probes.set(name, probe);
|
||||
}
|
||||
|
||||
/** Drop a probe — a per-instance cache going away with its host. */
|
||||
export function unregisterCacheProbe(name: string): void {
|
||||
probes.delete(name);
|
||||
}
|
||||
|
||||
if (typeof window !== 'undefined') {
|
||||
window.__yjCacheStats = () => {
|
||||
const out: Record<string, CacheStat> = {};
|
||||
|
||||
for (const [name, probe] of probes) {
|
||||
try {
|
||||
out[name] = probe();
|
||||
} catch {
|
||||
// A probe must never be able to break a measurement run.
|
||||
}
|
||||
}
|
||||
|
||||
return out;
|
||||
};
|
||||
}
|
||||
@@ -0,0 +1,65 @@
|
||||
/**
|
||||
* Load the `<track-details>` chunk at the point of use.
|
||||
*
|
||||
* `track-details` is 42 kB and is opened from a context menu in five
|
||||
* components — `track-list`, `cover-grid`, `queue-panel`,
|
||||
* `playlist-details` and `smart-playlist-details`. All five imported it
|
||||
* statically, so it rode in the startup chunk however `index.ts` split
|
||||
* the routes: a dialog nobody may open, parsed before first paint.
|
||||
*
|
||||
* The awkward part is that `document.createElement` (and lit rendering
|
||||
* a tag) on an *undefined* custom element yields an inert
|
||||
* `HTMLElement` rather than throwing — the same trap `index.ts`'s
|
||||
* `VIEW_LOADERS` exists for. All five hosts render
|
||||
* `<track-details></track-details>` in their template and reach it with
|
||||
* `@query`, so before the chunk lands that query returns a real
|
||||
* element with no `show()` on it: an optional-chained call would throw,
|
||||
* and a truthiness guard would silently do nothing. So every opener
|
||||
* awaits this first. Once `define()` runs, the already-rendered element
|
||||
* upgrades in place, which is why the hosts need no render guard.
|
||||
*
|
||||
* The promise is memoised, so the second open is free. A *rejected*
|
||||
* one is not: a chunk that failed to arrive once (a dropped
|
||||
* connection mid-session) must be retryable, so the failure clears the
|
||||
* memo and says so at the level the plan's rule picks — the user asked
|
||||
* for a dialog, it did not happen, and asking again is meaningful.
|
||||
*/
|
||||
|
||||
import { notificationStore } from '@store/notification-store.js';
|
||||
import { describeError } from '@utils/describe-error.js';
|
||||
|
||||
let pending: Promise<boolean> | null = null;
|
||||
|
||||
/**
|
||||
* Resolve once `<track-details>` is defined and its element upgraded.
|
||||
*
|
||||
* Returns `false` if the chunk could not be loaded, having already
|
||||
* told the user; the caller should simply return.
|
||||
*
|
||||
* @param retry Re-runs the action that wanted the dialog, offered to
|
||||
* the user as the notification's action.
|
||||
*/
|
||||
export function loadTrackDetails(retry?: () => void): Promise<boolean> {
|
||||
pending ??= import('@components/track-details/track-details.js')
|
||||
.then(() => customElements.whenDefined('track-details'))
|
||||
.then(() => true)
|
||||
.catch((err: unknown) => {
|
||||
// Let the next attempt try again rather than caching the
|
||||
// failure for the life of the session.
|
||||
pending = null;
|
||||
console.error('Failed to load track-details chunk:', err);
|
||||
|
||||
notificationStore.persistent({
|
||||
title: 'Could not open track details',
|
||||
text: describeError(err),
|
||||
key: 'track-details-chunk',
|
||||
action: retry
|
||||
? { label: 'Try again', run: retry }
|
||||
: undefined,
|
||||
});
|
||||
|
||||
return false;
|
||||
});
|
||||
|
||||
return pending;
|
||||
}
|
||||
@@ -0,0 +1,98 @@
|
||||
/**
|
||||
* A `Map` with a ceiling.
|
||||
*
|
||||
* `perf.M7`/`M8`: the Explore caches were never evicted, and Explore is
|
||||
* a cached primary view that never unmounts — so a desktop player left
|
||||
* open for days grew monotonically. Measured at twelve searches:
|
||||
* **+8.48 MB of retained heap**, climbing 0.7 MB per search with no
|
||||
* sign of levelling off, because a cover thumbnail is a ~27 kB base64
|
||||
* data URL and an artist photo is a ~128 kB one.
|
||||
*
|
||||
* JS `Map` already iterates in insertion order, so the whole LRU is:
|
||||
* re-insert on read, and drop from the front when over the cap. That
|
||||
* is deliberately all this is — a dependency, or a generic cache with
|
||||
* TTLs and weak refs, would be more machinery than the two call sites
|
||||
* justify.
|
||||
*
|
||||
* One rule for callers, learned by measuring: **two caches holding the
|
||||
* same string must have the same cap.** `artistImageCache` and
|
||||
* `exploreCache.artists` both hold the artist photo's data URL, so
|
||||
* bounding either one alone frees nothing at all — the other still
|
||||
* pins every string. A bound is only a bound if it covers every
|
||||
* reference.
|
||||
*/
|
||||
export class LRUMap<K, V> {
|
||||
private map = new Map<K, V>();
|
||||
|
||||
constructor(readonly limit: number) {
|
||||
if (limit < 1) throw new Error('LRUMap: limit must be at least 1');
|
||||
}
|
||||
|
||||
get size(): number {
|
||||
return this.map.size;
|
||||
}
|
||||
|
||||
/** Read, and mark the entry most-recently-used. */
|
||||
get(key: K): V | undefined {
|
||||
if (!this.map.has(key)) return undefined;
|
||||
|
||||
const value = this.map.get(key) as V;
|
||||
// Re-insertion moves it to the end of the iteration order, which
|
||||
// is what makes the front of the map the eviction candidate.
|
||||
this.map.delete(key);
|
||||
this.map.set(key, value);
|
||||
|
||||
return value;
|
||||
}
|
||||
|
||||
/**
|
||||
* Membership, *without* marking the entry used.
|
||||
*
|
||||
* Both Explore caches store `''` to mean "already attempted, no art"
|
||||
* — a negative marker that also prevents a duplicate in-flight
|
||||
* fetch. Those probes should not keep a dead entry alive ahead of
|
||||
* one that is actually being rendered.
|
||||
*/
|
||||
has(key: K): boolean {
|
||||
return this.map.has(key);
|
||||
}
|
||||
|
||||
set(key: K, value: V): this {
|
||||
this.map.delete(key);
|
||||
this.map.set(key, value);
|
||||
|
||||
while (this.map.size > this.limit) {
|
||||
const oldest = this.map.keys().next();
|
||||
|
||||
if (oldest.done) break;
|
||||
|
||||
this.map.delete(oldest.value);
|
||||
}
|
||||
|
||||
return this;
|
||||
}
|
||||
|
||||
delete(key: K): boolean {
|
||||
return this.map.delete(key);
|
||||
}
|
||||
|
||||
clear(): void {
|
||||
this.map.clear();
|
||||
}
|
||||
|
||||
values(): IterableIterator<V> {
|
||||
return this.map.values();
|
||||
}
|
||||
|
||||
keys(): IterableIterator<K> {
|
||||
return this.map.keys();
|
||||
}
|
||||
|
||||
entries(): IterableIterator<[K, V]> {
|
||||
return this.map.entries();
|
||||
}
|
||||
|
||||
[Symbol.iterator](): IterableIterator<[K, V]> {
|
||||
return this.map[Symbol.iterator]();
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,66 @@
|
||||
/**
|
||||
* A `FilePath → Track` lookup over an array the store owns.
|
||||
*
|
||||
* Five components turn a list of selected file paths back into tracks
|
||||
* with `filePaths.map(fp => tracks.find(t => t.FilePath === fp))`
|
||||
* (audit `perf.m6`). That is O(selection × total), and at 50 000
|
||||
* tracks "Select all → Edit tags" blocked the main thread for **six
|
||||
* seconds** — measured, through the real opener, before this existed.
|
||||
*
|
||||
* The cache is a `WeakMap` keyed on the **identity of the array**,
|
||||
* which is the signal this app already uses for exactly this: the
|
||||
* stores replace the array when its contents change and share every
|
||||
* unchanged member (see `library-store`'s `TrackPlayCountChanged`
|
||||
* patch, and `track-list`'s memoized filter/sort caches). So a stale
|
||||
* map is not reachable — a changed list is a different array and gets
|
||||
* a different map — and an array nobody holds any more takes its map
|
||||
* with it.
|
||||
*
|
||||
* Build cost is one O(total) pass, paid on the first lookup against a
|
||||
* given array and never again.
|
||||
*/
|
||||
|
||||
import type { library } from '@go/models';
|
||||
|
||||
const byArray = new WeakMap<
|
||||
readonly library.Track[],
|
||||
Map<string, library.Track>
|
||||
>();
|
||||
|
||||
/** The lookup for `tracks`, built once per array identity. */
|
||||
export function tracksByFilePath(
|
||||
tracks: readonly library.Track[],
|
||||
): Map<string, library.Track> {
|
||||
let map = byArray.get(tracks);
|
||||
|
||||
if (map) return map;
|
||||
|
||||
map = new Map<string, library.Track>();
|
||||
|
||||
for (const track of tracks) {
|
||||
// First wins: a duplicate path would be the same file, and
|
||||
// `find` returned the first too.
|
||||
if (!map.has(track.FilePath)) map.set(track.FilePath, track);
|
||||
}
|
||||
|
||||
byArray.set(tracks, map);
|
||||
|
||||
return map;
|
||||
}
|
||||
|
||||
/** Resolve file paths to tracks, dropping any that are not present. */
|
||||
export function tracksForPaths(
|
||||
tracks: readonly library.Track[],
|
||||
filePaths: readonly string[],
|
||||
): library.Track[] {
|
||||
const byPath = tracksByFilePath(tracks);
|
||||
const result: library.Track[] = [];
|
||||
|
||||
for (const filePath of filePaths) {
|
||||
const track = byPath.get(filePath);
|
||||
|
||||
if (track) result.push(track);
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
Reference in New Issue
Block a user