Repository navigation
perf(shell): sidebar toggle re-lays out offstage workspace layers and LayoutBuilders every frame #984
Description
Summary
The sidebar toggle animation (Ctrl+B) costs 3.4–4.6 ms build per frame in a half-screen window, which leaves little headroom at 120 Hz and none at 144 Hz (6.9 ms budget including raster).
Evidence (real app, CPU profile of sidebar_toggle)
- ≈ 21 % of samples in
RenderOffstage.performLayout: inactive workspace layers thatQueryaSwitchingBodykeeps offstage (perf(motion): offstage inactive workspace layers in QueryaSwitchingBody to prevent redundant layout #901) are still laid out on every frame, because the width changes each frame andOffstagelays out its child. - ≈ 23 % in
_LayoutBuilderElement._rebuildWithConstraints:LayoutBuilders in the workspace rebuild their subtree for every new width.
Scope
- Skip layout of offstage layers while they are hidden (lay them out once when they become visible again).
- Avoid rebuilding
LayoutBuildersubtrees per animation frame (breakpoint-based rebuilds, or animate with a transform / clip instead of changing the laid-out width). - Target: < 2 ms build per animation frame.
Activity
ZhuchkaTriplesix commentedon Sep 27, 2026
One confirmed win shipped, root causes remain (PR #999)
Traced the full chain: `Ctrl+B` → `_MainContentSplitState.toggleSidebar()` → a spring ticks → `_onWidthSpringChanged` calls bare `setState({})` on the whole state every frame → its `build()` is one big `LayoutBuilder` that reconstructs the `Row`/`Expanded` layout → the workspace's `Expanded` sibling genuinely gets new width constraints every frame (inherent to sharing row space with the animated sidebar) → those constraints propagate into every `Offstage`/`LayoutBuilder` beneath it, including ones that don't need per-pixel-width precision.
Shipped: `VerticalSplitPane`'s `LayoutBuilder` (used by every open SQL tab's editor/results split) only needs height, not width — it was rebuilding its whole subtree on every sidebar-animation frame anyway. Cached it, correctly invalidated (height changes, or top/bottom/fraction/etc. actually changing — verified via test, including a version that caught this fix being disabled).
Found but did NOT ship (a fragile-cache correctness risk caught by my own test): `WorkspaceEmptyHero` has the same "only needs a coarse breakpoint, not raw width" pattern, but its content also depends on the theme and an async-loaded recent-connections list. A cache keyed only on the breakpoint can go stale if the window is resized across that breakpoint while the hero sits offstage (invisible, so the user wouldn't notice immediately — but would see a wrong layout if they later disconnect back to it). Reverted rather than ship that.
Confirmed against Flutter SDK source (`packages/flutter/lib/src/rendering/proxy_box.dart`): `RenderOffstage.performLayout` unconditionally calls `child.layout(constraints)` even when `offstage == true` — only paint/hit-test/semantics are skipped. `Visibility(maintainState: true)` is implemented in terms of `Offstage` and has the identical cost. So the ~21% `RenderOffstage.performLayout` share of the profile can't be fixed by swapping widgets; it needs either a custom RenderObject that skips layout while inactive (accepting a stale cached size until shown again), or — better — restructuring `_MainContentSplitState` so the workspace's laid-out width doesn't change during the toggle at all: reserve the sidebar's final width in the `Row` immediately and animate the reveal via clip/transform on the sidebar's own content, exactly as this issue's scope suggests as the alternative to "breakpoint-based rebuilds." That's a real (and riskier, given how central `main_screen.dart` is) redesign, out of scope for this PR.
Benchmark caveat: the bundled `sidebar_toggle` scenario in `benchmark/app_perf_bench.dart` runs before any SQL tab opens, so it can't observe this fix's benefit (no `VerticalSplitPane` mounted yet at that point in the scenario order) — before/after for that specific scenario is within noise. The fix is verified correct via a targeted unit test instead.
Leaving #984 open for the `Offstage`/reserved-width redesign.
Merge pull request #999 from QueryaHub/issue/984-sidebar-toggle-relayout
ZhuchkaTriplesix commentedon Sep 27, 2026
Partially resolved by #1004 (merged into
dev): addedLazyOffstage, which skips re-laying-out hiddenQueryaSwitchingBodylayers every animation frame (the "skip layout of offstage layers" scope bullet). Verified correct with a dedicated test.Benchmarked honestly: the bundled
sidebar_togglescenario shows no measurable difference, since the dominant per-frame cost there is the visible tree's ownLayoutBuilderrebuilds reacting to the changing width — the issue's other scope bullet ("avoid rebuilding LayoutBuilder subtrees per animation frame"), still unaddressed. That needs either a custom RenderObject or restructuring_MainContentSplitStateso the workspace's laid-out width doesn't change during the toggle at all (reserve final width in the Row immediately, animate the reveal via clip/transform) — a bigger, riskier change to a central screen.Leaving this open for that remaining work.
Merge pull request #1019 from QueryaHub/issue/984-sidebar-toggle-layo…
ZhuchkaTriplesix commentedon Oct 8, 2026
Done in #1019, #999 and #1004