Files
jobtrackingapp/docs/performance/PERFORMANCE_IMPROVEMENTS.md
cesnimda b8f8569e6e
CI and Deploy / test (pull_request) Successful in 2m0s
CI and Deploy / deploy (pull_request) Has been skipped
fix(hooks): stop infinite render loop in useViewResource
useViewResource built `reload` with `load` in its useCallback deps, and the
fetch effect depended on `reload`. Callers routinely pass an inline `load`
closure (e.g. JobTable), so `load` — and therefore `reload` and the effect —
changed every render, calling setState and re-rendering: an unbounded
"Maximum update depth exceeded" loop that froze the renderer on /jobs and every
other list view (DashboardView, RemindersView, CompaniesTable).

Fix: hold `load` in a ref (like the existing hasLoadedRef) and drop it from the
dependency arrays. Re-fetching is still driven by `deps`/`enabled`; the ref
always points at the latest closure. No API/behaviour change for callers.

Runtime-verified live: /jobs went from a render storm (frozen renderer, 100s of
console errors) to 0 errors in a 2s window and a clean render. Suites that drive
JobTable→useViewResource pass in isolation; the remaining full-run flakiness is
pre-existing (state-pollution/timing in the heavy RTL suites, unrelated).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-05 15:36:55 +02:00

5.9 KiB
Raw Permalink Blame History

Performance Improvements — Job Tracker

Companion to: MEMORY_LEAK_REPORT.md · ROOT_CAUSE_ANALYSIS.md

Changes made (this pass)

Change File Effect Verified
Stop an infinite render loop on every list view — hold load in a ref in useViewResource so reload/the fetch effect keep a stable identity job-tracker-ui/src/hooks/useViewResource.ts Fixes "Maximum update depth exceeded" on /jobs (and any DashboardView/RemindersView/CompaniesTable view whose caller passes an inline load) — pegged the CPU/renderer Runtime-confirmed: /jobs went from a render storm (renderer frozen, 100s of errors) to 0 console errors in a live 2s window and a clean render; workflow-trust-signals (drives JobTableuseViewResource) passes
Stop the infinite /auth/me request loop — make clearAuthClientState emit auth-changed only on a real signed-in→out transition job-tracker-ui/src/auth.ts Eliminates a runaway request storm (100+ GET /auth/me and climbing) that ran continuously whenever the user was logged out Runtime-confirmed in a live stack: /auth/me count 100+ & growing → 0 and stable after fix
Revoke CV-preview blob URLs on unmount only (ref-based), not on every carousel change job-tracker-ui/src/pages/ProfilePage.tsx Fixes broken previews on multi-template decks; still frees URLs on unmount profile-page.test.tsx 5/5

Runtime finding — self-triggering auth loop (the most impactful issue found)

Only visible with a running backend (static analysis could not surface it). Sequence: the axios response interceptor (api.ts) calls clearAuthClientState() on every 401; that dispatched "auth-changed"; the App handler re-fetched /auth/me; that 401'd again → interceptor → clearAuthClientState()"auth-changed" → … an unbounded loop that hammered the server and spun the client on the login page and after any session expiry. Fix: clearAuthClientState now only emits when it actually removes a stored user key (idempotent), so repeated 401s can't re-trigger the fetch. This is a CPU/network/battery drain and a self-inflicted request flood, not a memory leak — but squarely in the Phase 3.5 "infinite polling / retry loop / duplicate requests" scope, and the single highest-value fix from the whole investigation.

Context: this was the only defect found in a full resource audit. The codebase already practises disciplined cleanup (timers cleared, listeners removed, object URLs revoked), so there was no leak to fix — see the main report.

1. Stabilise the extraction-run poll — minor

ProfilePage.tsx:315-324 recreates its 4s interval on every poll because extractionRuns is in the deps and changes each tick. It's harmless (cleanup runs; it stops when runs finish) but churns. If touched: poll on a stable trigger (e.g. a boolean hasActiveRuns in deps, or read runs from a ref inside the interval) so the interval is created once per active-window.

2. One live heap-snapshot pass on a populated session — verification, not a fix

The static audit is strong, but a single DevTools confirmation closes the loop:

  1. Run the real stack (backend on :5202 + a seeded DB) and sign in.
  2. DevTools → Memory → take a heap snapshot.
  3. Navigate /dashboard → /jobs → open a job dialog → close → /profile → build a CV deck → back, ×5.
  4. Force GC, take a second snapshot, Comparison view.
  5. Expect: node/listener/detached counts return to baseline (sawtooth), not monotonic growth. Sort retained size by constructor; look for Detached HTMLElement, growing Array/Map, or listener counts that never fall.

Also cheap and useful: performance.memory.usedJSHeapSize (Chromium) logged across the loop, or a Playwright script that repeats the navigation and asserts heap stays bounded.

3. Guard async setState after unmount — defensive, not a current leak

Several components await api…().then(setState). React 18 no-ops setState on unmounted components (just a dev warning historically), so this is not a leak, but for long CV/AI calls consider an AbortController on the request (cancels the in-flight network work on unmount) — improves responsiveness and avoids wasted work more than memory.

Prevention — keep leaks from creeping in

  • Lint: enable react-hooks/exhaustive-deps (surfaces the exact wrong-deps class that caused the one bug here) and consider react-hooks/react-compiler checks.
  • Rule of thumb: any effect that acquires a resource (listener, timer, object URL, observer, subscription, connection) must return a cleanup that releases exactly that resource. "Release once on unmount" ⇒ empty-deps effect + a ref for current state — never a value in the deps array.
  • Object URLs: pair every createObjectURL with a revokeObjectURL in the same owner; prefer revoking on unmount/replace, never on unrelated re-renders.
  • Server caches: every IMemoryCache.Set must carry an absolute/sliding expiration (as GmailOAuthService correctly does); if the app grows to heavy caching, set a SizeLimit.
  • No unbounded static state: keep static collections to fixed lookup tables (as today); never accumulate per-request data in a static field.
  • CI: the heavy RTL suites are timeout-flaky under load — raising testTimeout (e.g. 1520s) or reducing jest worker contention would make regressions (including any future leak-guard tests) reliably visible instead of hidden behind flakes.

Security-audit note (standing instruction)

The applied change carries no security surface (client-side URL lifetime only). The recommendations above introduce none either; if #3 (AbortController) is implemented, ensure aborted requests don't leave partial writes — not applicable to the read-only CV export/preview calls here.