Isolate scroll-driven virtualization updates - #4972
Conversation
7eefba1 to
efa27de
Compare
efa27de to
13a6a62
Compare
13a6a62 to
a6586e7
Compare
|
Thanks for this. @trevinhofmann FYI This has direct consequences for #4735. |
Co-Authored-By: Codex GPT-5 (unknown context) <noreply@openai.com>
Co-Authored-By: Codex GPT-5 (unknown context) <noreply@openai.com>
a6586e7 to
76bcd8e
Compare
Exercise Safari elastic overscroll and the real viewport resize path, verify size-map cleanup semantics, and cover document-height restoration with real browser scrolling. Co-Authored-By: Codex GPT-5 (unknown context) <noreply@openai.com>
Co-Authored-By: Codex GPT-5 (unknown context) <noreply@openai.com>
|
(Below written by AI and some minor modifications by me) Additional physical-device validation: I reran the exact pre-/post-patch comparison using a 150-level Galaxy S22 / Chrome 150Three paired PRE/POST blocks completed successfully with no retries. Every sample executed 99 Forward + 99 Back commands and 396 controlled scroll calls.
All three pairs favored the PR. Idle controls did not show a comparable CPU advantage. Memory endpoints were essentially unchanged, so no memory improvement is claimed. iPhone 13 mini / Mobile SafariAll seven retained app workloads completed the full navigation and returned to However monitoring of Safari was problematic for some reason and doing many runs only bench gave interpretable results:
One pair is insufficient and was ordered POST→PRE, so the iPhone result is inconclusive. This supersedes the earlier iPhone Overall, the added testing provides strong evidence of a targeted deep-tree performance improvement on Android, while making no broad cross-device or memory-performance claim. |
Co-Authored-By: Codex GPT-5 (unknown context) <noreply@openai.com>
Follow-up to #4969.
Problem
LayoutTreesubscribes to a computedviewportBottomderived fromscrollTop. Every scroll update therefore rerendersLayoutTree,TransitionGroup, and the full thought list even when no thought crosses the virtualization boundary.Exact differential testing also exposed a related bookkeeping dependency:
useSizeTrackingdeleted unmounted size entries by mutating the existing map. The old scroll-wide rerender eventually observed that mutation. Once the unnecessary parent rerender was removed, reverse virtualization could retain stale document height.Solution
TreeNodeand select only the booleanisBelowViewport.LayoutTree.LayoutTreeupdates when—and only when—its layout inputs actually change.Most scroll updates leave every node boolean unchanged, so they do not propagate through
LayoutTree,TransitionGroup, or the full thought list. Nodes still update when they enter or leave the virtualized region.The virtualization cutoff and visible UI are unchanged. Size cleanup is now immediate instead of relying on a later incidental render, which also removes transient phantom scroll space.
Correctness validation
The regression coverage exercises the real browser scroll-event path and verifies:
A focused Puppeteer regression uses real wheel input to mount and measure a wrapped thought, then returns to the top and waits for it to be virtualized again. On the original exact PR base (
b6902cf3), document height stayed stale at1776 → 1875 → 1875px; this PR restores it to1776 → 1875 → 1776pxand passes the final equality assertion.The production differential compared exact mounted thought values, ordering, editables, shims, bounds, transforms, opacity, breadcrumbs, nav/footer geometry, scroll height, and screenshots:
On an iPhone 13 mini running iOS 26.6, five complete top-to-bottom-to-top cycles of the 180-sibling fixture repeated the same top/bottom scroll heights and mounted counts. The WebContent PID and
performance.timeOriginstayed constant, no runtime error occurred, and the historical mount/unmount loop did not reproduce.Performance
The production Chrome workload imports a 150-deep path, sets the final cursor directly, lets the layout settle, and scrolls through 75 distinct positions. The Chrome results below were rerun after updating this branch through upstream
ebdfdef83, so both variants include the related VirtualThought cleanup from #4975.Chrome samples used fresh production-browser processes, one discarded warmup per variant, alternating run order, and no React hook, geometry monkeypatch, or long-task instrumentation. Task rMAD was 1.83%/1.67% and renderer-CPU rMAD was 1.78%/0.92% for baseline/candidate.
A separate instrumented run confirmed that React commits,
LayoutTreerenders, and nav/footer geometry reads during the 75-step deep-path scroll all dropped to zero. Both variants retained identical aggregate DOM/listener counts, editable counts, scroll range, and sampled positions; the exact differential above separately verifies node identity and visual/layout semantics.The physical result is corroboration from one iPhone 13 mini, not a general iOS benchmark. It predates the latest upstream-main refresh; the #4972 runtime files are unchanged, but the combined updated head was not rerun on the device. This synthetic workload isolates post-settle scrolling; it does not measure progressive-navigation speed or animation quality. No memory improvement is claimed.
In a prior candidate-only depth-scaling run, depth 500 completed the same scroll workload with 413.2 ms task time and 644.9 ms renderer CPU. This confirms remaining depth-dependent selector cost, but the increase was sub-proportional to the 3.33x increase in depth.
I also remeasured the opposite stress case on current main: 180 variable-height siblings, scrolling fully down and back so rows repeatedly cross the virtualization boundary. Across six balanced alternating production runs, renderer CPU increased 6.3% (3233.7 to 3437.4 ms), task time increased 4.2% (2071.3 to 2158.7 ms), and layout time increased from 70.0 to 90.2 ms, while script time decreased 4.3%. Every run matched the initial/final DOM shape, 7229px document height, 6565px scroll range, 150 positions, and final scroll position 0. This workload deliberately maximizes mount/unmount churn. The extra layout work comes from immediately publishing removed measurements; omitting it is faster but leaves stale document height, as the differential test above demonstrated. The optimization therefore strongly improves deep-path scrolling without claiming a universal win for every scroll shape.
Verification
yarn test --run src/hooks/__tests__/useSizeTracking.ts src/components/__tests__/LayoutTree.virtualization.ts src/components/__tests__/LayoutTree.tsyarn lint:tsc --pretty falseyarn lint:srcyarn build