Improve Modern UI resize and startup performance - #328462
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: Benjamin Christopher Simmonds (@benibenj)Matched files:
|
There was a problem hiding this comment.
🟡 Not ready to approve
Runtime toggles leave existing pane headers with stale inline line-height values.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Improves Modern UI resize and startup performance by eliminating repeated style reads and applying layout metrics earlier.
Changes:
- Push pane-header sizing globally and initialize overrides before restoration.
- Replace expensive agent-session
:has()selectors with row classes. - Add targeted startup, toggle, auxiliary-container, and pane-layout tests.
File summaries
| File | Description |
|---|---|
src/vs/base/browser/ui/splitview/paneview.ts |
Replaces computed-style reads with global header sizing. |
src/vs/base/browser/ui/splitview/paneview.css |
Documents the default CSS/TypeScript size coupling. |
src/vs/base/test/browser/ui/splitview/paneview.test.ts |
Tests pane constraints and body layout. |
src/vs/workbench/browser/layout.ts |
Removes the redundant Modern UI relayout. |
src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsViewer.ts |
Adds agent-session row modifier classes. |
src/vs/workbench/contrib/styleOverrides/browser/media/padding.css |
Replaces row-subject :has() selectors. |
src/vs/workbench/contrib/styleOverrides/browser/media/paneHeaders.css |
Documents the Modern UI header-size coupling. |
src/vs/workbench/contrib/styleOverrides/browser/styleOverrides.contribution.ts |
Applies dimensions before restoration and coordinates runtime layout. |
src/vs/workbench/contrib/styleOverrides/test/browser/styleOverrides.contribution.test.ts |
Tests startup and runtime override behavior. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 1
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟢 Ready to approve
The performance changes are consistently ordered, preserve styling behavior, and have focused regression coverage.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 0 new
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
|
Base:
|
There was a problem hiding this comment.
🟢 Ready to approve
The changes are coherent, preserve styling and accessibility behavior, and include focused coverage for the affected layout paths.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 0 new
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Fixes #328394
Summary
This addresses two Modern UI performance concerns:
Root causes and motivation
Resize
Pane.layout()invalidated and re-read--pane-header-sizewithgetComputedStyleon every layout pass. Split views therefore interleaved style reads and DOM writes across panes while a sash emitted synchronous changes for every pointer move.The Modern UI padding overrides also used relational
:has()selectors with.monaco-list-rowas the subject for agent session rows. Those selectors were considered for every list/tree row during style matching.Startup and runtime switching
The Modern UI class was available before initial layout, but
StyleOverridesContributiondid not apply the matching global scrollbar, notification, and pane dimensions until theRestoredphase. By then many widgets existed, so changing those global dimensions fanned out updates while extension loading and other post-restore work were active.Changing
workbench.experimental.modernUIalso triggered two layouts: an early layout fromLayout, followed by the authoritative layout after the override values changed.Moving this initialization earlier removes that late startup work and ensures restored widgets are created with the intended dimensions. The telemetry establishes the regression being addressed; a corrected local before/after startup benchmark is still needed to quantify how much of it this change recovers.
Changes
StyleOverridesContributionduringBlockRestore, before views, panels, and editors restore.:has()selectors with those modifier classes.Validation
Unit and static checks
PaneviewSplitviewStyleOverridesContributiongit diff --checkpassed.compile-clientemitted the changed sources without related errors. The repository currently reports pre-existing Agent Host/Claude SDK type errors unrelated to this change.Resize verification
Manual verification:
An automated probe with a 400-file Explorer performed five trials of 160 pointer moves in each Modern UI cohort. It observed:
--pane-header-sizeduring every resize trial;This verifies removal of the specific pane-header style read from the resize hot path. A trace-based base-versus-PR comparison is still needed to quantify the total resize improvement.
Stable telemetry context
The experiment token represented approximately 8.1–9.8% of Stable 1.130 desktop starts/machines after ramp-up, broadly consistent with the intended 10% target.
For a cleaned post-ramp sample, each machine contributed its median duration. The analysis excluded machines with more than five starts that day and machines observed in both token states:
Windows provides the most precise evidence and the largest user population, although macOS and Linux often show larger relative effects. The token indicates experiment assignment rather than directly logging the applied CSS state, and the frequency filter is only a proxy for excluding automation.
Local startup benchmark status
The initial ten-pair local benchmark is not used as evidence for startup improvement:
A corrected interleaved base/off, base/on, PR/off, and PR/on benchmark will be run separately before making a quantitative local startup claim.