perf(charts): skip identity zoom-transform replay on chart rebuild#445
Merged
Conversation
setupZoom unconditionally replayed the stored zoom transform after re-attaching the zoom behavior. zoom.transform dispatches start/zoom/end synchronously, and the chart's zoom handler answers with a full axes + grid + every-layer re-render — so every chart rebuild rendered everything twice, even when the user had never zoomed (identity transform, the overwhelmingly common case). Profiling the live site shows this doubles the cost of every ~250-490ms rebuild long task. Replay now happens only when there is actually a zoom to restore (stored transform or node state non-identity). Zoom preservation across rebuilds — the reason the replay exists (docs/d3-charts.md 'Zoom Transform Preservation') — is unchanged: non-identity transforms replay exactly as before, including charts with a non-1 defaultZoomK. The identity replay had one observable side effect: its emit dismissed a pinned tooltip on every rebuild. useD3ChartRenderer now performs that dismissal explicitly when the replay is skipped, so behavior is identical.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Second of the INP fixes (field CrUX INP 273 ms p75, failing CWV). The Firefox profile of the live inference tab shows every chart rebuild renders everything twice: once directly (axes, grid, all layers at base scales) and once more via
setupZoom's unconditionalsvg.call(zoom.transform, stored)— which synchronously dispatches a zoom event whose handler re-renders axes + grid + every layer again. The profiled hot path:When the user has never zoomed (identity transform — the overwhelmingly common case) the replay repositions nothing: it just repeats the full render for pixel-identical output, doubling every ~250–490 ms rebuild long task.
Changes
useChartZoom.setupZoom— replay the stored transform only when it (or the node's internal__zoomstate, as a defensive desync check) is non-identity. At identity, attaching the behavior already leaves d3's internal state consistent with the freshly drawn base-scale DOM.useD3ChartRenderer— the identity replay had one observable side effect: its emit dismissed a pinned tooltip on every rebuild. That dismissal is now performed explicitly when the replay is skipped, so observable behavior is unchanged.Zoom preservation is untouched (docs/d3-charts.md "Zoom Transform Preservation" / pitfalls "D3 Zoom Transform Loss"): non-identity transforms — user zooms and
defaultZoomKcharts — replay exactly as before, verified by tests.Impact
Halves the main-thread cost of every chart rebuild while not zoomed: initial mounts, data loads, comparison-date loads, legend/precision toggles, metric switches — across all D3Chart consumers (inference scatter, GPU timeline, evaluation/reliability bars, trends, calculator). Stacks with #444.
Tests
New
useChartZoom.setup.test.ts(jsdom, real d3-zoom on a real SVG):zoomTransformRefstays in sync after replaydefaultZoomK≠ 1 → initial transform still appliedFull unit suite passes (2037 tests).
pnpm typecheck/lint/fmtclean.Applies uniformly to official and
?unofficialrun=overlay rendering — both go through the same rebuild path; overlay rooflines/points reposition via the same non-identity replay when zoomed.Note
Medium Risk
Changes the shared D3 chart zoom setup path used on every rebuild; risk is mitigated by focused tests and preserved behavior for non-identity zoom and tooltip dismissal.
Overview
Stops redundant full chart re-renders when zoom is at identity (the common case) by not calling
zoom.transformduringsetupZoomunless the stored transform or the SVG node’s internal__zoomstate is non-identity.Non-identity zoom (user pan/zoom,
defaultZoomK) still replays exactly as before.useD3ChartRendererexplicitly dismisses a pinned tooltip on rebuild when that identity replay is skipped, matching the old side effect of the zoom handler firing.Adds
useChartZoom.setup.test.tscovering identity skip, zoom preservation, ref sync, defensive node/ref desync, anddefaultZoomKinitial apply.Reviewed by Cursor Bugbot for commit 0561024. Bugbot is set up for automated code reviews on this repo. Configure here.