perf(data-view): virtualize the timeline on both axes - #887
Open
rsbh wants to merge 10 commits into
Open
Conversation
`virtualized` culled horizontally only, so every card whose x fell in the time window mounted regardless of where it sat vertically. With one lane per row, 10k cards meant 10k wrappers and 10k ResizeObservers on a canvas ~820k px tall. Culling now covers both axes, at a cost proportional to what is on screen: - Cards are indexed by lane in CSR form (one flat Int32Array, no per-lane allocation). A frame walks only the lanes the viewport covers and searches the x window inside each, rather than slicing every card in the time window. Visible lanes are bounded by pane height over lane pitch, not by row count. - Lane lookup inverts the pitch arithmetically when the stack is uniform, and binary-searches the lane boxes when group bands break it. - The viewport is read in a layout effect, so the first paint is already culled instead of flashing the whole canvas. Lane heights become fixed to `estimatedRowHeight` while virtualized. A culled card never mounts and so never measures, so measured lanes would resize under the user mid-scroll. This is a behaviour change: a card taller than the value now overflows its lane instead of growing it. The unvirtualized path still measures and re-stacks exactly as before. Also replaces two hot spots the culling would otherwise inherit: - `packLanes` swaps its per-item lane scan for a sweep (free-lane bitmap plus a Dial bucket queue) past 64 items. Identical assignment — first-fit is smallest-free-lane — in O(n) rather than O(items x lanes). 10k mutually overlapping cards: 53.4ms -> 0.8ms; at realistic density 4.5ms -> 1.1ms. - Ordering cards by x moves to a counting sort, 1.9ms -> 0.1ms, degrading to a comparison sort on clustered input. 10k cards over a year: 10,000 rendered wrappers -> 255, 20,760 DOM nodes -> 965, 10,000 ResizeObservers -> 0. Per-frame cost in a real browser is not yet measured; jsdom does no layout. Lane assignment is pinned by golden digests recorded from the previous implementation, so the rewrites are verified against it rather than against their own output. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Phase 2 of the timeline virtualization work: the axis furniture was never culled, and the culling window was recomputed on every scroll frame. Chrome culling. Gridlines already sliced to the visible window, but tick labels, month bands, marker badges and lines, and group-section slots all rendered across the whole domain however narrow the window onto it was. They now share the card window. Over a year at day scale: 183 tick labels -> 31, 12 month bands -> 3, and a group slot scrolled far off-screen no longer mounts. Bands tile the domain edge to edge, so the one straddling the left bound is kept by stepping back from the first band past it, which is what the sticky month label rides on. Scroll work. Three changes, measured in a production build at 50k rows: - The culling window is quantized: it holds until the pane has travelled half its overscan, instead of committing React state on all 60 frames a second to rebuild a nearly identical slice. Scrolling is compositor work the browser does for free; this keeps React out of it. p95 frame 68.7ms -> 32.6ms. - Overscan drops from a full viewport per side to half. The mounted set covers (1 + 2r)^2 viewports, so cost grows with the square of overscan while the travel it buys grows linearly. Mounted cards at 10k: 287 -> 141. - `onVisibleRangeChange` reads the live offset from a ref rather than the quantized state, so it stays exact while no longer forcing a render. A timeline with only that prop set now re-renders on nothing while scrolling. Allocation and complexity, mostly aimed at mount: - `timedItems` materialised every row again to compute a two-number extent; now reduced in place, O(1) space. - Lane fields are written onto the positioned items instead of spread into new objects — one object per card for the whole pipeline instead of two. - `maxPackWidth` folded into the ordering pass rather than its own traversal. - The measurement-cleanup sweep and its row-id Set are skipped under a fixed lane pitch, where nothing reads the measurements. - `queryKey`'s JSON.stringify was running on every render, so on every commit. Also drops one ResizeObserver per card while virtualized (verified: 10,001 -> 0 at 10k rows), since a fixed pitch consumes no measurements. Adds apps/www/src/app/examples/timeline-stress as a manual-QA harness at 1k/10k/50k rows with toggles and a live DOM/frame readout. What this does not fix: at 50k the commit itself still costs ~70ms in a production build, and that cost is flat against the rendered set — 450 cards and 265 cards measure the same, and 17 cards on a taller canvas measured worse. It is browser-side style/layout/paint over a 14,600 x 139,170px canvas, so no amount of further culling touches it. Fixing it means not having a canvas that size: a viewport-sized layer positioned by transform over a full-size spacer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Grid, marker, and cursor lines are pinned `top: 0; bottom: 0`, so each is
a one-pixel-wide element as tall as the entire canvas — 28,798px at 10k
rows, 139,170px at 50k. Rasterizing a dashed sub-pixel border over that
height costs the same whether one card is on screen or four hundred,
which is why culling their count (183 -> 31) never moved frame time.
Clamped to the vertical cull window that already exists, they cost what a
viewport costs. Measured on the stress harness scrolling both axes, dev
build, 89 sampled frames:
before after
10k rows p95 291.7ms, 40 over 50ms p95 23.5ms, 2 over 50ms
50k rows p95 1609.5ms, 86 over 50ms p95 33.5ms, 2 over 50ms
Verified by injecting the old geometry back over the fixed build, so both
sides of the comparison ran in one session on one machine.
Only under `virtualized` — `needsViewport` leaves the viewport unmeasured
otherwise, so the unvirtualized path renders exactly as before.
The harness only ever drove a 365-day domain, so the horizontal axis stopped at 14,600px. A 5-year span widens the canvas to 73,040px, and because the same rows spread over 5x the domain overlap far less, it is a different shape of test rather than simply a bigger one: 50k rows go from a 139,170px canvas to 30,192px. Also surfaces canvas width and gridline height in the readout. Gridline height is the direct check on the clamp — it should track the pane (~1,000px), not the canvas.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Warning Review limit reached
Next review available in: 45 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe timeline now uses stable indexed x-ordering and sweep-based lane packing for large inputs. Sequence Diagram(s)sequenceDiagram
participant TimelineStressPage
participant DataViewTimeline
participant viewportRef
participant laneIndex
TimelineStressPage->>DataViewTimeline: configure data and virtualization
DataViewTimeline->>viewportRef: read viewport during scroll
viewportRef-->>DataViewTimeline: return live dimensions and offsets
DataViewTimeline->>laneIndex: resolve visible lanes and x-ranges
laneIndex-->>DataViewTimeline: return visible timeline elements
DataViewTimeline-->>TimelineStressPage: render timeline and measured metrics
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/www/src/app/examples/timeline-stress/page.tsx (1)
395-412: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winHoist the
markersarray out of render.The
markersliteral at Line 404 gets a new identity on every render of this page.DataViewTimelinememoizesresolvedMarkersonmarkers, so that memo recomputes on every commit, andtimeScaleconsumers downstream see the churn. This page measures frame cost, so the extra work lands in the number the harness reports.♻️ Proposed change
+const MARKERS = [ + { date: '2025-07-01', label: 'H2', variant: 'accent' as const } +];- markers={[{ date: '2025-07-01', label: 'H2', variant: 'accent' }]} + markers={MARKERS}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/www/src/app/examples/timeline-stress/page.tsx` around lines 395 - 412, Hoist the timeline marker definition out of the component render so the `markers` prop passed to `DataView.Timeline` remains referentially stable across renders. Reuse the shared constant for the existing `2025-07-01` H2 accent marker without changing its behavior.packages/raystack/components/data-view/components/timeline.tsx (1)
993-1009: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCorrect the stale overscan description.
OVERSCAN_RATIOis 0.5, so overscan is half a viewport per side, not a full one. This comment states a full viewport on each side. The same claim appears at Line 1506 ("one extra viewport on each side as overscan"). The math stays safe (the commit threshold is a quarter viewport of travel against half a viewport of coverage), but the stated justification does not match the constant.📝 Proposed comment fix
- // Otherwise hold the last window until the pane has travelled half its - // overscan. Scrolling is compositor work the browser does for free; - // committing state on every pixel drags React into all 60 frames a - // second to rebuild a slice that is nearly always identical. Overscan is - // a full viewport on each side, so half of it is spare coverage: the - // rendered set still spans the visible window with an overscan/2 margin - // at the moment of the next commit. + // Otherwise hold the last window until the pane has travelled half its + // overscan. Scrolling is compositor work the browser does for free; + // committing state on every pixel drags React into all 60 frames a + // second to rebuild a slice that is nearly always identical. Overscan is + // half a viewport on each side (`OVERSCAN_RATIO`), so half of that is + // spare coverage: the rendered set still spans the visible window with + // an overscan/2 margin at the moment of the next commit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/raystack/components/data-view/components/timeline.tsx` around lines 993 - 1009, Update the explanatory comments near the overscan commit threshold and the corresponding comment around the later overscan logic to state that OVERSCAN_RATIO provides half a viewport per side, not a full viewport. Keep the existing slack calculations and behavior unchanged.packages/raystack/components/data-view/data-view.types.tsx (1)
435-441: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueComplete the list of culled elements.
The implementation also culls axis ticks, month bands, markers, and group slots, and it clamps grid, marker, and cursor lines to the vertical window. Vertical culling additionally requires a measured pane height; with a zero-height pane the renderer keeps horizontal culling only. State both so consumers know what to expect.
📝 Proposed doc update
/** - * Render only the cards and gridlines near the visible viewport, culling on - * both axes — a frame costs what's on screen rather than what's in the data. - * Recommended whenever the domain is long or rows are numerous. + * Render only what is near the visible viewport, culling on both axes — a + * frame costs what's on screen rather than what's in the data. This covers + * cards, axis ticks, bands, markers, gridlines, and group slots; the grid, + * marker, and cursor lines are clamped to the visible vertical window. + * Recommended whenever the domain is long or rows are numerous. + * + * Vertical culling needs a measured pane height. Where the pane reports a + * height of 0 (SSR, or a test environment without layout), culling stays + * horizontal only. * * Lane heights become fixed to `estimatedRowHeight`; see the note there. */🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/raystack/components/data-view/data-view.types.tsx` around lines 435 - 441, Update the virtualization option documentation near the “Render only the cards and gridlines” description to list all culled elements: axis ticks, month bands, markers, and group slots, along with cards and gridlines. Document that grid, marker, and cursor lines are clamped to the vertical window, and that vertical culling requires a measured pane height; when the pane height is zero, only horizontal culling remains.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/www/src/app/examples/timeline-stress/page.tsx`:
- Around line 145-182: Update useFrameMonitor and its caller so the pane element
is held in state and the scroll-listener effect runs after the element is
assigned, rather than relying on paneRef.current during the initial effect.
Remove the dead initial until assignment, and either implement the stated
behavior of extending the measurement window while scrolling or revise the
comment to describe the existing fixed one-second window; ensure repeated
scrolls during an active measurement are handled consistently with that
behavior.
In `@packages/raystack/components/data-view/utils/order-by-x.tsx`:
- Around line 45-63: Update the ordering logic around the minX/maxX scan and
bucket sorting to detect any non-finite x value before computing buckets. When
present, use a comparison-based fallback with explicit ordering for finite
values, positive and negative infinity, and NaN, while preserving input order
for equal x values; retain bucket sorting only when every x is finite.
---
Nitpick comments:
In `@apps/www/src/app/examples/timeline-stress/page.tsx`:
- Around line 395-412: Hoist the timeline marker definition out of the component
render so the `markers` prop passed to `DataView.Timeline` remains referentially
stable across renders. Reuse the shared constant for the existing `2025-07-01`
H2 accent marker without changing its behavior.
In `@packages/raystack/components/data-view/components/timeline.tsx`:
- Around line 993-1009: Update the explanatory comments near the overscan commit
threshold and the corresponding comment around the later overscan logic to state
that OVERSCAN_RATIO provides half a viewport per side, not a full viewport. Keep
the existing slack calculations and behavior unchanged.
In `@packages/raystack/components/data-view/data-view.types.tsx`:
- Around line 435-441: Update the virtualization option documentation near the
“Render only the cards and gridlines” description to list all culled elements:
axis ticks, month bands, markers, and group slots, along with cards and
gridlines. Document that grid, marker, and cursor lines are clamped to the
vertical window, and that vertical culling requires a measured pane height; when
the pane height is zero, only horizontal culling remains.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5a3a5de8-f7c5-4688-ba3a-7b4b9425c668
📒 Files selected for processing (6)
apps/www/src/app/examples/timeline-stress/page.tsxpackages/raystack/components/data-view/__tests__/timeline.test.tsxpackages/raystack/components/data-view/components/timeline.tsxpackages/raystack/components/data-view/data-view.types.tsxpackages/raystack/components/data-view/utils/order-by-x.tsxpackages/raystack/components/data-view/utils/pack-lanes.tsx
Comment on lines
+45
to
+63
| let minX = Infinity; | ||
| let maxX = -Infinity; | ||
| for (let i = 0; i < n; i++) { | ||
| const { x } = items[i]; | ||
| if (x < minX) minX = x; | ||
| if (x > maxX) maxX = x; | ||
| } | ||
|
|
||
| const span = maxX - minX; | ||
| // Every item at the same x (or a non-finite extent): input order already is | ||
| // the tie-break order. | ||
| if (!(span > 0)) { | ||
| for (let i = 0; i < n; i++) order[i] = i; | ||
| return order; | ||
| } | ||
|
|
||
| // One bucket per item — the density that keeps buckets ~1 deep. | ||
| const bucketCount = n; | ||
| const scale = bucketCount / span; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find every orderByX caller and the origin of the `x` values it receives.
rg -nP -C6 '\borderByX\s*\(' --type=ts --type=tsx || rg -nP -C6 '\borderByX\s*\(' -g '*.ts' -g '*.tsx'
# Where packWidth / x are produced for the timeline pipeline.
rg -nP -C4 '\b(packWidth|renderWidth|estimatedPointWidth)\b' -g '*.tsx' packages/raystack/components/data-viewRepository: raystack/apsara
Length of output: 185
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- orderByX references ---'
rg -n -C 6 '\borderByX\s*\(' --glob '*.ts' --glob '*.tsx' . || true
printf '%s\n' '--- target file outline and relevant source ---'
ast-grep outline packages/raystack/components/data-view/utils/order-by-x.tsx --match orderByX --view expanded || true
sed -n '1,180p' packages/raystack/components/data-view/utils/order-by-x.tsx
printf '%s\n' '--- timeline dimension and x-value sources ---'
rg -n -C 5 '\b(packWidth|renderWidth|estimatedPointWidth|timeScale\.x|x\s*:)' packages/raystack/components/data-view --glob '*.ts' --glob '*.tsx' || trueRepository: raystack/apsara
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- timeline x construction ---'
rg -n -C 12 'timeScale\.x\(' packages/raystack/components/data-view/components/timeline.tsx packages/raystack/components/data-view/utils --glob '*.ts' --glob '*.tsx'
printf '%s\n' '--- timestamp validation and scale setup ---'
rg -n -C 8 'toTimestamp|isValid|createTimeScale|pxPerMs|domain' packages/raystack/components/data-view/components/timeline.tsx packages/raystack/components/data-view/utils --glob '*.ts' --glob '*.tsx' | head -n 240
printf '%s\n' '--- non-finite-related tests and guards ---'
rg -n -C 5 'NaN|Infinity|isFinite|finite|invalid.*time|invalid.*date' packages/raystack/components/data-view --glob '*.ts' --glob '*.tsx' || trueRepository: raystack/apsara
Length of output: 50371
🏁 Script executed:
#!/bin/bash
node - <<'JS'
function orderByX(items) {
const n = items.length;
const order = new Int32Array(n);
if (n === 0) return order;
if (n < 64) {
const plain = Array.from({length: n}, (_, i) => i);
plain.sort((a, b) => items[a].x - items[b].x || a - b);
order.set(plain);
return order;
}
let minX = Infinity;
let maxX = -Infinity;
for (let i = 0; i < n; i++) {
const {x} = items[i];
if (x < minX) minX = x;
if (x > maxX) maxX = x;
}
const span = maxX - minX;
if (!(span > 0)) {
for (let i = 0; i < n; i++) order[i] = i;
return order;
}
const bucketCount = n;
const scale = bucketCount / span;
const bucketOf = new Int32Array(n);
const starts = new Int32Array(bucketCount + 1);
for (let i = 0; i < n; i++) {
let bucket = Math.floor((items[i].x - minX) * scale);
if (bucket < 0) bucket = 0;
else if (bucket >= bucketCount) bucket = bucketCount - 1;
bucketOf[i] = bucket;
starts[bucket + 1]++;
}
for (let bucket = 0; bucket < bucketCount; bucket++) {
starts[bucket + 1] += starts[bucket];
}
const cursor = Int32Array.from(starts.subarray(0, bucketCount));
for (let i = 0; i < n; i++) order[cursor[bucketOf[i]]++] = i;
for (let bucket = 0; bucket < bucketCount; bucket++) {
const from = starts[bucket];
const to = starts[bucket + 1];
const size = to - from;
if (size < 2) continue;
for (let i = from + 1; i < to; i++) {
const index = order[i];
const {x} = items[index];
let j = i - 1;
while (j >= from && items[order[j]].x > x) {
order[j + 1] = order[j];
j--;
}
order[j + 1] = index;
}
}
return order;
}
function check(label, xs) {
const out = Array.from(orderByX(xs.map(x => ({x}))));
const counts = new Map();
for (const i of out) counts.set(i, (counts.get(i) || 0) + 1);
const permutation = out.length === xs.length &&
counts.size === xs.length && [...counts.values()].every(v => v === 1);
const finiteSorted = out.every((idx, pos) =>
pos === 0 || !(Number.isFinite(xs[idx]) && Number.isFinite(xs[out[pos - 1]]) &&
xs[idx] < xs[out[pos - 1]]));
console.log(label, {out: out.slice(0, 12), permutation, finiteSorted});
}
const finite = Array.from({length: 64}, (_, i) => i);
check('finite', finite);
check('NaN first', [NaN, ...finite]);
check('NaN middle', [...finite.slice(0, 32), NaN, ...finite.slice(32)]);
check('+Infinity first', [Infinity, ...finite]);
check('+Infinity middle', [...finite.slice(0, 32), Infinity, ...finite.slice(32)]);
check('-Infinity last', [...finite, -Infinity]);
check('both infinities', [-Infinity, ...finite.slice(0, 62), Infinity]);
JSRepository: raystack/apsara
Length of output: 1043
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- timestamp conversion ---'
rg -n 'export function toTimestamp|function toTimestamp' packages/raystack/components/data-view/utils/time-scale.tsx
sed -n '70,125p' packages/raystack/components/data-view/utils/time-scale.tsx
printf '%s\n' '--- domain extent reduction ---'
sed -n '555,625p' packages/raystack/components/data-view/components/timeline.tsxRepository: raystack/apsara
Length of output: 4692
🏁 Script executed:
#!/bin/bash
node - <<'JS'
function bucketOrder(xs, guard) {
const n = xs.length;
const order = new Int32Array(n);
let minX = Infinity;
let maxX = -Infinity;
for (let i = 0; i < n; i++) {
const x = xs[i];
if (x < minX) minX = x;
if (x > maxX) maxX = x;
}
const span = maxX - minX;
if (guard(span, xs)) return Array.from({length: n}, (_, i) => i);
const scale = n / span;
const bucketOf = new Int32Array(n);
const starts = new Int32Array(n + 1);
for (let i = 0; i < n; i++) {
let bucket = Math.floor((xs[i] - minX) * scale);
if (bucket < 0) bucket = 0;
else if (bucket >= n) bucket = n - 1;
bucketOf[i] = bucket;
starts[bucket + 1]++;
}
for (let b = 0; b < n; b++) starts[b + 1] += starts[b];
const cursor = Int32Array.from(starts.subarray(0, n));
for (let i = 0; i < n; i++) order[cursor[bucketOf[i]]++] = i;
return Array.from(order);
}
function report(label, xs, guard) {
const out = bucketOrder(xs, guard);
const seen = new Set(out);
const permutation = seen.size === xs.length && out.every(i => i >= 0 && i < xs.length);
const values = out.map(i => xs[i]);
console.log(JSON.stringify({
label, output: values.slice(0, 8).concat(values.slice(-3)),
permutation,
outputIndices: out.slice(0, 8).concat(out.slice(-3))
}));
}
const finite = Array.from({length: 64}, (_, i) => i);
const cases = [
['NaN first', [NaN, ...finite]],
['NaN middle', [...finite.slice(0, 32), NaN, ...finite.slice(32)]],
['+Infinity first', [Infinity, ...finite]],
['+Infinity middle', [...finite.slice(0, 32), Infinity, ...finite.slice(32)]],
['-Infinity last', [...finite, -Infinity]]
];
for (const [label, xs] of cases) {
report(label + ' / current', xs, span => !(span > 0));
report(label + ' / proposed span guard', xs,
span => !(span > 0) || !Number.isFinite(span));
report(label + ' / any non-finite identity fallback', xs,
(_span, values) => values.some(value => !Number.isFinite(value)));
}
JSRepository: raystack/apsara
Length of output: 2379
Handle non-finite x values before bucket sorting.
Checking span is insufficient. Mixed NaN values can produce duplicate or missing indices, while mixed infinities can produce an unsorted identity order. Detect non-finite x values and use a comparison fallback with explicit ordering.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/raystack/components/data-view/utils/order-by-x.tsx` around lines 45
- 63, Update the ordering logic around the minX/maxX scan and bucket sorting to
detect any non-finite x value before computing buckets. When present, use a
comparison-based fallback with explicit ordering for finite values, positive and
negative infinity, and NaN, while preserving input order for equal x values;
retain bucket sorting only when every x is finite.
`orderByX` buckets by `Math.floor((x - minX) * scale)`. For a NaN that expression is NaN, and the two guards only caught negatives, so it fell through: `bucketOf` is an Int32Array and stored it as 0, while `starts[NaN + 1]++` was a silent no-op on a typed array. Bucket 0 then received an item it had never reserved a slot for and the scatter overwrote its neighbour — one index duplicated, one lost. Downstream that is one card packed into a lane twice and another left unplaced. `colOf` in packLanes has the same shape. There a NaN column reads undefined and writes nothing, so the lane is filed into no column and never released — a leak rather than a lost card, but the same cause. Both guards are now negated (`!(bucket >= 0)`), which catches NaN and is identical to the old comparison for finite input — all six recorded lane goldens are unchanged. Not reachable from the timeline today: `toTimestamp` returns null for anything non-finite and the renderer skips those rows. This is hardening of exported utils against a caller that doesn't filter. Coverage lands in the following commit.
`packLanes` and `orderByX` were tested from inside timeline.test.tsx, which had grown to 2,060 lines covering three unrelated subjects. Moves them to pack-lanes.test.ts and order-by-x.test.ts, with the shared LCG, FNV-1a digest and item generator in helpers.ts so the recorded goldens keep being built from byte-identical input. Pure relocation for the existing 18 — the six inline snapshots are transcribed verbatim and still match. New coverage, aimed at the paths nothing reached: - lanes above 1024, where the free-lane bitmap's summary is two-level and every prior test topped out at 400 lanes, leaving the multi-block walk in `takeSmallestFree` unexercised. Widths shrink as x grows so high lanes free first, forcing the search past an all-zero block-0 summary. - both sides of each implementation threshold (63 vs 64 items), since both utils pick between two algorithms by item count and nothing pinned that they agree at the boundary — a mismatch means one extra card silently repacks every lane. - non-finite x and width, which is what surfaced the bug fixed in the previous commit. - gapPx of 0, zero-width items, entirely negative coordinates, and a single item. 187 tests pass, up from 176.
`takeSmallestFree` walks the bitmap summary to find the lowest free lane, which is O(lanes / 1024). When every lane is occupied it walks all of it and returns -1, and a saturated sweep does that once per item to learn the same thing every time — 50k cards on a single date is ~2.45M iterations of pure no. Counting the free lanes turns the empty case into a comparison. Lane assignment is untouched: the counter only short-circuits a search that was already going to fail. Measured, median of 7, packLanes end to end: clustered 50k 13.95ms -> 1.14ms all-overlap 20k 2.98ms -> 0.59ms realistic 50k/1y 6.51ms -> 6.27ms (unchanged, as expected) Realistic input never noticed because a free lane is nearly always in the first block. The shapes that pay are the degenerate ones — every card on one date, or spans long enough that nothing ever frees — which is exactly where a linear scan per item is least affordable.
Two claims went stale when culling gained a vertical axis: - "Vertical space is not virtualized. `virtualized` culls horizontally only" is now false in both halves. Replaced with what it actually does (cards, gridlines, ticks, bands, markers, and the line spans) plus the fact that it still defaults to false, which the old text never said. - The Cards section documented content-driven lane heights unconditionally. That only holds unvirtualized: a culled card never reports a height, so measuring would resize lanes mid-scroll. Virtualized lanes take a fixed `estimatedRowHeight` pitch and a taller card overlaps the lane below — the one behaviour change in this PR a consumer can trip over. The unvirtualized path keeps its own bullet rather than being dropped, since content-driven lane heights are a real reason to choose it.
`useFrameMonitor` took a ref, and the pane is found by query inside a requestAnimationFrame after the timeline paints. Assigning a ref does not re-run an effect and both deps were stable, so the effect saw `paneRef.current === null`, bailed, and never added its scroll listener. "Worst frame" read "scroll me" for the entire session — the one number the harness exists to produce. It now takes the element and holds it in state, so the effect re-runs once the pane exists. Verified: after scrolling it reports 23ms at 10k, matching an independent PerformanceObserver measurement of the same run. No measurement reported in this PR came from that readout; those were taken with PerformanceObserver directly, so the numbers stand. Two smaller fixes in the same hook: - `until` was assigned at declaration and overwritten before the loop started. The first assignment was dead. - The comment claimed the sampling window extends while the user keeps scrolling. It did not: the guard made every scroll during an active loop a no-op and `until` was captured per invocation, so it sampled a fixed second from the first event and ignored the rest of a drag. Now implemented as documented, via a ref the guard updates before it returns. Also hoists the `markers` literal, which was invalidating the timeline's resolved-markers memo on every commit — churn this page would otherwise report as the canvas's own cost.
Two comments still described a full viewport of overscan on each side. `OVERSCAN_RATIO` was halved to 0.5 in "cull timeline axis chrome" and these were not updated with it, so the stated justification no longer matched the constant it justified. Comment-only; the quantization math is unchanged and still safe, since the commit threshold is a quarter viewport of travel against half a viewport of coverage.
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
1 participant
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.
Description
Makes
DataView.Timelineusable at 10k–50k rows. Seven commits, each independently reviewable:perf(data-view): virtualize the timeline vertically— culling was horizontal only, so every lane in the domain stayed mounted. Adds a vertical window, a per-lane CSR card index, and fixed lane heights undervirtualized.perf(data-view): cull timeline axis chrome, cut per-scroll work— ticks, bands, markers and gridlines were rendered across the whole domain. Also quantizes the viewport so scrolling commits once per half-overscan instead of every frame, and decouplesonVisibleRangeChangefrom render.perf(data-view): clamp timeline grid lines to the viewport— the actual dominant cost, see below.chore(www): add a 5-year span to the timeline stress harness— QA harness only.fix(data-view): stop a non-finite x from dropping a card— latent bug inorderByX, found by the tests in 6.test(data-view): split the util suites out and cover the gaps— dedicated files forpackLanes/orderByX, plus the untested paths.perf(data-view): skip the free-lane search when no lane is free— degenerate-input hot spot, found by benchmarking after 6.Demo
Screen.Recording.2026-08-12.at.10.59.57.AM.mov
The saturated-sweep hot spot
takeSmallestFreewalks the bitmap summary in O(lanes / 1024). When every lane is occupied it walks all of it and returns-1, once per item — 50k cards on a single date is ~2.45M iterations that always answer no. A free-lane counter short-circuits it; lane assignment is untouched.packLanes, median of 7Realistic input never noticed — a free lane is nearly always in the first block. The shapes that pay are every card on one date, or spans long enough that nothing frees.
The gridline finding
Grid, marker and cursor lines are pinned
top: 0; bottom: 0, so each was a one-pixel-wide element as tall as the entire canvas — 28,798px at 10k rows, 139,170px at 50k. Rasterizing a dashed sub-pixel border over that height costs the same whether one card is on screen or four hundred, which is why culling their count in commit 2 (183 → 31) never moved frame time.This explains three earlier observations that culling could not: cost flat against the rendered set, a taller canvas measuring worse with fewer cards, and
contain: layout painthaving no effect.The
orderByXbugbucketOfis anInt32Array, so a NaN bucket index stored as0, while the matchingstarts[NaN + 1]++was a silent no-op on a typed array. Bucket 0 then received an item it had never reserved a slot for and the scatter overwrote its neighbour — one index duplicated, one lost, meaning one card packed into a lane twice and another left unplaced.Not reachable from the timeline:
toTimestampreturnsnullfor anything non-finite and the renderer skips those rows. Fixed anyway, since both are exported utils. The guard is now!(bucket >= 0), identical to the old comparison for finite input — all six lane goldens unchanged.Complexity work in commits 1–2
packLanesLane assignment is byte-identical throughout — six golden digests recorded from the pre-rewrite implementation still match.
Type of Change
How Has This Been Tested?
Unit — 187 tests pass. Written before the changes as characterisation tests: six
packLaneslane-assignment digests plus DOM-order and chrome goldens recorded from the unmodified implementation, so any behaviour drift fails the suite. Commit 6 adds coverage for the paths nothing reached — lanes above 1024 (the bitmap summary is two-level; every prior test topped out at 400 lanes), both sides of each implementation threshold at 63 vs 64 items, non-finite geometry,gapPxof 0, zero-width items, and entirely negative coordinates.Browser — measured on
/examples/timeline-stress, scrolling both axes, 89 sampled frames, dev build:Both sides ran in one session on one machine — the "before" rows were produced by injecting the old geometry back over the fixed build, so nothing is compared across runs or machines.
Also verified the clamped lines still cover the pane after a deep scroll (span −59→1391 against a 252→976 pane, margin on both sides exceeding the quantization slack), and that 1-year and 5-year spans render correctly at 50k.
Checklist:
Notes for the reviewer
Docs are not updated yet.
apps/www/src/content/docs/components/dataview/index.mdx:557still says "Vertical space is not virtualized.virtualizedculls horizontally only" — now false.virtualizedstill defaults tofalse. Flipping it was planned but is deliberately not in this PR: combined with fixed lane heights it would change rendering for every existing consumer, since a card taller thanestimatedRowHeight(default 66) would overlap the lane below instead of growing its own. Worth deciding separately.No regression test covers the gridline clamp itself — jsdom reports no geometry, so the guard would only assert that inline
height/bottomare set, not that they are correct. Happy to add that if you want the shape locked in.🤖 Generated with Claude Code