Repository navigation
renderer: an off-main present begins a Core Animation transaction only for a frame it shows (sync line) - #80
Merged
lawrencecchen merged 2 commits intoNov 5, 2026
Conversation
…its no Core Animation transaction (red) cmux FRAME OWNER REFACTOR S4 profile 4b41c42c: with the embedder presenting host sizes, each frame of a new size fails setSurfaceOffMain's checks and goes through main, but the off-main attempt first begins and commits a transaction: one empty commit on the GPU completion thread per present (405 ms of CA::Context::commit_transaction in a held-key round). The test counts the presenting thread's commits (the CommitOrder swizzle) for each refusal: no host size, another size than the host's, a layer that shows another size, a present the embedder has not committed, and an older frame. A frame it shows still commits once, outside the lock. (cherry picked from commit 694f7ff)
…y for a frame it shows setSurfaceOffMain began and committed a transaction before its checks, so each frame that went through main (a new size while the embedder presents host sizes, a frame the embedder has not committed, another size than the shown one) or that a newer frame superseded still committed an empty transaction on the GPU completion thread. cmux FRAME OWNER REFACTOR S4 profile 4b41c42c: with motion.frameOwner.terminals on, 405 ms of CA::Context::commit_transaction in one held-key round of 14 terminals, against main's own commit. The checks now run first, read-only, under the Core Animation lock (offMainDecisionLocked). Only a frame it shows begins a transaction, under the lock, takes the contents, and commits after the unlock: the no-commit-under-the-lock invariant holds. Renderer mode (the default) gets the same: a superseded frame no longer commits. (cherry picked from commit 7f2dcbf)
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The same two commits as #79 (cherry-picked with -x), for the sync line
release/cmux-sync-20261104that cmux #35478 pins. #79 has the independent review receipt (SHIP at 7f2dcbf) and the red and green fork runs; the layer file is identical on both lines.IOSurfaceLayer.setSurfaceOffMainbegan and committed a Core Animation transaction before its checks. So every frame that it did not show still committed an empty transaction on the GPU completion thread:cmux FRAME OWNER REFACTOR S4 measured this with Time Profiler (cmux step
4b41c42c, held keys-stress, 14 terminals,motion.frameOwner.terminalson): 405 ms ofCA::Context::commit_transactionunder the GPU completion queue in one round. Presents per round went from 84 (switch off) to about 850 (switch on). The new IOSurface per size cost only about +100 ms in the same round, and WindowServer'screate_surfacewas the same with the switch off and on.The checks now run first, read-only, under the Core Animation lock (
offMainDecisionLocked). Only a frame that it shows begins a transaction. The transaction begins under the lock (a begin commits nothing), takes the contents, and commits after the unlock. So the invariant "no commit while holding the lock" still holds. Renderer mode (the default) gets the same change: a superseded frame no longer commits.No C API change. No change to which frame is shown, or when.
Testing
694f7ffb7addshost present: an off-main present that does not show its frame commits no transaction. It counts the presenting thread's commits with the existingCommitOrderswizzle, for each refusal (no host size, another size than the host's, a layer that shows another size, a present the embedder has not committed, an older frame), and expects one commit, outside the lock, for a frame that it shows.host present:filter only (scratch branches):694f7ffb7, run 41953548703: 110/111 tests pass. Only the new test fails, atIOSurfaceLayer.zig:862,expected 0, found 1(a layer that shows another size: the off-main attempt committed an empty transaction).7f2dcbf36, run 41953551466: 111/111 tests pass, and lib-vt 97/97.Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Fixes
IOSurfaceLayer.setSurfaceOffMainso an off-main present only begins a Core Animation transaction for a frame it actually shows. Previously every frame that went through main (new size, uncommitted embedder present, size mismatch) or was superseded still committed an empty transaction on the GPU completion thread; now the read-only checks run first under the Core Animation lock, and the transaction commits only after the unlock, preserving the no-commit-while-locked invariant. Adds a test asserting refusals commit nothing on the presenting thread while a shown frame commits exactly once, outside the lock.Written for commit ec398e8. Summary will update on new commits.