Repository navigation
perf(input-router): a topology report that only selects other tabs keeps the router's tab index (R-INPUT S2 focus A/B) - #36262
Conversation
…ds no allocation in the router (red, R-INPUT S2 focus A/B)
…the router's tab index (a selection change rebuilt it: +13 % per report in the S2 focus A/B)
…us-core against the window focus alone (the FFI crate is its own workspace, which the fleet's Rust check does not build)
|
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)
red: cmux-next checks (god files, concurrency, crash safety, l10n); no finished base run on 64d6b1b to compare; do not merge until green; see lane-rules Shipped in:
The feat-cmux-next push run https://github.com/manaflow-ai/cmux/actions/runs/41990669105 at cf6dec5 failed: cmux-next checks (god files, concurrency, crash safety, l10n). @lawrencecchen |
|
Found 6 test failures on Blacksmith runners: Failures
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
|
Review (independent review subagent): SHIP at 3b4b7f9 Read-only correctness review of the branch's change (7e60135, 38789f7, cb89e40) at the merged head:
|
|
Non-green checks at 3b4b7f9, classified:
|
lawrencecchen
added
the
ready-to-land
…edes the older build (#36346) * ci(app-ffi-release): treeless build checkout; a newer FFI push supersedes the older build The build job's full clone of every ref was 5 to 10 minutes of a 7 to 15 minute build (12 recent runs); the Rust compile and tests were 1 to 4. fetch-depth 0 with filter tree:0 keeps every commit and ref the workflow-delta step reads and fetches trees on demand. The run-level group (cancel-in-progress: false) made a newer FFI push wait for the whole older run before building, while the older pin could no longer land (#36262). The build job now has its own group with cancel-in-progress; publish is never cancelled, and re-pins stay one at a time in app-ffi-repin. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-8707001042ea634e8771 * ci(app-ffi-release): publish one run per commit at a time Without the run-level group, two runs of one commit (a push and a dispatch, or two dispatches) could both pass the release view and race the create; the loser failed with a valid release in place. Publish now has a per-commit group that never cancels, so the second run finds the release and checks its checksum. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-8707001042ea634e8771 --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
REWRITE R-INPUT S2 acceptance, the focus bench (DESIGN.md: "focus bench within 5 %"). After S2b step B (#35875) every window's focus goes through the input router, and the router rebuilt its tab index on every topology report that ran the reducer, also when the report only selected another tab of the same panes (Cmd-1, a tab click, a switch back). The index holds each tab's id, surface, kind and host terminal, never the selection, so that rebuild made the same index again: one
Arc<str>per tab id, per surface and per host terminal, and two map inserts per tab.What changes:
InputRouter::focus_eventkeeps the index when the report lists the same tabs in the same order as the state it replaces (lists_same_tabs, a comparison of the two tab lists:Tabis id, surface, kind and terminal). A report that adds, drops, moves or changes a tab rebuilds it as before. An equal report still runs no reducer and no rebuild.Measured (the S2 focus A/B that found it)
Never-merge
FocusMachineSendBench(branchfix/r-input-focus-bench-nomerge): oneFocusMachine.send(a focus event's reduction and its reply read) and one topology report of a 30-pane window (two tabs a pane), Release configuration, exclusive measurement steps on one perf mini (cmux23-perf), alternating arms. Base: feat e241a67 (before #35875, the per-windowcmux_focus_*handle). Head: #35875's merge c6be943 (the sharedcmux_input_*router).Both head rounds agree, and so do both base rounds: the topology report through the router was 13 % slower at p50 than the window handle alone (8.04 -> 9.13 us; p99 9.08 -> 10.33-10.46 us), and a focus event 7-15 % slower (542-583 -> 625 ns). The acceptance is 5 %. In each round the bench's report changes which tab every pane shows, the case this PR fixes. The app links the Rust code from the pinned
CCmuxAppFFIrelease (Package.swift), so a Swift bench at a branch commit runs the pinned FFI, not this change: a run at c6be943 plus this fix (never-mergefix/r-input-focus-bench-fix-nomerge08cea81, step ce9d1f26, cmux23-perf) measured the unfixed router (send p50 625 ns, topology p50 9.13 us, the head's numbers), and I stopped that A/B. The end-to-end A/B runs after this merges, when app-ffi-release.yml publishescmux-app-ffi-<merge sha>and the next FFI pin lands; S2's 5 % acceptance stays open until then. The cause this PR removes is measured directly by the allocation test below.Tests
key_router::allocations::a_report_that_only_selects_another_tab_rebuilds_no_tab_index(cmux-tui/crates/cmux-focus-core/tests/input_router), red first: a 6-pane window whose panes each show their other tab; the router must make no more allocations for that report thanWindowFocusalone. Red on feat without the fix (never-mergefix/r-input-tab-index-red-nomerge9bce0f5 = feat 2c87a7e + the test), step a2ca7440d0ae3209c49c0ff3 (Linux): "the router made 42 allocations for a report that only selects other tabs, the window focus alone 12": 30 more, oneArc<str>for each of the 12 tab ids, 12 surfaces and 6 host terminals. Green: the full Rust check at 3b4b7f9 (this branch merged with feat 79a480d), step b78d2d95 (Linux): 1149 of 1149 tests passed, this one among them.(The test first went into the FFI crate's
tests/input_handle.rs, next to step A's corpus test. That crate is its own Cargo workspace, which the fleet'scmux-tui-rust-check.shdoes not build, so the red step ran 0 tests; the test moved tocmux-focus-core, which the check runs.)Observed / Not run
cargo test(CI'scmux-layout-reducer-ffi (own workspace)step runs it; step A's corpus test there compares every reply of the input handle with the window handle's).Changelog
none
No capture: no visible change (a router cost).
Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Stops the input router from rebuilding its tab index on topology reports that only change the selection, so Cmd-1, tab clicks, and switch-backs no longer pay the rebuild cost. A report that lists the same tabs in the same order keeps the index; one that adds, drops, moves, or changes a tab still rebuilds it. Measured in the S2 focus A/B, the rebuild made a 30-pane topology report 13% slower through the router (8.04 -> 9.13 us p50) and focus events 7-15% slower, over the 5% acceptance.
No change to the C ABI, to any reply, or to the app.
Tests
WindowFocusalone for a selection-only report.Written for commit 3b4b7f9. Summary will update on new commits.