perf(desktop): virtualize message timeline to stop the cold-switch beachball#1110
Closed
wpfleger96 wants to merge 3 commits into
Closed
perf(desktop): virtualize message timeline to stop the cold-switch beachball#1110wpfleger96 wants to merge 3 commits into
wpfleger96 wants to merge 3 commits into
Conversation
…achball
Channel switch streamed up to 200 uncontained MessageRows (each with
synchronous shiki markdown), then scrollToBottom("auto") forced a
full-document scrollHeight read-then-write reflow before paint over
every row — the macOS beachball Will reported on v0.3.25.
Windows the main timeline on @tanstack/react-virtual. The day-grouped
section tree is flattened to a typed TimelineItem[] stream plus a
messageId->itemIndex map from one walk (cannot drift), and every
DOM-querySelector scroll path (deep-link, search-jump, jump-to-unread,
scrollToBottom, load-older anchor) is re-pathed onto the index model so
windowing does not silently break jumps to off-screen rows.
Scroll convergence is split: @tanstack/react-virtual owns offset
convergence (its rAF loop re-aims getOffsetForIndex as rows mount and
measure); a pure reducer owns only staleness re-resolution and
termination — re-resolving the target's index by id each frame so a
concurrent prepend/delete cannot strand the loop on a stale index, and
terminating when the target is deleted or a 32-frame cap is hit. The
breaking math lives in lib/ under the .mjs suite.
The thread reply list stays content-visibility:auto rather than
virtualized — it is bounded, unpaginated, ungrouped, and shares the
scroll hook, so virtualizing it would force a second index re-path and a
head/prologue split for no beachball gain. Phase-2 route-chunk preload
warms the agents/channel/lazy-view chunks on idle to clear the
Agents-menu first-visit stall.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
… anchor Loading older messages under virtualization let three writers fight over scrollTop on overlapping frames, so the anchored row jittered or collapsed to the top (~33% of prepends) and the library's reconcile spun the full 5s MAX_RECONCILE_MS valve. Establish a single owner of scroll position across the whole fetch+restore window: - useLoadOlderOnScroll restores by scrollTop ONLY (drop scrollToIndex), via one getOffsetForIndex(anchorIndex + prepended, "start")[0] + intra-row gap write. getOffsetForIndex is a pure measurement-cache read, so no library scrollState is set and the reconcile loop has nothing to fight. - The viewport ResizeObserver in useTimelineScrollManager no longer runs a competing restore during a fetch: it skips while isFetchingOlder is true (the spinner's clientHeight 720->590 mount-shift fires before the lock is set) and otherwise defers to lockedScrollTopRef when the load-older restore holds it. MessageTimeline threads isFetchingOlder into the manager. The defect was invisible to unit tests (jsdom getBoundingClientRect -> 0) and to static traces; the new load-older E2E drives a real prepend on six fresh page loads and asserts the anchor holds every run, the scroller genuinely grew, and the reconcile terminates. emitMockHistory now honors the relay filter's until/limit so the mock relay paginates like a real one, which the E2E needs to exercise a genuine older page. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
biome check enforces line-wrapping that biome lint does not. The load-older test 07 had two over-width statements that passed local lint but failed the Desktop Core biome check gate. Format-only, no behavior change. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Collaborator
Author
|
Superseded by #1123 — the hybrid work (virtualization + single-owner anchored scroll + role-3 convergence + spinner slot) lives there, rebased onto eva and now CI-unblocked. |
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.
buzz beachballs on every main-nav transition (channel switch, Home/inbox, Agents menu) and on the first thread-open after a channel switch on
v0.3.25. The synchronous block is the message timeline: a channel switch streams up to 200 (CHANNEL_HISTORY_LIMIT) uncontainedMessageRows — each with synchronous shiki markdown — thenuseTimelineScrollManagerfiresscrollToBottom("auto")from auseLayoutEffect, andalignToBottomdoes a synchronousscrollHeightread-then-write that forces a full-document reflow over all 200 rows before paint. The Agents-menu stall is a separate cause: aReact.lazyroute chunk with no preload.This windows the main timeline on
@tanstack/react-virtual, eliminating the reflow at its root, plus a route-chunk preload for the lazy stall.What changed
Flatten the heterogeneous tree. The day-grouped
<section>tree is flattened to a typedTimelineItem[]discriminated-union stream (day-divider/unread-divider/system/message/thread-summary) in a pure lib builder. The same single walk produces themessageId -> itemIndexmap, so the stream and the map cannot drift. A singleuseMemokeyed on[deferredMessages, firstUnreadMessageId]owns both —firstUnreadMessageIdis load-bearing because theunread-divideritem shifts indices.Re-path every scroll path onto the index model. Windowing removes off-screen rows from the DOM, so the old
querySelector('[data-message-id]')lookups would silently fail for any off-screen target.scrollToMessage, in-channel search-jump, jump-to-oldest-unread,scrollToBottom, and the load-older prepend anchor now resolve through the index map instead of the DOM. Load-older captures the first-visible item index before fetch and re-aims atoldFirstIndex + Nafter the prepend, restoring the intra-row offset.Split scroll convergence.
@tanstack/react-virtualowns offset convergence — its internal rAF loop re-runsgetOffsetForIndexas off-screen rows mount andmeasureElementcorrects their estimated heights. A pure reducer (scrollConvergence.ts) owns only what the library cannot: it re-resolves the target's index by id every frame, so a concurrent prepend or delete that shifts the target re-aims the library instead of stranding it on a stale index; it terminates withconverged: falseif the target is deleted mid-settle, and caps at 32 frames. A thin rAF adapter (useConvergentScrollToMessage.ts) drives the virtualizer and re-issuesscrollToIndexonly when the index moves, so a steady settle does not reset the library's stable-frame counter.Thread reply list gets
content-visibility: autorather than virtualization. It is bounded (replies to one root), unpaginated, ungrouped, and sharesuseTimelineScrollManager— virtualizing it would force a second index re-path on the shared hook and a head/prologue split for no beachball gain.content-visibility: autoskips layout for off-screen reply rows while keeping them DOM-resident, so the shared deep-link path still works there.Route-chunk preload (Phase 2).
agents.tsx,channels.$channelId.tsx, andChannelScreenLazyViews.tsexportpreload*()for theirReact.lazychunks; a single idle-gated effect inAppShell.tsxwarms all three once startup is ready, clearing the Agents-menu first-visit stall.Tests
The breaking math lives in
lib/under the.mjssuite: convergence within the 32-frame cap on mixed measured/estimated heights, landing within tolerance, termination when a row keeps re-measuring, target-removed-mid-settle terminating withconverged: false, and the staleness re-resolve. The builder tests gate divider placement (3-day channel, unread divider mid-day-2) and map index correctness across new-message / prepend / delete.