Skip to content

Avoid cloning occluders during graphics cache checks - #11746

Merged
lawrencecchen merged 1 commit into
mainfrom
feat-tui-graphics-context-cache
Sep 3, 2026
Merged

lawrencecchen merged 1 commit into
mainfrom
feat-tui-graphics-context-cache

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary:

  • Compare graphics cache context fields directly against the freshly computed occluder slice.
  • Move the original occluder Vec into the cache only when the context changes.

Validation:

  • git diff --check
  • Remote Blacksmith cargo test --locked graphics_scene_cache (2 tests passed)
  • Hosted cmux-tui focused verification dispatched for this commit

No new test was added because allocation behavior has no independent user-visible assertion; existing graphics scene cache tests cover the unchanged and changed context paths.


Summary by cubic

Stops cloning the occluder list when checking the graphics scene cache. The cache check now compares the cached fields directly against the freshly computed occluders and moves the Vec into the cache only when the context changes, cutting per-frame allocations with no behavior change.

Written for commit 336cab7. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Performance
    • Improved graphics scene cache comparisons by reducing temporary state and redundant processing.
    • Improved rendering update efficiency when occlusion information changes.

@vercel

vercel Bot commented Sep 3, 2026 •

Copy link
Copy Markdown

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

Project Deployment Actions Updated
cmux166 Canceled Canceled Sep 3, 2026 3:29pm UTC
cmux41 Canceled Canceled Sep 3, 2026 3:29pm UTC

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@coderabbitai

coderabbitai Bot commented Sep 3, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: 455c625a-43d0-4ec2-8218-c65ea3d8e412

📥 Commits

Reviewing files that changed from the base of the PR and between 18bec20 and 336cab7.

📒 Files selected for processing (1)
  • cmux-tui/crates/cmux-tui/src/app.rs

Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

The graphics scene context check computes occluders once and compares them directly with cached values. The cache update moves the computed occluders into the stored GraphicsSceneContextKey.

Changes

Graphics context cache

Layer / File(s) Summary
Direct occluder comparison and cache update
cmux-tui/crates/cmux-tui/src/app.rs
The context check compares cached fields directly with current values. The cache update moves the computed occluders into the stored key.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 336ca

This change removes a redundant occluder-vector clone while preserving graphics cache invalidation behavior. The updated cache paths are covered by existing focused tests, with no remaining merge-readiness risk identified.

Suggested reviewers: lawrence703

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: avoiding unnecessary cloning of occluders during graphics cache checks.
Description check ✅ Passed The description explains what changed, why it changed, and how it was validated. It does not use the template headings and omits the review trigger and checklist, but the required technical context is…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation ✅ Passed PASS: The pull request changes only cmux-tui/crates/cmux-tui/src/app.rs, a Rust file. The exact commit has no changed Swift files or Swift actor-isolation declarations. The Swift actor-isolation che…
Cmux Swift Blocking Runtime ✅ Passed PASS: The pull request changes only cmux-tui/crates/cmux-tui/src/app.rs, which is Rust. The parent-to-HEAD diff introduces no Swift files or Swift blocking/timing primitives. The custom check is the…
Cmux Browser Automation Off-Main ✅ Passed PASS: The commit changes only cmux-tui/crates/cmux-tui/src/app.rs. The diff updates graphics occluder cache comparison and GraphicsSceneContextKey construction. It does not modify `Sources/Termina…
Cmux Expensive Synchronous Load ✅ Passed PASS: The pull request changes only cmux-tui/crates/cmux-tui/src/app.rs, a Rust file. The diff adjusts graphics occluder cache comparison and ownership. It adds or moves no production Swift code and…
Cmux Cache Substitution Correctness ✅ Passed PASS. The pull request changes only cmux-tui/crates/cmux-tui/src/app.rs, which is Rust; the changed-path check found no Swift, TypeScript, or JavaScript files. The custom check applies only to produ…
Cmux No Hacky Sleeps ✅ Passed PASS: The pull request changes only cmux-tui/crates/cmux-tui/src/app.rs, a Rust source file. The custom check applies to TypeScript, JavaScript, shell, and non-Swift build/runtime scripts. The diff …
Cmux Algorithmic Complexity ✅ Passed PASS. The only changed file is Rust source, and the diff replaces a cloned Vec<Rect> with one direct slice comparison. That comparison remains linear and does not add a nested scan, repeated sorting…
Cmux Swift Concurrency ✅ Passed PASS: The pull request changes only cmux-tui/crates/cmux-tui/src/app.rs, a Rust file. The commit diff has no .swift paths and introduces no Swift concurrency patterns. The Swift concurrency check …
Cmux Swift @Concurrent ✅ Passed PASS: The pull request changes only cmux-tui/crates/cmux-tui/src/app.rs, a Rust file. The exact diff contains no Swift files, Swift functions, or Swift call sites. Therefore the @concurrent check …
Cmux Swift Package Boundaries ✅ Passed PASS: The pull request changes only cmux-tui/crates/cmux-tui/src/app.rs, which is Rust. No Swift file or Swift package boundary is changed. The custom check applies only to production Swift changes.
Full details: Description check

Explanation

The description explains what changed, why it changed, and how it was validated. It does not use the template headings and omits the review trigger and checklist, but the required technical context is present.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 too large.)

Full details: Cmux Swift Actor Isolation

Explanation

PASS: The pull request changes only cmux-tui/crates/cmux-tui/src/app.rs, a Rust file. The exact commit has no changed Swift files or Swift actor-isolation declarations. The Swift actor-isolation check is therefore inapplicable.

Full details: Cmux Swift Blocking Runtime

Explanation

PASS: The pull request changes only cmux-tui/crates/cmux-tui/src/app.rs, which is Rust. The parent-to-HEAD diff introduces no Swift files or Swift blocking/timing primitives. The custom check is therefore not applicable.

Full details: Cmux Browser Automation Off-Main

Explanation

PASS: The commit changes only cmux-tui/crates/cmux-tui/src/app.rs. The diff updates graphics occluder cache comparison and GraphicsSceneContextKey construction. It does not modify Sources/TerminalController.swift or Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Wire/ControlCommandExecutionPolicy.swift, and the changed hunk contains no browser socket command, WebKit/page wait, worker routing, or main-actor routing change. The custom check is therefore not applicable.

Full details: Cmux Expensive Synchronous Load

Explanation

PASS: The pull request changes only cmux-tui/crates/cmux-tui/src/app.rs, a Rust file. The diff adjusts graphics occluder cache comparison and ownership. It adds or moves no production Swift code and no synchronous agent-history load.

Full details: Cmux Cache Substitution Correctness

Explanation

PASS. The pull request changes only cmux-tui/crates/cmux-tui/src/app.rs, which is Rust; the changed-path check found no Swift, TypeScript, or JavaScript files. The custom check applies only to production Swift, TypeScript, and JavaScript changes, so it is not applicable.

Full details: Cmux No Hacky Sleeps

Explanation

PASS: The pull request changes only cmux-tui/crates/cmux-tui/src/app.rs, a Rust source file. The custom check applies to TypeScript, JavaScript, shell, and non-Swift build/runtime scripts. The diff does not introduce a covered fixed sleep, timer, polling wait, or delayed dispatch.

Full details: Cmux Algorithmic Complexity

Explanation

PASS. The only changed file is Rust source, and the diff replaces a cloned Vec&lt;Rect&gt; with one direct slice comparison. That comparison remains linear and does not add a nested scan, repeated sorting/filtering, or a slower algorithm. The existing per-placement occluder scan in graphic_placements_for_area is unchanged. The diff also moves the original vector into the cache only on context change, so it does not worsen existing complexity.

Full details: Cmux Swift Concurrency

Explanation

PASS: The pull request changes only cmux-tui/crates/cmux-tui/src/app.rs, a Rust file. The commit diff has no .swift paths and introduces no Swift concurrency patterns. The Swift concurrency check is therefore not applicable.

Full details: Cmux Swift `@Concurrent`

Explanation

PASS: The pull request changes only cmux-tui/crates/cmux-tui/src/app.rs, a Rust file. The exact diff contains no Swift files, Swift functions, or Swift call sites. Therefore the @concurrent check is not applicable.

✨ 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 feat-tui-graphics-context-cache

Warning

Some tools did not complete. Review the errors below.

🔧 ast-grep (0.45.2)
cmux-tui/crates/cmux-tui/src/app.rs

ast-grep timed out on this file


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.

@lawrencecchen
lawrencecchen force-pushed the feat-tui-graphics-context-cache branch 3 times, most recently from 0a707f9 to 9dbdf5d Compare September 3, 2026 01:53
@blacksmith-sh

This comment has been minimized.

@lawrencecchen
lawrencecchen force-pushed the feat-tui-graphics-context-cache branch 4 times, most recently from 2ff6eaa to b348b77 Compare September 3, 2026 02:31
@cursor

cursor Bot commented Sep 3, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@lawrencecchen
lawrencecchen force-pushed the feat-tui-graphics-context-cache branch 3 times, most recently from 4f9079a to 6fa83c0 Compare September 3, 2026 03:11
@cursor

cursor Bot commented Sep 3, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@lawrencecchen
lawrencecchen force-pushed the feat-tui-graphics-context-cache branch 4 times, most recently from ebcc31a to c366104 Compare September 3, 2026 04:21
@cursor

cursor Bot commented Sep 3, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@lawrencecchen
lawrencecchen force-pushed the feat-tui-graphics-context-cache branch from c366104 to 336cab7 Compare September 3, 2026 04:28
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Correctness and allocation note: graphic_occlusion_rects() still creates one fresh Vec<Rect> per graphics emit call. This patch removes only the duplicate occluders.clone() allocation. The temporary vector is borrowed while placements are built, then moved into GraphicsSceneContextKey only when context_changed is true. Cache invalidation paths and key fields are unchanged, and the existing cache tests cover unchanged and changed context paths. No allocation benchmark was added because the test API has no stable allocation counter. A future profile can assess reusing scratch storage or comparing iterators.

@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@lawrencecchen
lawrencecchen merged commit 39a2f3a into main Sep 3, 2026
29 of 31 checks passed
rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 3, 2026
829c6af Fix Linux iOS release origin guard (manaflow-ai#11754)
a013ecb Guard PTY child during terminal host startup (manaflow-ai#11425)
1b0f61d cloud: lgx 12 vCPU / 24 GB size, 20260903c ladder, Freestyle preconnect, one-exec attach (manaflow-ai#11783)
d300944 Fix RemoteResumeBindingTests legacy snapshot fixture (manaflow-ai#10338)
39a2f3a Avoid cloning graphics occluders for cache checks (manaflow-ai#11746)
7a5e0c5 ci(tui): make ignored coverage and browser smoke explicit (manaflow-ai#11752)
18bec20 ci: poll Gatekeeper after stapling the Computer Use helper (manaflow-ai#11778)
e473108 ci: simplify reusable TUI package checkouts (manaflow-ai#11749)
0915892 fix(tui): reconcile every PyPI wheel after upload (manaflow-ai#11740)
1cbd743 fix: make SSH auth cleanup fork resilient (manaflow-ai#11497)

# Conflicts:
#	.github/workflows/cmux-tui-build-package.yml
#	.github/workflows/cmux-tui-nightly.yml
#	.github/workflows/cmux-tui-sdks.yml
#	.github/workflows/cmux-tui.yml
#	.github/workflows/tui-publish-pypi.yml

This branch was successfully deployed

2 active deployments
Preview – cmux41 — 336cab7a Deployed Sep 3, 2026 by vercel[bot]
Preview – cmux166 — 336cab7a Deployed Sep 3, 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.

1 participant