Repository navigation
ghostty-next: pin 6cc749a8a, a frame the embedder presents itself wakes no main-thread present (S4 flip prerequisite) - #35900
Conversation
…edder presents itself wakes no main-thread present GhosttyNextKit and the ghostty-next gitlink move together from 7d43bb68b to 6cc749a8a on release/cmux-9ce9b7d0-e2: - #72: ghostty.h's lock-span rule for ghostty_surface_host_frame's present (comments only); - #73: each surface's thread wakes counted by source in ghostty_surface_render_counters (new fields at the end; cmux reads the struct by field); - #75: with the embedder presenting host sizes (motion.frameOwner.terminals on), a frame kept for its present that the layer cannot show in its bounds goes to no main-queue block. That block was dropped on main: one main-thread wake per moving terminal and frame, the held-key cost that keeps the switch off (keys-stress A/B at 8255dfd: ON missed more refreshes in 6 of 6 rounds). Release xcframework-6cc749a8a53b1d62465e0faf848e430a82f690aa-apple-v6 (run 41922548637), zip sha256 018d2bdb8b7f26c9e97ff6732b4e80e52f24ca3086e456efe0fb6d8aa38f4a40 (SHA256SUMS). None of the changed fork files is in the libghostty-vt link graph, so vt-link-graph.json changes only its commit; package-notices names the new paths (src/renderer, src/termio/Thread.zig, src/apprt/embedded.zig, include/ghostty.h, NEXT.md; no license file or dependency change).
|
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)
Shipped in:
|
|
Review (independent subagent): SHIP at 44ab81a I reviewed the exact head in a new detached worktree, read only. I did not build, test or run the app. I found no correctness defect in the pin, the API use, the notice files or the doc. The PR description has errors. Correct it before you add What I checked (correct)
Findings (description only; no push is necessary)F1. The Testing section is out of date. It says that the A/B steps "run" and "Not run yet: the suites and the gate A/B". All four steps are done with exit 0 ( F2. The held-key A/B does not support the Summary sentence "This is the held-key cost that keeps the switch off." Missed refreshes, OFF / ON:
Held keys-stress at this head: ON is higher in 6 of 6 rounds (OFF 6, 6, 10, 6, 5, 12; ON 13, 15, 16, 16, 13, 13), and 86 > 1.25 x 45 + 3. This row fails your gate rule, almost the same as at the baseline. The pin closes the single-press keys-stress gap (ON 13 / OFF 17; it was 23 / 14), but not the held-key gap. Why it matters: the flip decision uses this PR. Fix: in the Summary, say that fork PR 75 removes the per-frame main wake and fixes the single-press rows, and that held keys-stress still fails the gate. Cite the held baseline step above. Optional, in a later PR: at F3. Your own rule needs one rerun of each of these rows:
I do not hold the PR for the OFF rows. With the switch off, the fork adds no main-thread work, so these are most probably run-to-run noise. But the body says the OFF arm "must also pass". Record the reruns, or remove that sentence. F4. REVIEW.md section 12: add a Coordination with #35478
Base finding (not caused by this PR)
Not done by me: no build, no test, no app. I read the step logs and the A/B reports with Review (independent subagent): SHIP at 44ab81a |
|
Base finding, not caused by this PR:
|
Summary
FRAME OWNER REFACTOR S4, the step before the default flip of
motion.frameOwner.terminals(cmux-ad m6). GhosttyNextKit and the ghostty-next gitlink move together from7d43bb68bto6cc749a8aonrelease/cmux-9ce9b7d0-e2:ghostty.htext only (the lock-span rule ofghostty_surface_host_frame's present).ghostty_surface_render_counters_s; cmux reads the struct by field, no Swift change).With the switch off (the default), nothing a person sees changes: renderer mode is unchanged in the fork.
The release:
xcframework-6cc749a8a53b1d62465e0faf848e430a82f690aa-apple-v6(run 41922548637), zip sha256018d2bdb8b7f26c9e97ff6732b4e80e52f24ca3086e456efe0fb6d8aa38f4a40(its SHA256SUMS). None of the changed fork files (src/renderer, src/termio/Thread.zig, src/apprt/embedded.zig, include/ghostty.h, NEXT.md) is in the libghostty-vt link graph, sovt-link-graph.jsonchanges only its commit, andpackage-notices.jsonnames the new paths (no license file or dependency change).Coordination: cmux #35478 (upstream sync, risk:high) moves the same pin to
a6b9048e9onrelease/cmux-sync-20261104, which does not have this fix. The same fork changes are merged on that line too (manaflow-ai/ghostty-next#76 and #78, tip58f44beb579). Fork #77 (a test and one shared helper, no behavior change) merged after this pin, asae7c1f741; this pin does not contain it. If this PR lands first, #35478 must pin58f44beb579or later; if #35478 lands first, this PR moves to the sync line (comment 6644453471 on #35478).Testing
15c50cdb6a3a9a3150dddcc2: all 3 suites pass (TerminalKeyResizeFrameTests 2 tests, TerminalFrameOwnerMainThreadTests 1, TerminalSurfaceCommitSizeTests 6; key resize misplaced 0, unsized 0, stretched 0, PTY grids per terminal [1, 1], main ms per frame p50 0.32, p95 0.46, max 0.77); the 42 terminal suites: step80c78c6554c4b3a313463af7: all 42 suites pass.--key-repeat=6): single presses stepf39f3cb82c7418e4b2decd3e, held keys stepe16d78afdddfd1f84690e79c(both run). The first single-press stepac3662e31afed4a0f3716afcfailed on the fleet, not in the code: the warm slot on cmux24 had a GhosttyNextKit precompiled module built from the oldghostty.h("ghostty.h has been modified since the module file ... needs to be rebuilt"); the step dropped that.build, andf39f3cb8reruns the same SHA. Any warm slot with a module of the old pin can fail once like this after the pin moves. The gate rule (pixel-perfect): 8 rows; a row fails if ON is higher in 4 or more net rounds, or ON's missed total is more than 1.25 x OFF's + 3, or ON's motion main p95 is more than 1.15 x OFF's; one rerun per failing row; this PR's OFF arm must also pass that rule against the baseline's OFF arm. Baseline at8255dfd7ac42(Frame owner S4: terminals follow their pane in every frame of a key resize (switch-on path; default stays off) #35470's merge, the old pin), single presses, missed refreshes per round: keys-stress OFF 1, 2, 2, 3, 4, 2 (14) / ON 4, 3, 3, 5, 5, 3 (23), ON higher in 6 of 6 rounds; keys 10 / 15; STRESS drag 7 / 11; light drag 0 / 0 (step0a1e445d2262d43d8fd0a0ab).host present:filter on the fork's CI): red run 41915563142 (the two embedder-mode tests fail,expected 0, found 1), green run 41917705365 (109/109, lib-vt 97/97).Gate A/B results at this head (cmux24-perf, rule from pixel-perfect; missed refreshes OFF / ON):
e16d78afdddfd1f84690e79c): keys 23 / 15 pass, light 2 / 2 pass, STRESS drag 11 / 9 pass, keys-stress 45 / 86 FAIL (ON higher in 6 of 6 rounds). The rule's one rerun (step41988e97abde8ca7dfc9f9c9): keys-stress 45 / 76, ON higher in 6 of 6 rounds: FAIL confirmed. The baseline at8255dfd7ac42was 46 / 89 (stepee5d440bbac9c256a471d656). So Release v0.46.0 #75 alone does not remove the held-key cost; the switch stays off (the flip is a separate PR).f39f3cb82c7418e4b2decd3e): keys 16 / 12 pass, light 0 / 4 FAIL rule (b) (all in one round), STRESS drag 8 / 9 pass, keys-stress 17 / 13 pass.8255dfd7ac42): single keys 10 -> 16 missed, motion main p95 6.75 -> 9.21 ms (FAIL); held STRESS drag 6 -> 11 (FAIL rule (b)); the other 6 rows pass. The only change on the OFF path is Multi-tab notifications #73's per-surface wake counters. Spread check (Lead's order: is OFF 6 vs 11 inside the run-to-run spread?): yes, PASS. The OFF arm alone, base and pin interleaved (base, pin, pin, base) on cmux24-perf, 6 rounds, STRESS drag, keys and keys-stress in one launch, the same bench file on both. The base is this commit's parentd8936fb5a11. Steps: base3e90283a, pin3f6ba259, pin19db9742, baseb45bdbf2. Missed refreshes (pixel-perfect's two-build rule: a row fails only if both pin runs are worse than both base runs and the means differ by more than 1.25 x + 3, or motion p95 by more than 1.15 x):So the earlier OFF 6 vs 11 on the held STRESS drag was run-to-run spread (and a base 12 commits older), not Multi-tab notifications #73's counters. No change moves the counters.
Single light ON 4 / OFF 0 is flip-only (the ON arm), so it goes to the flip PR's gate, not to this PR. If the difference is outside the spread, Multi-tab notifications #73's counters move off the hot path in the fork first, and this PR waits for that.
Observed / Not run
prepush-gate.pypasses.cmux-next swift testis red on run 41925421646. 12 of its 17 failing tests fail the same way on 4 unrelated PRs of the same hour (41941156936, 41940386041, 41939732509, 41937839721). The other 5 (countsByDomainMatchInventory,exportIsFresh,reportIsFresh,checkedInSchemaMatchesTheCatalog,paletteTitlesUseTitleCase) check generated action and schema files that this PR does not touch; the merge ref was built at 22:27Z from a feat state with those files stale, which the base autofix333c3976d0f8(23:24Z) regenerated.Frames: terminal: with the switch off (the default), what the person sees in each frame of a key resize or a divider drag does not change: the same frames as at the old pin
7d43bb68b; the only OFF-path change is #73's wake counters. With the switch on (not shipped), a frame that the layer cannot show at its bounds no longer wakes the main thread; the frames shown are the same.Changelog
none