Repository navigation
perf(layout): New Column, New Row and Cmd-D adopt pane views made ahead (no pane view built in the key's turn) - #35521
Conversation
TERMINAL CREATE COST item 2 (STATUS 11836): New Terminal Tab from the docked agent chat opens a New Column; its key turn built the pane's views (3.5 ms) and added them to the window (2 ms of AppKit's _setWindow walk) on cmux21.
…es before the key (its background work)
…ad; a New Column's key turn builds no pane view TERMINAL CREATE COST item 2 (STATUS 11836, 11933). New Terminal Tab from the docked agent chat opens a New Column; its key turn built the pane's strip model and views (PaneContentView, TabStripView and its buttons: 3.5 ms on cmux21). As Chrome keeps a spare renderer for its next tab, each shown workspace keeps one PaneController.Spare, made out of every window as one background task on the app's scheduler (IdleSpare in CmuxNextWakeups) and only while the app is still: after the key's frame committed and when no layout motion runs (a motion's start stops a make already queued; its landing asks again). A stand-in pane (New Column, New Row, Cmd-D) adopts it in the key's turn. A parked workspace drops its spare and makes none. Review of 2e67a529267 (NO SHIP): the spare was parked in the window, so the adopted pane left and re-entered the window in the key's turn (no walk saved; WindowEntryHandoff broken for a page stand-in), and its refill could run in the first spring frame. Both fixed: the spare is out of every window, and the refill waits for a still layout. The 2 ms window walk of the new host stays until PL-motion-core answers STATUS 11937.
…y, made out of every window
…ing and at unpark; a test covers the motion gate Review of f19c7f2 (P2): a landing's refill and an unpark's refill now run after that frame committed (AfterFrameCommit), never in the landing or switch frame; a motion's start stops a queued make at once. NewColumnSparePaneTests now also checks that no spare is made while the new column springs in, and that one is made after it lands.
…re (TERMINAL CREATE COST item 2)
…wned; crash ratchet)
…its motion-gate check has a spring
… and off; its key-turn checks do not depend on motion Lead review of green 496476cc: the motion check assumed a running spring. The key-turn assertions (the adopted strip model and view, no new spare in the turn) now hold in both arguments; the refill rule is checked with the springs pinned on (no make in a spring frame, one after the landing) and off (one after the key's frame).
…icitly (no spring in the test setup); no pane springs Lead review of green 85ecea4f: with the springs pinned on, the test setup's New Column still did not spring, so the check rested on motion state. The key-turn checks run with no animation; the refill rule is driven through LayoutRootView.setLayoutInMotion (a spring's start and landing).
|
Review (independent review subagent): SHIP at 60a96e6 Three review rounds: NO SHIP at 2e67a529267 (the spare was parked in the window, so the 2 ms walk was not saved and WindowEntryHandoff broke; the refill could run in a spring frame), then SHIP at f19c7f2, f1eb390 and 60a96e6. P2s still open: no single command is red on the base and green on the head; the test does not go through sync's real spring start; the cost of a new spare after a workspace switch is not measured (all listed under Observed / Not run). |
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
Shipped in:
The feat-cmux-next push run https://github.com/manaflow-ai/cmux/actions/runs/41888017760 at dba5f44 failed: cmux-next checks (god files, concurrency, crash safety, l10n). |
|
Found 3 test failures on Blacksmith runners: Failures
|
…d in the window; the New Column key turn puts no new view in the window TERMINAL CREATE COST item 4 (lead, STATUS 12604; assigned after PL-motion-core did not answer STATUS 11937/12346). With #35521 the New Column key turn still added the new pane's host and pane views to the window: AppKit's _setWindow walk over that subtree, 2 ms of the 9 ms turn on cmux21. LayoutViewContext now keeps one spare PaneHostView: the App's pane spare (IdleSpare, made only while the layout is still) is put in a host made at the same time and parked hidden in a SpareHostShelf inside the LayoutRootView, so its window walk runs at idle. host(for:) adopts that host when the provider's new content is the spare content (PaneHostView.adopt binds its pane). A parked workspace drops it. The shelf takes no hits and has no accessibility children. The #35521 test now expects the pane views parked hidden in the window.
…4's red and green runs Review (claude-independent) nit at 86a8945: the doc link named parkSpare(_:), the method is parkSpare(_:in:). Comment only. Item 4 evidence (TERMINAL CREATE COST, STATUS 12604, 12670, 12684), suite `scripts/ci/package-test-lane.sh suite Packages/macOS/CmuxNext NewColumnSpareHostTests`: - red a6c56d2d at 43cc302 (#35521's code, test only): the key's turn put a new pane host and pane view in the window. - red f747bb32 at 17bc80f (86a8945's test on its parent 4ea645b): :122 hostProbe.sameWindowCalls 1 ("window w -> same") and :124 the host's superview is ScreenContentView, not the SpareHostShelf it waited in. - green 0e47bca4 at 86a8945: the 7 suites pass (NewColumnSpareHostTests 1 test, NewColumnSparePaneTests, ClosedPaneContentStaysUntilLandingTests, StandInPaneCloseTests, DockChatStripOrderTests, NewTabReplaceInPlaceTests, CmdWOnReplacedStandInTests); compile 8841294c passed.
|
Swift logic follow-up (tip watcher swift-logic row, STATUS 13027 "SWIFT LOGIC CULPRITS"; lead order). This PR raised
plans/cmux-next/layer-ownership.md says Rust owns every state, list and rule and Swift is the platform shell. Please move the new logic of this PR in a follow-up: to Rust (the crate that owns the area; the owner lists in STATUS 12147-12165 name the target per area), or at least out of the app into its package. If a declaration above is platform code (AppKit, Core Animation, OS calls, strings, tokens, DEBUG tooling), reply here and the table gets a PLATFORM row with your reason instead. The swift-logic row is a report, not a gate: nothing blocks on this comment. |
…n moves no view in the window (#35841) * test(layout): a New Column's key turn builds no pane view (red) TERMINAL CREATE COST item 2 (STATUS 11836): New Terminal Tab from the docked agent chat opens a New Column; its key turn built the pane's views (3.5 ms) and added them to the window (2 ms of AppKit's _setWindow walk) on cmux21. * test(layout): the New Column spare test runs the app scheduler's frames before the key (its background work) * perf(layout): a stand-in pane adopts a strip model and views made ahead; a New Column's key turn builds no pane view TERMINAL CREATE COST item 2 (STATUS 11836, 11933). New Terminal Tab from the docked agent chat opens a New Column; its key turn built the pane's strip model and views (PaneContentView, TabStripView and its buttons: 3.5 ms on cmux21). As Chrome keeps a spare renderer for its next tab, each shown workspace keeps one PaneController.Spare, made out of every window as one background task on the app's scheduler (IdleSpare in CmuxNextWakeups) and only while the app is still: after the key's frame committed and when no layout motion runs (a motion's start stops a make already queued; its landing asks again). A stand-in pane (New Column, New Row, Cmd-D) adopts it in the key's turn. A parked workspace drops its spare and makes none. Review of 2e67a529267 (NO SHIP): the spare was parked in the window, so the adopted pane left and re-entered the window in the key's turn (no walk saved; WindowEntryHandoff broken for a page stand-in), and its refill could run in the first spring frame. Both fixed: the spare is out of every window, and the refill waits for a still layout. The 2 ms window walk of the new host stays until PL-motion-core answers STATUS 11937. * test(layout): the New Column test checks the adopted views by identity, made out of every window * perf(layout): the pane spare refills after the frame commit at a landing and at unpark; a test covers the motion gate Review of f19c7f2 (P2): a landing's refill and an unpark's refill now run after that frame committed (AfterFrameCommit), never in the landing or switch frame; a motion's start stops a queued make at once. NewColumnSparePaneTests now also checks that no spare is made while the new column springs in, and that one is made after it lands. * chore(arch): reviewed app_lines exception for the New Column pane spare (TERMINAL CREATE COST item 2) * fix(wakeups): IdleSpare has a nonisolated deinit (generic main-actor class rule) * fix(layout): the pane spare's make holds its workspace weakly (no unowned; crash ratchet) * test(layout): the New Column spare test pins the pane springs on, so its motion-gate check has a spring * test(layout): the New Column spare test runs with the pane springs on and off; its key-turn checks do not depend on motion Lead review of green 496476cc: the motion check assumed a running spring. The key-turn assertions (the adopted strip model and view, no new spare in the turn) now hold in both arguments; the refill rule is checked with the springs pinned on (no make in a spring frame, one after the landing) and off (one after the key's frame). * test(layout): the New Column spare test drives the layout motion explicitly (no spring in the test setup); no pane springs Lead review of green 85ecea4f: with the springs pinned on, the test setup's New Column still did not spring, so the check rested on motion state. The key-turn checks run with no animation; the refill rule is driven through LayoutRootView.setLayoutInMotion (a spring's start and landing). * test(layout): a New Column's key turn puts no new view in the window (red) TERMINAL CREATE COST item 4 (lead, STATUS 12604): with #35521 the New Column key turn still added the new pane's host to the window (AppKit's _setWindow walk: 2 ms of the 9 ms turn, cmux21). * perf(layout): a stand-in pane adopts a pane host made ahead and parked in the window; the New Column key turn puts no new view in the window TERMINAL CREATE COST item 4 (lead, STATUS 12604; assigned after PL-motion-core did not answer STATUS 11937/12346). With #35521 the New Column key turn still added the new pane's host and pane views to the window: AppKit's _setWindow walk over that subtree, 2 ms of the 9 ms turn on cmux21. LayoutViewContext now keeps one spare PaneHostView: the App's pane spare (IdleSpare, made only while the layout is still) is put in a host made at the same time and parked hidden in a SpareHostShelf inside the LayoutRootView, so its window walk runs at idle. host(for:) adopts that host when the provider's new content is the spare content (PaneHostView.adopt binds its pane). A parked workspace drops it. The shelf takes no hits and has no accessibility children. The #35521 test now expects the pane views parked hidden in the window. * fix(layout): the spare pane API file imports AppKit publicly (its public methods take NSView) * fix(layout): a workspace switch keeps the parked pane spare; the host test records real window changes Review of 0eeb782 (NO SHIP, P1): park dropped the spare host while it was in the window, so every workspace switch took that subtree out of the window in its key turn (the cost moved from New Column to the switch). A parked workspace now keeps its spare, hidden in its hidden layout (as WindowRootView keeps a hidden workspace's views in the window); it only stops making new ones. NewColumnSpareHostTests: the adopted host is the parked one by identity, the probe counts only calls whose window differs (green 042231ea saw one same-window call), and the new column's tab strip counts as shown. * test(layout): the host test logs each probe event (class, superview change) and probes the parked host too Lead on green a5d859d9: one viewWillMove(toWindow:) reached the probe in the pane views. The probe was a direct subview of PaneContentView, which owns its subviews; a removal of the probe itself also calls viewWillMove(toWindow: nil). The test now records every event of both probes (in the pane views and in the parked host) and counts only window changes of the subtree the probe was put in, so the next run names the view that moves. * perf(layout): the spare pane host waits hidden in the active screen view; the key's turn only unhides it Lead on green e150352e: the host waited in a shelf view, so the key's turn moved it to the screen view, and AppKit called viewWillMove(toWindow:) on both subtrees (the same window; probe log "window w -> same"). The host now waits hidden in the screen view where the new pane shows (a hidden host there is a state the screen already has: a closing pane's retired host), so the screen's reconcile adds nothing and only unhides it. The shelf is gone. The host test resets its probe counters after the probe's own add (that add was the counted window change), and now requires no same-window call either, and that the host shows in the screen view it waited in. * docs(layout): PaneHostView.adopt links parkSpare(_:in:); record item 4's red and green runs Review (claude-independent) nit at 86a8945: the doc link named parkSpare(_:), the method is parkSpare(_:in:). Comment only. Item 4 evidence (TERMINAL CREATE COST, STATUS 12604, 12670, 12684), suite `scripts/ci/package-test-lane.sh suite Packages/macOS/CmuxNext NewColumnSpareHostTests`: - red a6c56d2d at 43cc302 (#35521's code, test only): the key's turn put a new pane host and pane view in the window. - red f747bb32 at 17bc80f (86a8945's test on its parent 4ea645b): :122 hostProbe.sameWindowCalls 1 ("window w -> same") and :124 the host's superview is ScreenContentView, not the SpareHostShelf it waited in. - green 0e47bca4 at 86a8945: the 7 suites pass (NewColumnSpareHostTests 1 test, NewColumnSparePaneTests, ClosedPaneContentStaysUntilLandingTests, StandInPaneCloseTests, DockChatStripOrderTests, NewTabReplaceInPlaceTests, CmdWOnReplacedStandInTests); compile 8841294c passed.
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Summary
New Terminal Tab from the docked agent chat opens a New Column. Its key turn took 10-11 ms on cmux21. Of that, 3.5 ms built the new pane's strip model and views (
PaneContentView,TabStripViewand its buttons). As Chrome keeps a spare renderer for its next tab, each shown workspace now keeps onePaneController.Spare. It is made out of every window, as one background task on the app's scheduler (IdleSpare, new in CmuxNextWakeups), and only while the layout is still: after the key's frame committed, after a layout motion landed, after an unpark. A layout motion's start stops a make that is already queued. A stand-in pane (New Column, New Row, Cmd-D) adopts the spare in the key's turn, so that turn builds no pane view. A parked workspace drops its spare and makes none.The spare stays out of the window until its host joins it. So the new pane enters the window once, as before (WindowEntryHandoff, 835342c), and the 2 ms
_setWindowwalk of the new host stays in the key turn for now. Making the host ahead needs PL-motion-core (STATUS 11937).App lines: +27, under the reviewed exception
plans/cmux-next/arch-exceptions/newcolumn-pane-spare.json(PaneController.Sparebuilds App views; the gating uses App types).Performance
New Column from the docked agent chat. Journey tcc-newcol-xtp: 44 warm presses,
debug.action_budgetcommit ms. Both arms ran on cmux21 through the mini scheduler. Bench builds: base 4eb4aa08 (this PR's feat base), head 5f5a690f (app code of 60a96e6's source change).WorkspaceContentController.makeContentViewScreenContentView.reconcileThe cold first press was 74.5 ms (base) and 105.6 ms (head). Earlier rounds of the same journey gave 72 to 107 ms for the first press in both arms, so I claim nothing for it. The arms ran about 3.5 h apart on the same mini.
Display fleet run (gui-session-v1)
cmux-ci run --display --label gui-session-v1, display worker cmux9s-Mac-mini): done, exit 0. NewColumnSparePaneTests, ChatDockToolPlacementTests (display-only), DockChatStripOrderTests, ClosedPaneContentStaysUntilLandingTests, NewTerminalTabKeyFrameTests: 5 of 5 suites pass, on a real console session.Testing
NewColumnSparePaneTests(no pane springs; a layout motion driven throughLayoutRootView.setLayoutInMotion). The test asserts:Observed / Not run
Observed: the steps above; the journey rounds in Performance.
Not run, or not shown by one command:
content.paneSpare, which does not exist on the base; the red run used the earlier form of the test.LayoutRootView.sync(it drivessetLayoutInMotionitself).Changelog
Changed: a new column, row or split opens with less work in its first frame
Proof
No capture: nothing changes on screen.