feat(audit): Batch 22 — perf close-out (PERF3 benchmarked, PERF6/L162 by-design)
PERF3: pumpBench.test.ts pins pump()'s pipeline cost — 0.43ms @5k items, 1.47ms @20k (median) — negligible at a few events/sec, so the count+pointer rewrite of the core queue is declined as risk without reward; the benchmark stays as a 5ms regression tripwire. PERF6/L162: closed as documented-by-design (virtualization bounds mounted thumbs; a theme prop conflicts with PERF5's stable renderer). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
+20
-16
@@ -1316,10 +1316,11 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
|
||||
the shell absorbing the overflow. Verified with an Electron probe at 1100×720 and 800×520 per screen:
|
||||
exactly one active scroller per screen in the normal case, `<main>`/body never scroll, floors hold
|
||||
(probe metrics + screenshots).*
|
||||
- [ ] **L162 — Per-thumbnail store subscription.** Every `MediaThumb` calls `useResolvedDark()` (two store
|
||||
subscriptions) instead of receiving a resolved theme prop — N subscriptions per list. *Deferred — same as
|
||||
PERF6: virtualization/the 500-cap bounds the mounted count, and a prop conflicts with PERF5's hoisted
|
||||
`renderQueueRow`; documented there.*
|
||||
- [x] **L162 — Per-thumbnail store subscription.** Every `MediaThumb` calls `useResolvedDark()` (two store
|
||||
subscriptions) instead of receiving a resolved theme prop — N subscriptions per list. *Resolved as
|
||||
documented-by-design with PERF6 (Batch 22): virtualization bounds the mounted count (all three MediaThumb
|
||||
lists are windowed since Batch 17), and a theme prop would conflict with PERF5's hoisted stable row
|
||||
renderer. See PERF6 for the full rationale.*
|
||||
- [x] **L163 — QueueItem makes 9 separate `useDownloads` selector calls** instead of one destructured
|
||||
selection — repeated boilerplate per row. *Fixed: one `useShallow` selection replaces the 10 individual
|
||||
subscriptions in [QueueItem.tsx](src/renderer/src/components/QueueItem.tsx) (the actions are stable, so the
|
||||
@@ -2453,15 +2454,17 @@ acceptable first:** memoized `QueueItem`s don't re-render when their item is unc
|
||||
`templates: listTemplates()` eagerly as an argument ([download.ts](src/main/download.ts):182) *before*
|
||||
`selectExtraArgs` checks `customCommandEnabled`, so the file is read even when custom commands are off.
|
||||
**Fix:** pass a lazy getter, or short-circuit on the gate first.
|
||||
- [ ] **PERF3 — `pump()` is O(n) per queue event.** It `filter`+`reverse`+`slice`s the entire items array
|
||||
- [x] **PERF3 — `pump()` is O(n) per queue event.** It `filter`+`reverse`+`slice`s the entire items array
|
||||
on every add/done/error/cancel/progress-driven change; with M33's unbounded channel enqueue (thousands of
|
||||
items) every event becomes O(n), and several events fire per second during active downloads. **Fix:**
|
||||
track running/queued counts + a pointer instead of rescanning.
|
||||
*Deferred (core queue, risk > reward unattended): M33 added the enqueue cap, so `items` is bounded (a few
|
||||
thousand at most), and an O(n) scan over that a few times per second is negligible in practice. Rewriting
|
||||
`pump()` to a count+pointer model touches the app's central concurrency logic — exactly the download-engine
|
||||
code that Batch 15 verifies live — so it's not worth the regression risk for a micro-optimization on a
|
||||
bounded list. Revisit with the Batch 15 live pass if profiling ever flags it.*
|
||||
*Resolved with benchmark evidence, no rewrite (Batch 22): [test/pumpBench.test.ts](test/pumpBench.test.ts)
|
||||
measures pump()'s exact array pipeline on the real item shape — median **0.43 ms for a full scan+promote
|
||||
over 5,000 items** and **1.47 ms over 20,000** (beyond any realistic channel enqueue); the common
|
||||
per-progress-event no-free-slot path is cheaper still. At a few events per second that is a fraction of a
|
||||
percent of one core — rewriting the app's central concurrency logic to a count+pointer model would be all
|
||||
regression risk for no observable gain. The benchmark stays in the suite as a regression tripwire (5 ms
|
||||
budget).*
|
||||
- [x] **PERF4 — `summarizeQueue` runs twice per progress tick.** Once in App's store subscription (which
|
||||
also pushes a taskbar IPC every tick, L25) and once in DownloadsView's render (no `useMemo`, L94) — each
|
||||
O(items), every ~second per active download. **Fix:** compute once, memoize, throttle the taskbar push.
|
||||
@@ -2472,13 +2475,14 @@ acceptable first:** memoized `QueueItem`s don't re-render when their item is unc
|
||||
`renderItem` are passed as inline arrows, so the virtualizer can't rely on stable references (risking
|
||||
re-measures/cache churn). **Fix:** `useCallback`/hoist them. *Fixed: the three callbacks are hoisted to
|
||||
stable module-level functions in [DownloadsView.tsx](src/renderer/src/components/DownloadsView.tsx).*
|
||||
- [ ] **PERF6 — Per-thumbnail store subscriptions.** Each `MediaThumb` calls `useResolvedDark()` (two
|
||||
- [x] **PERF6 — Per-thumbnail store subscriptions.** Each `MediaThumb` calls `useResolvedDark()` (two
|
||||
store subscriptions) (L162); a long list creates N×2 subscriptions. **Fix:** resolve once, pass as a prop.
|
||||
*Deferred (bounded + a conflicting tradeoff): the lists that use `MediaThumb` are virtualized (Downloads
|
||||
queue, Library) or capped (History ≤500), so only the ~visible rows mount — N is small and each
|
||||
`useResolvedDark` is a cheap memoized boolean selector. Threading `isDark` as a prop would also fight PERF5
|
||||
(the hoisted, stable `renderQueueRow`, which would have to close over `isDark`), and moving the hook up to
|
||||
`QueueItem` yields the same one-subscription-per-row. Low value, mild conflict — left as documented.*
|
||||
*Resolved as documented-by-design (Batch 22; the analysis was already complete): the lists that use
|
||||
`MediaThumb` are virtualized (Downloads queue, Library — and, since Batch 17, History too) so only the
|
||||
~visible rows mount — N is small and each `useResolvedDark` is a cheap memoized boolean selector. Threading
|
||||
`isDark` as a prop would fight PERF5's hoisted, stable `renderQueueRow` (which would have to close over it),
|
||||
and moving the hook up to `QueueItem` yields the same one-subscription-per-row. Low value, mild conflict —
|
||||
the current shape is the deliberate choice.*
|
||||
- [x] **PERF7 — (ref R3) O(n²) media-items rewrites** dominate the main-process cost when downloading a
|
||||
channel; closed by the cached/atomic `jsonStore` (SIMP1) — the single highest-value perf fix.
|
||||
- [x] **PERF8 — No code-splitting.** All views (incl. the rarely-used Terminal/Settings) load up front in
|
||||
|
||||
Reference in New Issue
Block a user