Skip to content

perf(desktop): the Session rail re-renders on every AppShell commit #4109

Description

@Astro-Han

Problem

A single session switch produces about 20 React commits, and the Session rail is re-rendered by every one of them. The rail's own state changes once — the active row moves.

This was found while attributing steady-state renderer CPU. Idle costs 0.1%; a loop switching sessions every 1.5 s holds the renderer at 34% with peaks near 77%, and roughly 58% of the busy time is inside React render and commit.

Evidence

Measured on main (1975adb) with a 32-row sidebar, driving the dev app over CDP. Counts come from a probe on Intl.DateTimeFormat (SessionItem and SessionItemActions each construct one per render) and a MutationObserver.

One session switch, before any fix:

React commits~20
Full re-renders of the 32-row list~19
DOM mutations2,838
… of which inline style writes on sidebar astryx-button1,664

Every Astryx button removes and re-adds its anchor-name on each render ("--a, --b""--b""--a, --b"), so a rail render is also a style recalculation. That accounts for the UpdateLayoutTree 317 ms / Layout 250 ms seen in a DevTools trace of one switch.

The immediate cause of the leaf misses has a fix in flight separately (not yet on main): setActiveId was a function declaration in the useAppShellSessionWorkspace body, so it changed identity on every AppShell render and invalidated the whole chain down to renderSessionRow, defeating SessionNavRow's memo. A memo comparator probe reported {'onSelectSession,actions': 574, 'session,onSelectSession,actions': 64} with zero equal comparisons.

Why the leaf memo is not enough

With identities stabilised, alternating both variants inside one running instance (six switches each):

unstablestable
Row renders per switch45985
memo hits per switch0432
DOM mutations3,4381,992
Renderer JS521 ms380 ms
CPU under a repeated-switch loop34% (peak 77%)24% (peak 55%)

432 + 85 ≈ 16 × 32: the rail's parent chain still ran about 16 times per switch and rebuilt an element for all 32 rows each time. memo then bailed out at the leaf. The work moved from rendering rows to allocating elements and comparing props — a memo is a filter on a render that should not have reached the rail at all.

Callback identity is load-bearing across SessionNavigationHostSessionListPanelSessionHistoryListSessionListGroupsSessionNavRow, and it is enforced by hand at every layer. One ordinary-looking function declaration anywhere upstream cancels it, and useExhaustiveDependencies cannot see it, because the dependency arrays are correct.

What a memo boundary is actually worth

SessionNavigationHost was given a memo boundary, with its callback props stabilised through the useStableActions facade so the boundary can bail out. Measured by alternating the comparator inside one running instance (18 paired trials, six rows × three passes, order reversed between passes):

median busy JS per switch
boundary disabled206 ms
boundary enabled185 ms
paired delta13 ms (on wins 15/18, sign test p ≈ 0.008)

The boundary works: instrumenting the comparator shows the rail is asked to render ~14 times per switch and bails out on 10–12 of them. The three or four that get through differ on controller, staleSessionIds, and selection — and selection is the switch itself, so it is not removable.

These figures are unreliable — see the correction below.

So ~72% of the rail's renders were eliminated for a ~6% cut in busy JS. The rail's ~1,000 fibers are individually cheap; "68% of the recoverable fibers" was a count, not a share of time. Self time saved is correspondingly diffuse — the largest single entry is 45 ms summed over 36 trials, inside an Astryx layout effect.

Correction. The click helper behind this table selected [data-session-id] across the whole document and did not check the viewport. The corrected helper in scripts/perf/cdp-client.mjs was later observed opening Settings mid-run, after which a measurement describes an app that is not showing the rail at all. The run above cannot be repeated — the memo variant no longer exists — so treat 206 ms / 185 ms / 13 ms as unverified rather than as evidence. What survives is the qualitative point, which the corrected measurement in #4125 supports independently: moving the rail's state out of the shell cuts busy JS per switch from 585 ms to 397–430 ms, so the render never reaches the rail in the first place.

This closes the cheapest-looking route. Adding memo boundaries does not fix this class of defect, because there is no hot component to protect: the cost is spread across thousands of small renders, and the only lever with an order of magnitude in it is how many times the whole tree renders at all.

Proposed direction

Move the controller's call site, not the component.

