perf(diff-viewer): virtualize diff body rendering for large diffs#773
Conversation
Add info-level logs along the diff-open path so slow clicks can be attributed to the right stage. Frontend logs cover modal open, loadFiles, loadFileDiff (including cache hits), and switchContext with elapsed millis. Backend fills in the previously-silent remote cache hit paths and the remote cache-miss collection step in get_diff_files and get_file_diff. Signed-off-by: Matt Toohey <contact@matttoohey.com>
The existing diff-open instrumentation brackets the wrong window: it starts at "[diff] open" (DiffModal init) and stops at computeLineDiff, leaving two uninstrumented gaps where the perceived latency actually lives. Close both. Click→open: markDiffOpenClick() stamps performance.now() at every diff-open trigger in BranchCard (Diff button, review/commit/worktree opens). createDiffViewerState logs the delta to "[diff] open" as clickToOpen, capturing the DiffModal + DiffViewer mount cost. Render tail: a guarded $effect in DiffModal times from a file's diff becoming available through to pixels on screen via a double-rAF, so syntax highlighting, DOM build, and layout/paint after computeLineDiff are no longer invisible. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Phase 1 of virtualizing the diff body: decouple anchor positioning from
the assumption that every line is in the DOM, so it stays correct once
the body renders only a window of lines.
Stamp every rendered .line with data-line-index={i} (absolute index) in
all four pane {#each} blocks, and add a lineAt(pane, n) helper that
resolves a line by that attribute. Convert the positional NodeList
lookups (querySelectorAll('.line')[n]) in updateToolbarPosition,
decideCommentPosition, updateCommentEditorPosition,
updateLineSelectionToolbar, decideLineCommentPosition,
updateLineCommentEditorPosition, and the range-copy loop to identity
lookups via lineAt.
Replace handleSelectionDragMove's rect hit-test loop with arithmetic
(floor((clientY - paneTop + scrollY) / lineHeight), clamped) so the drag
focus line is derived from scroll math instead of iterating rendered
rows — correct and faster even when rows are unrendered.
No windowing yet: this is a correctness-preserving change against the
current full render. Comment open/jump, range-selection toolbar, and
drag-select behave identically. Typecheck passes.
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Phase 2 of virtualizing the diff body: render only the lines near the
viewport instead of the entire file, building on Phase 1's index-by-
identity refactor.
Add per-pane window deriveds (beforeWindow/afterWindow) computed from the
scroll offset, line height, and viewport height the scroll controller
already owns: firstVisible = floor(scrollY / lineHeight), then a slice
[firstVisible - OVERSCAN, firstVisible + visibleCount + OVERSCAN) clamped
to the line count. computeWindow returns absolute indices, so iterating
beforeWindow.indices keeps every i-consumer (highlighting, boundaries,
class lookups, mouse handlers) unchanged — i is still the absolute line
index. A keyed {#each ... (i)} lets Svelte reuse rows across scrolls.
Position the slice under the existing -scrollY transform with a single
top spacer (.line-spacer) sized windowStart * lineHeight; no bottom
spacer is needed because the scrollbar height comes from the separate
contentHeight derived, not the node count.
To keep the window reactive on resize and font changes (not just scroll),
track lineHeight and viewportHeight per pane as $state, updated wherever
dimensions are already measured. OVERSCAN is 40 rows — generous enough to
cover the comment editor's vertical extent near a viewport edge.
Applied to all four line {#each} blocks: two-pane before/after, deleted-
file before, and new-file after. scrollToRow, markers, and connector
rendering are untouched (they read lengths/offsets, not nodes).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
…ents Phase 3 of virtualizing the diff body: make the comment editor and line- selection toolbar wait for the windowed body to render their anchor rows before measuring, instead of assuming the rows exist on the next frame. After Phase 2, scrolling to a comment sets a pane's scroll offset, which recomputes the window and renders the target rows — but only on a later frame. focusCommentInViewer's single requestAnimationFrame could fire before the anchor mounted, so lineAt() returned null and the editor silently never appeared. The same one-shot defer backed handleCommentHighlightClick's fallback and handleLineMouseUp's toolbar. Replace those one-shot rAFs with a bounded mount-then-measure retry: measureAfterWindowRender ticks a measure callback across up to WINDOW_RENDER_MAX_FRAMES (5) frames, stopping as soon as it reports success (anchor resolved and positioned) or there is nothing to position. A trackRaf callback lets a newer request cancel an in-flight wait, and both pending frame ids are cancelled on unmount. positionLineCommentEditor requires both the first and last range rows to be mounted before running, since decideLineCommentPosition measures space-above/space-below from both and the editor can render above the range. This also moves the position decision off the synchronous path in focusCommentInViewer and handleCommentHighlightClick, where it previously measured against unmounted rows and fell back to 'below'. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 673271b198
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| <div | ||
| class="line-spacer" | ||
| style="height: {afterWindow.start * afterLineHeight}px" | ||
| ></div> | ||
| {#each afterWindow.indices as i (i)} |
There was a problem hiding this comment.
Preserve horizontal width across virtualized lines
When a file has its longest line outside the currently rendered window, measureContentWidth() now only sees the windowed DOM under .lines-wrapper, so contentWidth is set to the max width of the visible slice rather than the whole file. The scroll controller clamps horizontal scrolling to that smaller width, which means users cannot pan far enough to read long offscreen lines until they vertically scroll them into the window. This affects both panes because the body is now rendered from beforeWindow/afterWindow instead of all lines.
Useful? React with 👍 / 👎.
| for (let i = selectedLineRange.start; i <= selectedLineRange.end; i++) { | ||
| const lineEl = lineElements[i]; | ||
| const lineEl = lineAt(pane, i); | ||
| if (lineEl) { |
There was a problem hiding this comment.
Copy selected lines from data, not mounted rows
With virtualization, a selected range can include rows that are no longer mounted after the user scrolls before copying, so lineAt(pane, i) returns null and those selected lines are silently omitted from the clipboard. Previously every .line existed in the DOM, but after this change copying should read from beforeLines/afterLines (or another non-virtualized source) so offscreen selected lines are still copied.
Useful? React with 👍 / 👎.
The diff-body windowing (b45c465 → 673271b) renders only the rows near the viewport, but three spots still assumed every row was in the DOM. Two were real bugs for ranges taller than the window (~OVERSCAN + viewport); one was dead CSS. Range copy: handleCopy's selectedLineRange branch read line text from the DOM via lineAt, so any selected line outside the rendered slice resolved to null and was silently dropped — a drag-selection spanning more than the window copied only a partial range. Reconstruct the copied text from the beforeLines/afterLines source arrays by index instead (absolute line index maps 1:1 to the source-line array). The native-selection branch stays DOM-bound, as a browser selection can't extend to unrendered rows anyway. Comment editor: positionLineCommentEditor required both the first and last range rows mounted before measuring, because decideLineCommentPosition reads space-above from the first row and space-below from the last. For a range taller than the window the two ends can never co-mount, so it spun out WINDOW_RENDER_MAX_FRAMES and the editor silently never appeared. Now both-ends-mounted keeps the full space-based decision (short ranges), and the tall-range case anchors to whichever end is rendered — callers scroll to start first, so it lands in the window — passing the resolved row to updateLineCommentEditorPosition so the anchor isn't re-derived from the preference against an unmounted row. CSS: drop the inert flex-shrink: 0 on .line-spacer; its wrapper is display: block so it never applied, and the inline height already reserves the unrendered-rows space. Typecheck passes. Signed-off-by: Matt Toohey <contact@matttoohey.com>
… window The diff-body virtualization (b45c465 → 26e6287) renders only the rows near the viewport, but measureContentWidth still read the widest line from the DOM via .lines-wrapper's scrollWidth. With most rows unmounted, that saw only the widest *rendered* line, so a file whose longest line sat outside the window clamped horizontal scrolling too narrow — long offscreen lines couldn't be panned into view, and the horizontal scrollbar thumb was mis-sized. Because updateContentWidths runs on lines change / resize / style mutation but not on scroll, scrolling the long line into the window didn't fix it either; the under-clamp persisted until an unrelated resize or font change. Measure from the source text instead. measureContentWidth now takes the pane's source lines, picks the widest by display-column count (tabs expanded to 8-column stops, iterated by code point), and measures just that one line in a hidden offscreen probe span appended to the pane so it inherits the live computed font and white-space: pre. Add the 24px .line-content horizontal padding and floor at pane.clientWidth to keep the .lines-wrapper min-width: 100% behavior. One probe node keeps the virtualization intact while the result reflects the whole file at any scroll position. Applied to all four measurement sites (before/after dimension effects and updateContentWidths) by passing beforeLines/afterLines. Empty panes still return 0. Typecheck passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Summary
Speeds up rendering of large diffs in the diff viewer by virtualizing the diff body — only a window of visible lines is rendered to the DOM at a time, rather than the entire diff.
Changes
DiffViewer.svelte): render only a window of diff lines around the viewport instead of the full diff body.diffViewerState.svelte.ts): index diff lines by identity so the rendered window can be tracked and mapped correctly.diff_commands.rs,DiffModal.svelte,BranchCard.svelte): log diff load timing on both frontend and backend, plus click→open and post-render timing, to measure the performance improvements.🤖 Generated with Claude Code