fix(bench): measure row-height error on recorded frames only - #456
Merged
Conversation
Closes #455. `sampleVisibleRows` measured each visible row's height error eagerly, and that measurement resolves `getComputedStyle` on EVERY cell of the row (the `canCellOverflowVertically` filter). Every polled frame paid it: the pre-trigger quiet wait and the trigger frame included, whose errors are then discarded. Per poll that is 72 forced style resolutions for pretable, 640 for tanstack, 943 for mui — inside the very window the harness is timing, so `interaction_latency_ms` taxed adapters in proportion to their rendered DOM. Measured on S2/sort: the harness consumed 8.8% of pretable's interaction window and 34.6% of TanStack's. The samples now carry the row element and its rendered height, and `recordRowHeightErrors` measures lazily — the cost lands on exactly the frames whose errors are recorded, in the same task as the sample, so the DOM state read is the frame's own. After the fix the harness share is 4.3% and 20.3% respectively (the remainder is the settle signature itself), TanStack's measured window shrinks 15.2 -> 13.6ms, and the adapter-work ratio the metric exists to expose is unchanged at ~3.2x — which is the point: the fix moves the observer tax, not the comparison. The new test counts cell style resolutions on either side of the trigger and asserts three things at once: the quiet wait resolves none, the measured window still resolves some, and `measurable_rows` still reports — so deleting the measurement outright cannot pass. Mutation-checked both ways: restoring the eager measurement reddens the counting arm; starving the measurement reddens that test plus two pre-existing metric tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
Vercel preview readyPreview: https://pretable-dari2ijyq-cacheplane.vercel.app Updated automatically by the |
This was referenced Aug 17, 2026
blove
added a commit
that referenced
this pull request
Aug 17, 2026
Closes #458. interaction_latency_ms latches the first changed rAF frame, and under that definition a synchronous engine that blocks the main thread for its whole sort and a cooperative one that never blocks were indistinguishable — the interaction path collected no long-task metric at all (the scroll and updates paths already did). Every comparative interaction table published so far was structurally blind to the one property pretable pays its latency premium for. The observer attaches AT the trigger, not at function entry: a long task during the pre-trigger quiet wait is mount tail, and charging it to the interaction would poison the metric. It is a push-based PerformanceObserver, so unlike the height-error walk removed in #456 it adds no per-frame DOM work. Emitted as post_interaction_long_tasks_count / _ms, absent (not zero) when the host cannot observe long tasks — the same absent-vs-zero rule as the row-height error. First measurement, S2 sort, 3 repeats each: 3,000 rows: pretable 0 long tasks | tanstack 0 (its ~15ms block is under the 50ms longtask threshold) 50,000 rows: tanstack reports ~33ms latency while blocking the main thread for 62ms (1 task, 3/3 runs, sd <1ms) The test plays synthetic entries into a stubbed observer at three moments — during the run's own quiet wait, inside the trigger, and asserts teardown. Mutation-checked: attaching at function entry, never disconnecting, and dropping the metrics each redden it. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.
Closes #455.
The bug
sampleVisibleRowsmeasured each visible row's height error eagerly, on every polled frame — and that measurement resolvesgetComputedStyleon every cell of the row (canCellOverflowVertically). The pre-trigger quiet wait and the trigger frame paid it too, then discarded the result. Per poll:All inside the window the harness is timing — so
interaction_latency_mstaxed adapters in proportion to their rendered DOM. Measured on S2/sort: 8.8% of pretable's window was the harness; 34.6% of TanStack's.The fix
The sample carries the row element and its rendered height;
recordRowHeightErrorsmeasures lazily, on exactly the frames it records. Recording happens in the same task as the sample, so the element is still attached and the DOM state read is the frame's own — the measurement's semantics are unchanged, only its placement.No flag to get backwards: the eager/lazy split I sketched on the issue had a boolean threaded through a shared
sample()closure, where a wrong value in one direction silently starves the p95. This shape can't express that mistake.Effect, measured
Same protocol as the finding (source-map-classified samples in the interaction window):
The remainder is the settle signature itself (
rowId@topper row), which is the detector's actual job. And the number the metric exists to expose — the adapter-work ratio — is unchanged at ~3.2×, which is the point: this moves the observer tax, not the comparison. #452's gap stands.Verification
The new test counts cell style resolutions on either side of the trigger and asserts three things at once: the quiet wait resolves none, the measured window still resolves some, and
measurable_rowsstill reports — so deleting the measurement outright cannot pass.Mutation-checked both ways:
heightError = null)155 bench tests, typecheck, lint, format.
Not done here
The interaction baselines in
2026-08-16-s2-mount-and-interaction.jsonwere measured with the old tax and are not re-run — their tick distributions carry an adapter-proportional observer cost, worst for MUI. Worth a re-measure in the next quiet window; the file's own findings already lean on the adapter-work ratio, which this PR shows is stable.🤖 Generated with Claude Code