useSessionNavigationController() is called in AppShell's render body (app-shell.tsx:1672). That single fact — not the size of any file — is what puts the rail's state above the whole tree. In React the position in the tree is the scope of the state: a hook called in the shell's render body has the whole tree as its scope; the same hook called in a provider has its readers as its scope.

  1. Call useSessionNavigationController once inside SessionNavigationServicesProvider, which already exists and already sits above the rail.
  2. Read it through a selector with custom equality — the external-store mechanism app-shell-session-ui-state.ts and use-app-shell-session-ui-selector.ts established in perf(desktop): AppShell subscribes to all session UI state at one granularity, so every stream token re-renders the whole shell #1985. No new state library (a non-goal of refactor(desktop): make AppShell a renderer composition root #3439), no new concept.
  3. Delete what that makes redundant: SessionNavigationHost's 15 props, its memo boundary, the commands and layoutuseMemos, renderSessionRow's eight-entry dependency array, the three redeclarations of the same prop set across SessionListPanel / SessionHistoryList / SessionListGroups, and the useStableActions facades that exist only because the state sits at the top.

Cross-feature intent stays explicit. Switching a session also clears active messages and exits the Work Hub; those are commands the shell issues, not state the rail subscribes to, and they must not become implicit subscriptions.

The useStableActions facade (#1043), the hand-carried prop chains, and the memo boundaries are three treatments for one condition: state held above its readers. Removing the condition removes all three. This issue should end with fewer concepts than it started with.

This is the same ground as Establish Session catalog authority and Extract Session Navigation in #3439, narrowed to one measurable defect that can ship on its own.

Why this generalises

app-shell.tsx declares 16useState of its own. The 536 hooks on the AppShellContent fiber come from the 37 custom hooks it calls — useShellChatModel, useShellSearch, useShellResume, useWorkbarController, useSessionNavigationController, and so on. They are already separated by feature. What is not separated is where they are called.

(Corrected. This paragraph first said 7 and 38. Those came from a counter that required a ( directly after the hook name and so skipped every useState<T>(...); the same defect made the gate in #4125 report 39 hooks / 69 call sites where the real figures are 42 / 83, and let two hooks that were in no inventory pass silently. Found by adversarial review, fixed in that PR.)

That reframes the remaining work in #3439. Session Navigation is already a complete model / controller / ui / ports slice, and its controller is still invoked from AppShell's render body — so extracting the slice did not, by itself, change the scope of any state. The unit of progress is the call site, and it has a definition of done that a checklist item does not: a hook is either called in the shell's render body or it is not.

A lint rule can hold it. Allow-list the hooks currently called in AppShellContent; forbid additions; let the list converge to zero. That is cheaper and less flaky than a render-count assertion, and every removal is individually verifiable. The MutationObserver contract below stays as the performance-side check.

Acceptance

A contract spec, not an identity assertion: switching sessions renders the rail at most K times, asserted against a MutationObserver on the sidebar. Result-shaped, so it also covers future causes — a new unstable prop, a new context, a new effect cascade.

Attribution of the ~20 commits

Resolved. It is not a cascade to be broken, and nothing is misbehaving.

AppShellContent carries 536 hooks — 108 useState/useReducer, 9 useSyncExternalStore, 110 effects — above the entire tree with no boundary beneath it. Of ~19 commits per switch, ~14 are full-tree renders. Each is triggered by one to five setState calls arriving from independent async sources in separate tasks: the click handler, the session-read-state commit, the agent graph, two message loads, four transcript contextKey promises, the observation seed, two commitSessions, two onLoadEarlierHistory, connections onSnapshot / seedSnapshot, and two session-event health updates. React cannot batch across tasks, so each becomes its own commit.

Measured against an identity-insensitive walk of the fiber tree, 96–99% of every full-tree render is recoverable by perfect memoisation: only 13–56 fibers per commit have genuinely changed props. Median busy JS is 281 ms per switch, and top-8 self time is ~120 ms — the cost is spread, not concentrated, which is the same shape the memo measurement above reports.

The commit count and the per-render cost are the two factors, and both are the same root cause: state held above its readers. Neither is fixable at the leaf.

Earlier note, kept because it rules out an alternative: the scheduling timeline shows a commitRoot → flush passive effects → new state update → next render cascade, and it is not animation-driven — emulating prefers-reduced-motion cuts requestAnimationFrame callbacks from 188 to 13 while the render count is unchanged (17.2 → 19.3).

Related


Measurements and this report were produced with Claude Code; the numbers come from CDP profiling, Function.prototype.bind interception of React's dispatchSetState, and fiber-tree walks on a running dev build, not from static reading. Every A/B comparison alternates configurations inside one running instance with paired trials, because restarting the app shifts these metrics by orders of magnitude.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions