Skip to content

Fix portal hit-test CPU on pointer movement - #6592

Merged
austinywang merged 20 commits into
mainfrom
issue-6586-portal-hit-test-cpu
Jun 22, 2026
Merged

austinywang merged 20 commits into
mainfrom
issue-6586-portal-hit-test-cpu

Conversation

@austinywang

@austinywang austinywang commented Jun 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #6586 by removing unbounded portal hit-test work from high-frequency pointer movement paths.

The supplied Time Profiler captures are the reproduction evidence. No manual gesture transcript was supplied, so the interaction path is inferred from the repeated stacks in the profiles.

Profile Evidence Before

From time-profile-184312.xml:

  • Main Thread running: 10112 ms of 12370 ms sampled.
  • Inclusive cmux frames:
    • BonsplitTabBarPassThrough.shouldPassThroughToPaneTabBar(windowPoint:below:): 1338 ms
    • BonsplitTabBarPassThrough.hasUnderlyingBonsplitTabBarBackground(at:below:): 1333 ms
    • BonsplitTabBarPassThrough.hasBonsplitTabBarBackground(at:in:): 1328 ms
    • WindowBrowserHostView.hitTest(_:): 583 ms
    • WindowTerminalHostView.performHitTest(at:currentEvent:): 512 ms
    • WindowTerminalHostView.updateDividerCursor(at:): 503 ms

From time-profile-184454.xml:

  • Main Thread running: 3377 ms of 4483 ms sampled.
  • Repeated split-divider hit-testing remained visible through WindowBrowserHostView.dividerHit and WindowTerminalHostView.dividerCursorKind.

What Changed

  • mouseMoved / mouseEntered / mouseExited / cursorUpdate tab-strip pass-through now trusts BonsplitTabBarHitRegionRegistry and skips the fallback class-name subtree scan on registry miss.
  • Terminal and browser portal hosts cache split-divider rectangles in window coordinates and hit-test those rectangles on pointer movement instead of recursively walking split subtrees per event.
  • Divider caches invalidate on host frame changes, host subview churn, cursor-rect rebuilds, and same-window NSSplitView.didResizeSubviewsNotification.
  • Browser divider regions preserve whether the split is inside hosted content so app split dividers still pass through while hosted inspector dividers remain interactive.

Tests

  • Added a test-only commit first:
    • hover tab-strip registry misses must not recurse into TabBarBackgroundNSView descendants
    • repeated terminal pointer moves must reuse cached divider rectangles instead of converting through the split view every event
  • Not run locally by request; PR CI is the execution proof.

Notes

No user-facing strings were changed, so no localization catalog updates were needed.


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


Summary by cubic

Cuts CPU on pointer hover by replacing recursive portal hit‑testing with cached, liveness‑checked split‑divider regions and registry‑only tab‑bar routing. Fixes #6586.

  • Bug Fixes

    • Hover pass‑through now relies only on BonsplitTabBarHitRegionRegistry; non‑hover events keep a bounded top‑band fallback scan.
    • Browser and Terminal cache split‑divider regions in window coords with split‑bounds clipping and liveness checks; caches invalidate on geometry/structure changes and same‑window NSSplitView.didResizeSubviewsNotification via bounded observers with safe cleanup; Browser keeps hosted‑inspector dividers interactive while app splits pass through.
  • Tests

    • Added PortalHitTestingPerformanceTests covering registry‑only hover routing, reuse of cached divider regions on pointer moves, stale cache rejection after split removal, and cache refresh on root/nested insertions, visibility changes, and container moves.

Written for commit ac4c396. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Improved tab-bar pass-through so routing honors only registered tab-bar hit regions, with behavior varying correctly by pointer event type.
    • Prevented incorrect pass-through on unregistered tab-strip surfaces.
  • Performance Improvements
    • Refactored split-divider hit-testing and cursor rect handling to use cached divider regions with automatic invalidation on window/layout/view changes.
  • Tests
    • Added performance and correctness coverage for portal hit-testing and cached divider-region behavior across cache invalidation scenarios.

@vercel

vercel Bot commented Jun 22, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Jun 22, 2026 11:45am
cmux-staging Building Building Preview, Comment Jun 22, 2026 11:45am

@coderabbitai

coderabbitai Bot commented Jun 22, 2026 •

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Replaces recursive AppKit view-tree scanning on every pointer event with a cached PortalSplitDividerRegion model in both WindowBrowserHostView and WindowTerminalHostView. Adds PortalSplitDividerCacheInvalidator for observation-driven invalidation. Gates tab-bar pass-through to registered regions only for hover-style events via a new eventType parameter.

Changes

Portal hit-testing performance: caching and event-type gating

Layer / File(s) Summary
Tab-bar pass-through: registered-regions-only gating for hover events
Sources/BonsplitTabBarPassThrough.swift
shouldPassThroughToPaneTabBar gains an optional eventType parameter; a new usesRegisteredTabBarRegionsOnly(eventType:) helper short-circuits the recursive scan for .pointerHover events, and passThroughDecision forwards eventType to the updated call.
PortalSplitDividerRegion: shared divider geometry type
Sources/PortalSplitDividerRegion.swift
New PortalSplitDividerRegion stores split view identity, divider index, window-relative rects, orientation, and hosted-content flag. isLive validates all constraints; collect(in:hostView:) recursively traverses the view tree, computing divider rects in window coordinates and setting isInHostedContent per split view. Removes need for recursive per-event divider discovery.
PortalSplitDividerCacheInvalidator: observation-driven invalidation
Sources/PortalSplitDividerCacheInvalidator.swift
New PortalSplitDividerCacheInvalidator registers frame/bounds notifications and KVO on isHidden/subviews; fires onChange callback to invalidate cache and removes all observers on invalidate() or deinit.
BrowserWindowPortal: divider region cache with isInHostedContent
Sources/BrowserWindowPortal.swift
WindowBrowserHostView replaces its local DividerRegion struct with a PortalSplitDividerRegion typealias, adds cached region state and a resize-observer token, invalidates on window/frame/subview changes, and uses the cache in resetCursorRects and splitDividerHit. Removes shouldPassThroughToSplitDivider in favor of dividerHit.isInHostedContent.
TerminalWindowPortal: divider region cache and cursor-hit refactor
Sources/TerminalWindowPortal.swift
WindowTerminalHostView switches DividerRegion to a typealias, adds cached region state and a resize observer with deinit cleanup, and rewrites splitDividerCursorKind to use region-based expanded-rect containment instead of recursive view-tree walking.
Tests and Xcode project wiring
cmuxTests/PortalHitTestingPerformanceTests.swift, cmux.xcodeproj/project.pbxproj
Adds six tests covering: hover tab-bar pass-through uses only registered regions; pointer-move hit-tests reuse cached regions; cache ignores removed split views; cache refreshes on root subview insertion, visibility change, and container move. Xcode project wired for new source and test files.

Sequence Diagram(s)

sequenceDiagram
    participant AppKit as AppKit (pointer/layout event)
    participant HostView as WindowTerminalHostView / WindowBrowserHostView
    participant Cache as cachedSplitDividerRegions
    participant Region as PortalSplitDividerRegion.collect()
    participant Invalidator as PortalSplitDividerCacheInvalidator

    rect rgba(200, 80, 80, 0.5)
        Note over AppKit,Invalidator: Cache invalidation path
        AppKit->>HostView: viewDidMoveToWindow / setFrameSize / didAddSubview / willRemoveSubview
        HostView->>Invalidator: observe(geometryViews, structureViews, onChange:)
        Invalidator->>Cache: invalidateSplitDividerRegionCache() → nil
        HostView->>AppKit: invalidateCursorRects
    end

    rect rgba(80, 120, 200, 0.5)
        Note over AppKit,Region: Hit-test / cursor-rect path (pointer move)
        AppKit->>HostView: performHitTest / splitDividerCursorKind / resetCursorRects
        HostView->>Cache: splitDividerRegions()
        Cache-->>HostView: nil (miss) → Region.collect(in:hostView:)
        Region-->>Cache: [PortalSplitDividerRegion] stored
        HostView->>HostView: dividerHit / dividerCursorKind iterates cached regions only
        HostView-->>AppKit: hit result (isInHostedContent, cursor kind)
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

  • #6586 – Portal split/tab hit testing burns main thread on pointer movement: This PR directly addresses the profiled hotspots (BonsplitTabBarPassThrough.shouldPassThroughToPaneTabBar, WindowBrowserHostView.dividerHit, WindowTerminalHostView.dividerCursorKind) by introducing the caching and registered-region-only gating described in the issue's suggested fix direction.

Possibly related PRs

  • manaflow-ai/cmux#4863: Both PRs modify Sources/BonsplitTabBarPassThrough.swift to gate tab-bar/pointer pass-through decisions using the incoming NSEvent.EventType via WindowInputRoutingContext.
  • manaflow-ai/cmux#4290: Both PRs adjust the pane tab-bar pass-through decision to honor BonsplitTabBarHitRegionRegistry hit regions based on eventType, short-circuiting fallback routing.
  • manaflow-ai/cmux#3720: Both PRs condition shouldPassThroughToPaneTabBar routing on whether a visible hosted terminal surface is hit, overlapping in the tab-bar pass-through gating logic.

Suggested reviewers

  • Ari4ka
  • lawrencecchen

Poem

🐇 No more walks through every view's nook,
The divider rects are cached — go take a look!
On hover, skip the scan, trust the registry's list,
Main thread stays cool — no more pointer-move gist.
The rabbit approves: fewer trees, faster tricks! 🌿


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 1 warning)

Check name Status Explanation Resolution
Cmux No Test Or Debug Seam In Production Source ❌ Error Added test-seam isPointerDragActiveForTesting under #if DEBUG in production source Sources/TerminalWindowPortal.swift, violating the naming pattern rule for test-debug seams. Remove the #if DEBUG static var isPointerDragActiveForTesting = false declarations from WindowTerminalPortal and TerminalWindowPortalRegistry classes; this facility should not exist in shipping source. Refer to PR #6452 for the can...
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (21 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix portal hit-test CPU on pointer movement' accurately and concisely summarizes the main change: addressing CPU performance in portal hit-testing during pointer events.
Linked Issues check ✅ Passed The PR implementation fulfills all acceptance criteria from issue #6586: removes unbounded recursive tree-walking from pointer paths, caches divider regions, trusts registry for tab-bar routing, adds focused tests, and includes profile-based reasoning.
Out of Scope Changes check ✅ Passed All changes directly address the performance bottleneck identified in #6586: tab-bar pass-through logic, terminal/browser divider caching, cache invalidation, and performance tests. No unrelated modifications detected.
Cmux Swift Actor Isolation ✅ Passed Both new production files (PortalSplitDividerRegion and PortalSplitDividerCacheInvalidator) are properly annotated @MainActor. The nonisolated(unsafe) properties are justified with documentation an...
Cmux Swift Blocking Runtime ✅ Passed PR adds portal hit-test caching with KVO/Notification-based invalidation; no blocking primitives (semaphores, sleeps, locks, or sync waits) introduced in production Swift code.
Cmux Expensive Synchronous Load ✅ Passed PR contains no agent-history loads, transcript parsing, JSON/JSONL file I/O, or expensive syscalls. Changes are UI hit-testing optimization using view-tree traversal and caching—not scope of the rule.
Cmux Cache Substitution Correctness ✅ Passed Cache substitution in this PR is for transient UI geometry (divider regions, cursor rects) not persistence/undo. Cache has liveness checks (PortalSplitDividerRegion.isLive), event-driven invalidati...
Cmux No Hacky Sleeps ✅ Passed Check not applicable: PR contains only Swift source and build config changes. Rule explicitly excludes Swift (covered by swift-blocking-runtime.md) and only applies to TypeScript, JavaScript, shell...
Cmux Algorithmic Complexity ✅ Passed PR improves algorithmic complexity by replacing unbounded recursive tree walks with cached divider regions (single-pass collect, O(dividers) hot path) and registry-gated hover routing, eliminating...
Cmux Swift Concurrency ✅ Passed PR introduces AppKit-boundary-only callbacks in new @MainActor classes using proper MainActor.assumeIsolated isolation; no DispatchQueue, Task, Combine, or fire-and-forget patterns in new code.
Cmux Swift @Concurrent ✅ Passed All new/modified Swift code properly isolates UI-bound synchronous work on @MainActor. No async functions introduced; observer callbacks correctly use MainActor.assumeIsolated. No @concurrent viola...
Cmux Swift File And Package Boundaries ✅ Passed New files (164 + 73 lines) are well below size limits with single coherent responsibilities; existing files decreased (-62 net); AppKit bridge code correctly placed in app target per rules.
Cmux Swiftpm Lockfiles ✅ Passed PR makes no SwiftPM dependency changes, package reference changes, or .gitignore modifications; only adds local build file entries to Xcode project for new Swift source files.
Cmux Swift Logging ✅ Passed PR contains no prohibited logging: no print/debugPrint/dump/NSLog in production code, no MainActor Logger isolation issues, no unguarded sensitive data, and test files are appropriately scoped.
Cmux User-Facing Error Privacy ✅ Passed No user-facing error messages added; only test file contains fatalError in a developer-only utility function. PR confirms no localization updates needed.
Cmux Full Internationalization ✅ Passed PR contains no user-facing strings; all modified production code strings are debug-only within #if DEBUG blocks, new files have no user-facing text, test assertions are exempt, and no string catalo...
Cmux Swiftui State Layout ✅ Passed PR is entirely AppKit-focused with no SwiftUI state patterns. Uses proper @MainActor bridge classes, not SwiftUI observation. Check is not applicable.
Cmux Architecture Rethink ✅ Passed PR introduces caching and observer-based invalidation with clear single ownership per host, proper lifecycle teardown, MainActor isolation, and documented platform bridge justification. No timing r...
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PR modifies portal host hit-testing through refactored views and helper classes; no new standalone windows/panels/controllers added. Test-only NSWindow fixtures exempt per rule.
Cmux Source Artifacts ✅ Passed All PR files are intentional source code (5 Swift modules, 1 test file, 1 Xcode config). No local tool output, logs, screenshots, temp folders, caches, or build artifacts added.
Description check ✅ Passed The PR description is comprehensive and addresses all major template sections with substantial detail about the problem, solution, and testing approach.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-6586-portal-hit-test-cpu

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@austinywang
austinywang force-pushed the issue-6586-portal-hit-test-cpu branch from f2dcc74 to bb413c9 Compare June 22, 2026 09:33
@austinywang
austinywang force-pushed the issue-6586-portal-hit-test-cpu branch from bb413c9 to 176d651 Compare June 22, 2026 09:34
@greptile-apps

greptile-apps Bot commented Jun 22, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

Replaces the per-pointer-event recursive portal hit-testing with a cached split-divider region approach and a registry-only fast path for hover events. The motivation is clear: profile captures showed over 1.3 s of sampled time in hasBonsplitTabBarBackground / shouldPassThroughToPaneTabBar on pointer movement.

  • Tab-bar pass-through now short-circuits at the registry miss for mouseMoved/mouseEntered/mouseExited/cursorUpdate — the costly fallback class-name subtree scan is skipped entirely for hover events; non-hover events keep the bounded scan.
  • Split-divider hit-testing for both WindowTerminalHostView and WindowBrowserHostView collects PortalSplitDividerRegion snapshots once per geometry change, then hit-tests the flat array of cached window-coordinate rects on every pointer event; the cache is invalidated by frame/bounds notifications, isHidden KVO, subviews KVO, NSSplitView.didResizeSubviewsNotification, and direct host-frame overrides.
  • Test coverage adds PortalHitTestingPerformanceTests verifying registry-only hover routing, cache reuse on repeated pointer moves, stale-region rejection after split removal, and cache refresh on root/nested insertion and visibility changes.

Confidence Score: 5/5

Safe to merge. The changes are a well-bounded performance fix on the pointer-event hot path with no behavioral regressions.

The refactoring replaces recursive walks with a flat cached-rect array backed by multiple invalidation edges. The allLive() safety net ensures stale rects cannot persist. Six new tests cover the key liveness scenarios. No actor isolation mistakes, no blocking primitives, no user-facing text changes, and no new test-only seams in production source.

No files require special attention.

Important Files Changed

Filename Overview
Sources/PortalSplitDividerRegion.swift New @mainactor final class capturing a divider's window-coordinate rect, bounds clip, split-view weak ref, and isInHostedContent flag; collect() builds the cache and returns observed-view sets for the invalidator.
Sources/PortalSplitDividerCacheInvalidator.swift New @mainactor final class managing NSKeyValueObservation and notification tokens for frame/bounds/isHidden/subviews changes; uses nonisolated(unsafe) for teardown from deinit; correctly calls invalidate() before re-observing.
Sources/TerminalWindowPortal.swift Replaces per-event recursive walks with cached splitDividerRegions() path; adds did/willRemoveSubview overrides for cache invalidation.
Sources/BrowserWindowPortal.swift Same caching pattern as the terminal portal; preserves isInHostedContent distinction so hosted-inspector dividers remain interactive while app split dividers pass through.
Sources/BonsplitTabBarPassThrough.swift Adds eventType propagation; hover events return (false, false) immediately on a registry miss, skipping the entire hasBonsplitTabBarBackground subtree scan.
cmuxTests/PortalHitTestingPerformanceTests.swift Six tests covering registry-only hover routing, cached rect reuse, stale-cache rejection, root/nested insertion refresh, visibility-change invalidation, and container-move invalidation.
cmux.xcodeproj/project.pbxproj Registers the two new Sources files and the test file in the correct build phases and groups.

Reviews (9): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile

Comment thread Sources/TerminalWindowPortal.swift Outdated
Comment thread Sources/TerminalWindowPortal.swift
@austinywang
austinywang merged commit 814947b into main Jun 22, 2026
30 checks passed

This branch was successfully deployed

1 active deployment
Preview – cmux — ac4c3964 Deployed Jun 22, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Portal split/tab hit testing burns main thread on pointer movement

1 participant