From 63f0a40f0ec522514c8f5bd22198f84ef7e9664a Mon Sep 17 00:00:00 2001 From: debont80 Date: Wed, 1 Jul 2026 11:09:09 -0400 Subject: [PATCH] perf(renderer): memoize queue summary + stabilize list callbacks (L94/PERF4/PERF5/L163) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - L94/PERF4: add queueSummaryOf — a 1-entry items-ref memo over summarizeQueue. The store hands out a new items array per change, so App's taskbar subscription and DownloadsView's render compute the aggregate once per change (second caller hits the cache) instead of twice per tick. summarizeQueue stays pure/tested. - PERF5: hoist VirtualList estimateSize/getKey/renderItem to stable module-level functions so the virtualizer doesn't churn its measure/key cache. - L163: one useShallow selection replaces QueueItem's 10 individual useDownloads subscriptions (stable actions -> never re-renders the row). typecheck + 268 tests + eslint + prettier green. Co-Authored-By: Claude Opus 4.8 --- CODE-AUDIT.md | 23 +++++++---- src/renderer/src/App.tsx | 4 +- src/renderer/src/components/DownloadsView.tsx | 18 ++++++--- src/renderer/src/components/QueueItem.tsx | 38 ++++++++++++++----- src/renderer/src/store/queueStats.ts | 16 ++++++++ 5 files changed, 74 insertions(+), 25 deletions(-) diff --git a/CODE-AUDIT.md b/CODE-AUDIT.md index 70b8315..6257598 100644 --- a/CODE-AUDIT.md +++ b/CODE-AUDIT.md @@ -702,8 +702,11 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n - [ ] **L93 — SettingsView bypasses the store layer.** It calls `window.api` directly for cookies/ffmpeg/yt-dlp/app-update/backup while using stores for settings/templates/errorlog — mixed data-access in one component. -- [ ] **L94 — `summarizeQueue` recomputed every render** in [DownloadsView.tsx](src/renderer/src/components/DownloadsView.tsx) - (no `useMemo`) — and again on every store change in App's taskbar effect. Memoize/share. +- [x] **L94 — `summarizeQueue` recomputed every render** in [DownloadsView.tsx](src/renderer/src/components/DownloadsView.tsx) + (no `useMemo`) — and again on every store change in App's taskbar effect. Memoize/share. *Fixed: a 1-entry + items-ref memo `queueSummaryOf` in [queueStats.ts](src/renderer/src/store/queueStats.ts) — the store hands + out a new items array per change, so both App's taskbar subscription and DownloadsView's render summarize + it once per change (the second caller hits the cache) instead of twice (also closes PERF4).* - [ ] **L95 — "Sign in" hyphenation varies** in the Cookies card ("Sign in…", "Site to sign in to", "Sign-in window", "Signing in…"). - [ ] **L96 — Ellipsis-on-dialog-buttons inconsistent.** "Export backup…/Import backup…/Sign in…" use `…` (opens a dialog), but "Browse" / "Check for updates" / "Update now" don't. Adopt the `…` = opens-a-dialog convention. @@ -859,8 +862,10 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n double-scroll or misalign the virtualized list's viewport. - [ ] **L162 — Per-thumbnail store subscription.** Every `MediaThumb` calls `useResolvedDark()` (two store subscriptions) instead of receiving a resolved theme prop — N subscriptions per list. -- [ ] **L163 — QueueItem makes 9 separate `useDownloads` selector calls** instead of one destructured - selection — repeated boilerplate per row. +- [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 + shallow compare never re-renders the row).* *Round 8 (2026-06-29) — formatting micro-inconsistencies:* @@ -1698,12 +1703,16 @@ acceptable first:** memoized `QueueItem`s don't re-render when their item is unc 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. -- [ ] **PERF4 — `summarizeQueue` runs twice per progress tick.** Once in App's store subscription (which +- [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. -- [ ] **PERF5 — `VirtualList` gets new function identities each render.** `estimateSize`/`getKey`/ + *Fixed with L94: `queueSummaryOf` computes the aggregate once per items change (shared ref cache), so + App's subscription and DownloadsView's render no longer each run it; the taskbar IPC push was already + deduped by computed value (L25).* +- [x] **PERF5 — `VirtualList` gets new function identities each render.** `estimateSize`/`getKey`/ `renderItem` are passed as inline arrows, so the virtualizer can't rely on stable references (risking - re-measures/cache churn). **Fix:** `useCallback`/hoist them. + 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 store subscriptions) (L162); a long list creates N×2 subscriptions. **Fix:** resolve once, pass as a prop. - [x] **PERF7 — (ref R3) O(n²) media-items rewrites** dominate the main-process cost when downloading a diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 49de5a5..f15a646 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -19,7 +19,7 @@ import { useDownloads } from './store/downloads' // was guaranteed by downloads.ts importing it; now that the cycle is broken, App // owns the eager load so it survives the views becoming lazy-loaded (PERF8). import './store/sources' -import { summarizeQueue } from './store/queueStats' +import { queueSummaryOf } from './store/queueStats' import { useResolvedDark } from './store/systemTheme' import { logError } from './reportError' @@ -82,7 +82,7 @@ function App(): React.JSX.Element { let lastMode = '' let lastBadge = -1 function push(items: ReturnType['items']): void { - const s = summarizeQueue(items) + const s = queueSummaryOf(items) const mode = s.active ? (s.failed > 0 ? 'error' : 'normal') : 'none' if (s.progress === lastFraction && mode === lastMode && s.downloading === lastBadge) return lastFraction = s.progress diff --git a/src/renderer/src/components/DownloadsView.tsx b/src/renderer/src/components/DownloadsView.tsx index 8f4d905..3bed9f3 100644 --- a/src/renderer/src/components/DownloadsView.tsx +++ b/src/renderer/src/components/DownloadsView.tsx @@ -10,13 +10,19 @@ shorthands } from '@fluentui/react-components' import { ArrowDownloadRegular, ArrowClockwiseRegular } from '@fluentui/react-icons' -import { useDownloads } from '../store/downloads' -import { summarizeQueue } from '../store/queueStats' +import { useDownloads, type DownloadItem } from '../store/downloads' +import { queueSummaryOf } from '../store/queueStats' import { DownloadBar } from './DownloadBar' import { QueueItem } from './QueueItem' import { VirtualList } from './VirtualList' import { ScreenHeader, useScreenStyles } from './ui/Screen' +// Hoisted so the virtualizer gets stable function identities and doesn't churn its +// measurement/key cache on every render (PERF5). +const estimateRowSize = (): number => 100 +const getItemKey = (item: DownloadItem): string => item.id +const renderQueueRow = (item: DownloadItem): React.JSX.Element => + const useStyles = makeStyles({ root: { // Fill the scroll area so the queue list below can flex to the remaining @@ -77,7 +83,7 @@ export function DownloadsView(): React.JSX.Element { const clearFinished = useDownloads((s) => s.clearFinished) const retryAll = useDownloads((s) => s.retryAll) - const summary = summarizeQueue(items) + const summary = queueSummaryOf(items) const hasFinished = items.some( (i) => i.status === 'completed' || i.status === 'error' || i.status === 'canceled' ) @@ -146,9 +152,9 @@ export function DownloadsView(): React.JSX.Element { className={styles.listScroll} gap={10} overscan={6} - estimateSize={() => 100} - getKey={(item) => item.id} - renderItem={(item) => } + estimateSize={estimateRowSize} + getKey={getItemKey} + renderItem={renderQueueRow} /> )} diff --git a/src/renderer/src/components/QueueItem.tsx b/src/renderer/src/components/QueueItem.tsx index 77d9a75..31024a6 100644 --- a/src/renderer/src/components/QueueItem.tsx +++ b/src/renderer/src/components/QueueItem.tsx @@ -25,6 +25,7 @@ import { ErrorCircleFilled, EyeOffRegular } from '@fluentui/react-icons' +import { useShallow } from 'zustand/react/shallow' import { useDownloads, type DownloadItem } from '../store/downloads' import { thumbUrl } from '../thumb' import { fmtSchedule } from '../datetime' @@ -115,16 +116,33 @@ export const QueueItem = memo(function QueueItem({ }): React.JSX.Element { const styles = useStyles() const focus = useFocusStyles() - const cancel = useDownloads((s) => s.cancel) - const pause = useDownloads((s) => s.pause) - const resume = useDownloads((s) => s.resume) - const prioritize = useDownloads((s) => s.prioritize) - const saveForLater = useDownloads((s) => s.saveForLater) - const queueNow = useDownloads((s) => s.queueNow) - const remove = useDownloads((s) => s.remove) - const retry = useDownloads((s) => s.retry) - const openFile = useDownloads((s) => s.openFile) - const showInFolder = useDownloads((s) => s.showInFolder) + // One shallow-compared selection instead of 10 separate subscriptions per row + // (L163). The actions are stable, so this never re-renders from the store. + const { + cancel, + pause, + resume, + prioritize, + saveForLater, + queueNow, + remove, + retry, + openFile, + showInFolder + } = useDownloads( + useShallow((s) => ({ + cancel: s.cancel, + pause: s.pause, + resume: s.resume, + prioritize: s.prioritize, + saveForLater: s.saveForLater, + queueNow: s.queueNow, + remove: s.remove, + retry: s.retry, + openFile: s.openFile, + showInFolder: s.showInFolder + })) + ) const active = item.status === 'downloading' || item.status === 'queued' diff --git a/src/renderer/src/store/queueStats.ts b/src/renderer/src/store/queueStats.ts index ef0d2ed..66c79ac 100644 --- a/src/renderer/src/store/queueStats.ts +++ b/src/renderer/src/store/queueStats.ts @@ -66,6 +66,22 @@ export function summarizeQueue(items: DownloadItem[]): QueueSummary { } } +// A 1-entry memo of the last summary, keyed on the `items` array reference. The +// store hands out a new `items` array on every change, and both App's taskbar +// subscription and DownloadsView's render summarize that same array per change — so +// caching by reference computes the O(n) aggregate once per change instead of twice +// (PERF4 / L94). `summarizeQueue` stays pure for the unit tests. +let cachedItems: DownloadItem[] | null = null +let cachedSummary: QueueSummary | null = null + +/** Memoized `summarizeQueue`: recomputes only when the `items` reference changes. */ +export function queueSummaryOf(items: DownloadItem[]): QueueSummary { + if (items === cachedItems && cachedSummary) return cachedSummary + cachedItems = items + cachedSummary = summarizeQueue(items) + return cachedSummary +} + // --- Duplicate detection ---------------------------------------------------- function normalizeUrl(url: string): string {