perf(web): mount session row dialogs only while they are open - #1233
Merged
Conversation
Every SessionItem rendered its rename dialog and both confirm dialogs unconditionally, passing isOpen to keep them closed. With 28 sessions that is 84 mounted Radix dialogs that the user cannot see, and each one carries a Presence/DialogProvider/DialogContent/DialogPortal subtree that re-renders whenever the row re-renders. Measured against a live hub, dialogs accounted for roughly half of the whole fiber tree: 1680 nodes before, 886 after. A single machine-updated event - which arrives every 20s per machine and only carries changed CPU and memory numbers - was re-rendering over 1600 dialog-internal components. The same file already mounts SessionExportDialog and the reopen-error ConfirmDialog on demand, so this only makes the remaining three consistent with the pattern next to them. dialog.tsx has no open/close transition, so nothing animates on unmount. Steady-state effect, real Chromium, 70s, stacked on top of the activeAt patch fix: fiber tree 1680 -> 886 components re-rendered 54k -> 15.5k long tasks 26 / 2713ms -> 0 / 0ms main thread blocked 3.6% -> 0.0% frame rate 58.4 -> 59.8 fps Verified interactively: long-press opens the menu, Rename opens the dialog prefilled with the current name, Escape closes it, and no dialog is in the DOM while idle.
tiann
force-pushed
the
perf/mount-session-dialogs-on-demand
branch
from
July 29, 2026 11:58
12c9d0b to
53c21d4
Compare
There was a problem hiding this comment.
Findings
- None.
Summary
- Review mode: initial
- No correctness, security, regression, data-loss, performance, or maintainability issue found in the added/modified lines.
- Residual risk: no automated
SessionItemcoverage asserts that rename/archive/delete dialogs remain unmounted while closed and still complete their open/close flows.
Testing
- Not run (automation); PR code was not executed under the review security policy. The PR reports web typecheck and 1,598 passing web tests; the GitHub
testcheck was still in progress when reviewed.
HAPI Bot
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.
Problem
Every
SessionItemrenders its rename dialog and both confirm dialogs unconditionally, passingisOpento keep them closed (SessionList.tsx). With 28 sessions that is 84 mounted Radix dialogs the user cannot see, and each carries aPresence/DialogProvider/DialogContent/DialogPortalsubtree that re-renders whenever the row does.Measured against a live hub, those dialogs were roughly half of the entire fiber tree — 1680 nodes before, 886 after. Profiling a single
machine-updatedevent (arrives every 20s per machine, carries only changed CPU/memory numbers) showed it re-rendering 1600+ dialog-internal components:Fix
Mount them on demand. The same file already does this for
SessionExportDialogand the reopen-errorConfirmDialog, so this only brings the remaining three in line with the pattern sitting right next to them.dialog.tsxhas no open/close transition (its onlytransition-*is the close button's hover color), so nothing animates on unmount.Measured effect
Real Chromium against a live hub, 70s steady state, stacked on top of the activeAt patch fix (#1232). The fiber-tree number is attributable to this change alone:
Long tasks disappear entirely.
Verification
Driven interactively in a real browser with real pointer events, not synthetic clicks:
Testing
tsc --noEmit -p web/tsconfig.jsoncleanwebsuite: 1598 pass, 0 fail