Skip to content

perf: skip geometry reconciliation on the text-input redraw - #226

Merged
arzafran merged 1 commit into
mainfrom
perf/183-m2-fix-a
Jul 31, 2026
Merged

perf: skip geometry reconciliation on the text-input redraw#226
arzafran merged 1 commit into
mainfrom
perf/183-m2-fix-a

Conversation

@arzafran

Copy link
Copy Markdown
Member

Closes the remaining half of #183. Measured with the in-process sampler, not the socket harness.

What this does

After every printable keystroke the keyDown path called forceRefresh, which reasserts the surface's display id and runs forceRefreshSurface()'s geometry reconciliation. Both exist for topology changes — split close/reparent, and the stuck-vsync state after wake-from-sleep. A character causes neither: typing changes grid contents, not the surface's size or which display it lives on.

Adds requestRedrawAfterInput(), which only calls ghostty_surface_refresh, and uses it at the text-input call site. forceRefresh is unchanged and still used everywhere else.

Early-out conditions are deliberately identical to forceRefresh's, so when a redraw happens is unchanged — only the work done differs.

The numbers

Two before/after pairs, 400 keystrokes each, timed in-process at the call site:

metric forceRefresh requestRedrawAfterInput
p50 0.318 / 0.159 ms 0.004 / 0.003 ms
p95 0.602 / 0.269 ms 0.036 / 0.008 ms
p99 0.726 / 0.369 ms 0.113 / 0.020 ms
mean 0.330 / 0.169 ms 0.011 / 0.004 ms

Complete separation on every metric across both pairs — roughly 40-100x.

Honest about magnitude: the absolute saving is ~0.15-0.3 ms per keystroke. Not transformative on its own, but it is pure waste on the path this codebase treats as sacred, and removing it costs nothing.

Why measuring this took three attempts

The first attempt used tests/test_workspace_churn_up_arrow_lag.py and produced four runs of completely overlapping ranges. That harness measures a socket RPC round trip — IPC, JSON, DispatchQueue.main.sync, AppKit dispatch, then the work. The transport is order-milliseconds; the effect is ~0.3 ms. It was structurally incapable of resolving this, and more samples would not have helped.

Hence #219 (the in-process sampler), #223 (the CI step that runs it) and #224 (its failure diagnostics). #225 fixed a crash those uncovered on the way — the typing profiler terminated the app whenever it was enabled, because eventFields read keyCode on non-key events.

Test plan

  • Two before/after pairs, no overlap on any metric
  • xcodebuild -scheme programa** BUILD SUCCEEDED ** (verified from log contents)
  • CI green

Branch is perf/183-m2-fix-a rather than something tidier: the original needed a rebase onto the crash fix, and force-push was correctly denied, so the same single commit went to a fresh name.

After every printable keystroke the keyDown path called forceRefresh, which
reasserts the surface's display id and runs forceRefreshSurface()'s geometry
reconciliation. Both exist for topology changes -- split close/reparent, and the
stuck-vsync state after wake-from-sleep -- and a character cannot cause either.
Typing changes grid contents, not the surface's size or which display it is on.

Adds requestRedrawAfterInput(), which only calls ghostty_surface_refresh, and
uses it at the text-input call site. forceRefresh is unchanged and still used
everywhere else.

The early-out conditions are deliberately identical to forceRefresh's, so when a
redraw happens is unchanged; only the work done differs.

This half was unmeasurable until now: the lag harness sends an arrow key, which
never satisfies shouldRefreshAfterTextInput, so forceRefresh never ran under it.
#216 added a lag_combo input so a printable key can drive the same harness
through the real responder chain.

Refs #183
@arzafran
arzafran merged commit 9ce4d8c into main Jul 31, 2026
24 checks passed
@arzafran
arzafran deleted the perf/183-m2-fix-a branch July 31, 2026 03:30
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