Skip to content

perf(charts): eliminate redundant D3 rebuilds at mount#444

Open
Oseltamivir wants to merge 3 commits into
masterfrom
perf/stabilize-chart-mount-renders
Open

perf(charts): eliminate redundant D3 rebuilds at mount#444
Oseltamivir wants to merge 3 commits into
masterfrom
perf/stabilize-chart-mount-renders

Conversation

@Oseltamivir

@Oseltamivir Oseltamivir commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Context

Field data (CrUX) has the site failing Core Web Vitals on INP (273 ms p75). A Firefox performance profile of the live inference tab shows every chart performs 2–3 full D3 teardown/rebuild passes immediately after mount, each ~300 ms of main-thread blocking — all rendering identical pixels. Two unstable identities in shared hooks are responsible.

Changes

useThemeColors — on mount, setThemeColors(getChartThemeColors()) ran in a setTimeout(0) even though the synchronous useState initializer already read the correct computed styles (next-themes applies the theme class in a blocking inline script before hydration). The new object identity invalidates getCssColor → every chart's layers memo → full rebuild. Now:

  • the first defined resolvedTheme is recorded and skipped (no state update, no rebuild)
  • actual theme switches still re-read via setTimeout(0) (load-bearing: lets the <html> class flip settle before consumers re-resolve CSS variables)

useResponsiveChartDimensions — the ResizeObserver fires once right after observe() with the same width the ref callback just measured, and the hook produced a fresh dimensions object for it; dimensions is a dependency of the chart render effect, so that initial observation triggered a second full rebuild. Same-size updates now return the previous object so React bails out. Same guard applied to the height-sync effect.

Both fixes are identity-only — real theme switches and real resizes behave exactly as before.

Impact

Removes ~600-900 ms of main-thread blocking from initial chart mount (2 charts × 1-2 redundant ~300 ms rebuilds), benefiting every D3 chart (inference, GPU timeline, evaluation/reliability bars, trends, calculator).

Tests

  • useThemeColors.test.ts — identity stability across mount effects and the next-themes hydration sequence (regression tests: fail against the old implementation), re-read on real theme change, no re-read on same-theme re-render, vendor color resolution, high-contrast color map.
  • useResponsiveChartDimensions.test.ts — same-size observation keeps identity (regression test: fails against old implementation), new width updates, observer cleanup on container change/detach.
  • Full unit suite passes (105 files / 2043 tests). pnpm typecheck, pnpm lint, pnpm fmt clean.

Not chart-data-path-specific, so no overlay-specific behavior change: unofficial-run overlays render through the same D3Chart mount path and simply stop double-rebuilding too.


Note

Low Risk
Performance-only changes to chart hooks and color caching; behavior for real theme switches and resizes is preserved and covered by new tests.

Overview
Reduces redundant D3 chart work at mount by keeping hook return values referentially stable when values do not actually change, and by caching expensive high-contrast palette generation.

useThemeColors no longer re-fetches theme colors on the first resolvedTheme from next-themes (hydration matches what useState already read), so themeColors / getCssColor stay stable and charts skip pointless rebuilds; real theme switches still refresh after setTimeout(0).

useResponsiveChartDimensions reuses the same dimensions object when width/height are unchanged—including the initial ResizeObserver callback that duplicates the ref measurement—and applies the same guard when only height prop updates.

generateHighContrastColors adds a module-level cache keyed by vendor, theme, count, and palette mode so repeated iwanthue clustering is skipped without changing output colors.

New Vitest coverage exercises mount/hydration stability, resize observer behavior, and cache hits/misses.

Reviewed by Cursor Bugbot for commit 3126dd8. Bugbot is set up for automated code reviews on this repo. Configure here.

Addendum: iwanthue palette caching (second commit)

Same color-subsystem, same goal (stop repeating identical expensive work): `generateHighContrastColors` runs iwanthue's force-vector clustering (quality 50 × 5 attempts, tens of ms) on every recompute — visible in the profile's render-path `useMemo` stacks at mount and on every high-contrast legend/theme change. The output is deterministic (seeded) and independent of key names — a vendor group's palette depends only on (vendor, theme, count, ban/preferred mode, lightness bounds) — so palettes are now cached in a module-level map keyed by exactly those inputs. Key space is tiny; no eviction.

Tests: 3 new cache tests using a `{ spy: true }` mock of iwanthue (repeat request → no re-run, fails without the cache; name-independence → cache hit with distinct colors; different count → new entry). All 30+ pre-existing color-quality tests (brand zones, banned hues, min pairwise distances, determinism) pass unchanged — output colors are identical.

Profiling the live site showed every chart runs 2-3 full (visually
identical) D3 teardown/rebuild passes right after mount, each ~300ms of
main-thread time. Two unstable identities cause this:

- useThemeColors re-set themeColors via setTimeout(0) on mount even
  though the synchronous useState read already saw the correct computed
  styles (next-themes applies the theme class pre-hydration). The new
  object identity invalidates getCssColor -> layers -> full rebuild.
  Now only an actual resolvedTheme change re-reads colors; the
  setTimeout(0) is kept for real switches so the <html> class flip
  settles first.

- useResponsiveChartDimensions produced a new dimensions object from
  the ResizeObserver's initial callback (same width the ref callback
  just measured) and from same-value height updates. Same-size updates
  now keep the previous object so React bails out.

Both fixes are identity-only: real theme switches and real resizes
behave exactly as before.
@Oseltamivir Oseltamivir requested a review from adibarra as a code owner June 12, 2026 04:20
@vercel

vercel Bot commented Jun 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
inferencemax-app Ready Ready Preview, Comment Jun 12, 2026 5:58am

Request Review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 9aa141e. Configure here.

return;
}
if (appliedThemeRef.current === resolvedTheme) return;
appliedThemeRef.current = resolvedTheme;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SSR hydration skips theme refresh

High Severity

After SSR, themeColors can keep a server placeholder rootStyles because the effect no longer schedules the post-mount getChartThemeColors() refresh when resolvedTheme is undefined, and the first defined theme is skipped. Charts may error in getCssColor or show wrong CSS variables until the user toggles theme.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 9aa141e. Configure here.

iwanthue's force-vector clustering (quality 50 x 5 attempts) costs tens
of milliseconds per call and showed up repeatedly in the profile's
useMemo render paths whenever high-contrast colors recompute (mount,
legend toggles, theme switches). The output is fully deterministic
(seeded RNG) and independent of the key names - a vendor group's
palette depends only on item count, vendor zone/ban mode, theme seed,
and lightness bounds - so identical requests across renders, charts,
and tabs now share one cached entry. Key space is tiny (vendors x
themes x counts x 3 modes); no eviction needed.

Existing color-quality tests (brand zones, bans, min distances) pass
unchanged, confirming identical output.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant