Reduce workspace switch portal hide work

Merged
AU
4mo ago
+202-11

Summary

  • Avoid reapplying portal-rendering state to every workspace on each workspace-switch mount reconciliation.
  • Track the last portal-rendering state reconciled by ContentView and only call setPortalRenderingEnabled for workspaces whose desired state changed.
  • Add focused coverage for the new reconciliation planner so already-disabled workspaces do not repeat portal-hide work on later switches.

Evidence

Reproduced against the already-running /Applications/cmux.app without building a dev app.

Fixture:

  • workspace:205: 1 terminal surface.
  • workspace:207: 12 terminal surfaces.
  • Repeated cmux select-workspace --window window:1 between the two already-open workspaces.

Observed pre-fix CLI wall time:

  • Switching from 12 terminal surfaces to the one-surface workspace: median 752.521 ms, mean 767.426 ms, p90 818.444 ms.
  • Switching from the one-surface workspace back to 12 terminal surfaces: median 204.406 ms, mean 197.117 ms, p90 238.931 ms.

The local direction differs from the issue wording: the cost showed up while retiring/unmounting the many-surface workspace. The product effect is the same repeated toggle latency involving already-open large workspaces.

Source inspection mapped that repeated cost to ContentView.reconcileMountedWorkspaceIds, which called setPortalRenderingEnabled(false) for every unmounted workspace every reconciliation. That method clears layout follow-up and hides every terminal/browser portal for the workspace even if it was already disabled, so the work repeats with the number of portal-backed surfaces.

A sample capture was attempted during switching, but the release app produced an empty call graph after thread-state sampling failures, so it is not used as hotspot proof.

Validation

  • git diff --check
  • Not run: local tests or builds, per task instruction. CI should run the added unit coverage.

Notes

A true failing-first UI/performance regression test was not practical without adding a test seam around AppKit portal side effects. The added unit tests cover the new bounded reconciliation policy rather than pretending to measure UI latency in unit tests.

No user-facing strings were added or changed, so no localization files were updated.

Closes #7226

View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.

Summary by cubic

Eliminates repeated portal hide/show during workspace switches by tracking per-workspace portal state and applying only deltas in stable tab order. Persists the state across handoff to cut switch latency, addressing #7226.

  • Bug Fixes
    • Added WorkspacePortalRenderingPlan and WorkspacePortalRenderingChange to compute enable/disable deltas and the next-state snapshot.
    • Updated ContentView to use WorkspacePortalRenderingPlan(...).applying(to:), apply only returned changes in orderedTabIds order, and persist lastReconciledPortalRenderingStatesByWorkspaceId (set to false during handoff for the retiring workspace).
    • Added tests for one-time disable on initial unmount, reporting only mount transitions, advancing the previous-state snapshot, and tolerating duplicate ordered workspace IDs.

Summary by CodeRabbit

  • New Features
    • Added a workspace portal-rendering reconciliation plan that applies only required enable/disable updates.
    • Persist the last reconciled portal-rendering enabled state per workspace to maintain consistent behavior across mount/unmount cycles.
  • Bug Fixes
    • Avoids repeated hide/show actions when a workspace’s portal-rendering state hasn’t changed.
    • Improves mount/selection transitions and prevents stale workspaces from being reported for updates.
  • Tests
    • Added unit tests validating change computation and snapshot reconciliation behavior.