Skip to content

The pilot settles on owed work, not on a CPU-time guess - #257

Merged
trissim merged 7 commits into
mainfrom
perf/settle-not-sleep
Nov 7, 2026
Merged

trissim merged 7 commits into
mainfrom
perf/settle-not-sleep

Conversation

@trissim

@trissim trissim commented Nov 6, 2026

Copy link
Copy Markdown

The pilot settles on owed work instead of sleeping on a CPU-time guess.

Pilot.press slept wait_for_idle(0) twice per key (at least 20 ms each, looping in 20 ms steps while the loop thread burned more than 1 ms per step), and every pause slept it again. The exact signal was already there: each queue says Owing after a put and Settled when its pump finds it empty.

main this branch
full suite (-n auto, snapshots excluded) 39.8 s, 21 failed 26.5 s, 22 failed (only test_slug 4/5 and a gc-timed worker test differ, both seen failing on main runs)
test_pilot.py, test_data_table.py, test_app.py, serial 27.9 s 12.8 s
Toad's 5 keypress-heavy files, serial, one worker 135.9 s ~88 s

One owner for "the app has handled what it was given"

App._settle repeats until nothing is owed:

  1. one loop turn, so input a driver handed over through call_soon_threadsafe is on its queue;
  2. waits behind every owing pump on the screen; work that posts more makes its target owe again;
  3. delivers a resize still waiting on its 1/120 s debounce;
  4. runs the screen update itself (Screen._update_now: the timer's update, the rest of a layout split into steps, then a prompt so the idle check resumes the timer for work requested meanwhile), at most SETTLE_UPDATES (2) times: a layout's own consequences land in the second, and a widget that asks for a layout on every layout never stops asking.

It stops when the app raises or exits (the app's loop task ending is part of every wait). A running animation is not owed work: it may run for a minute, and callers who need it finished use Pilot.wait_for_animation.

press (before its first key and after each), pause, exit, the animation waits and run_test all call it. Pilot._wait_for_screen is deleted; WaitForScreenTimeout moves to app.py beside its raiser; textual._wait has no caller left and goes with its test.

Bugs the sleep was hiding

  • The key raced its own wait. _press_keys sent keys through driver.send_message, a thread-safe hop that lands a loop turn later. Keys are now posted on the loop _press_keys already runs on.
  • A key that stops the app hung on the dead app's animator; only assert driver is not None got it out.
  • call_after_refresh strands its callback while a refresh stays pending without a published frame (my first cut waited on it: 49 suite tests sat on the 30 s round timeout). The settle runs the update instead of waiting for a frame.
  • text_wrap and text_overflow were declared repaint-only but decide how many lines an auto-sized widget measures. Their test passed on main only because the startup resize, still behind its debounce, re-laid the screen out after the change.

Tests that leaned on the sleep

  • Tabs moves its underline UNDERLINE_DELAY (0.02 s) after activation: the literal is now on the class, and the animation tests pause for it.
  • test_log_from_worker's app exited itself after one refresh, racing the thread it tested.
  • test_data_table pauses through the pilot instead of importing wait_for_idle.
  • Toad: Neovim's mode after escape and a click aimed during a sidebar scroll animation now wait for their receipts (Two tests wait for what they assert on, not for a pilot sleep to cover it toad#1610, merged).

trissim and others added 6 commits November 6, 2026 17:58
Pilot.press slept wait_for_idle(0) twice per key (at least 20 ms each, looping in 20 ms
steps while the loop thread burned 1 ms) and Pilot.pause slept it again, beside the
exact signal the queue already keeps: Owing after a put, Settled when its pump finds it
empty. The sleep was hiding two bugs, which are fixed here too.

- App._settle owns "the app has handled what it was given". It waits behind every owing
  pump on the screen until a round finds none, then waits for the frame those messages
  asked for (layout and paint run on the update timer, not a queue) through
  call_after_refresh, and checks the queues again. It stops when the app raises or exits:
  the app's own loop task ending is part of the wait.
- press, pause, exit, wait_for_scheduled_animations and run_test's first settle all call
  it; Pilot._wait_for_screen is deleted, and WaitForScreenTimeout moves to app.py beside
  its raiser.
- _press_keys posts the key on the loop it already runs on. driver.send_message hops
  through call_soon_threadsafe, so the key landed after the wait that followed it; the
  20 ms sleep covered the race.
- A key that stops the app ends _press_keys. It used to wait on the dead app's animator;
  only `assert driver is not None` got it out.
- textual._wait has no caller left and goes with its test; test_data_table pauses through
  the pilot.

tests/test_pilot.py, test_data_table.py, test_app.py: 265 passed in 12.8 s
(main: 265 passed in 27.9 s).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
…not owed work

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
… queues

The full suite found the settle returning before work that was owed but not queued:

- Driver input is posted from a loop callback (call_soon_threadsafe): each round first
  gives the loop one turn.
- A terminal resize waits on its 1/120 s debounce: the settle delivers it.
- Layout and paint run on the screen's update timer: instead of waiting on
  call_after_refresh, whose callbacks a pending refresh that never publishes strands for
  good (49 suite tests hit the 30 s round timeout that way), the settle runs the update
  itself through Screen._update_now, which also awaits a layout split into steps and
  prompts the screen so its idle check resumes the timer for work requested meanwhile.
- At most SETTLE_UPDATES (2) updates per settle: a layout's own consequences land in the
  second, and a widget that asks for a layout on every layout never stops asking.
- _press_keys settles once before its first key: a typed key arrives after the app has
  handled what it was given (a screen push, a mount), and the command palette's tests
  showed the key landing before the palette existed.
- Pilot.pause and the animation waits settle instead of running a bare timer update.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
…made on another thread

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
…y, the worker's log

- Tabs starts moving its underline UNDERLINE_DELAY (0.02 s) after a tab activates, a
  timer, not owed work; the literal moves onto the class and the animation tests pause
  for it instead of riding the pilot's old 20 ms sleep.
- test_log_from_worker's app exited itself after its first refresh, racing the thread
  whose log the test asserts; run_test closes the app, so the self-exit goes and the
  test waits for the worker.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
…to-sized widget measures

Declared repaint-only, they left an auto-height widget at its old height. On main the
test that changes them passed only because the startup resize, still behind its 1/120 s
debounce, re-laid out the screen after the change; the settle delivers resizes on time.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
Copilot AI balanced review requested due to automatic review settings November 6, 2026 23:46

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

_on_click and action_press each restated the -active class test. A caller that
presses the same button again within its 0.2 s effect is refused; with the
pilot's sleeps gone, Toad's log controls did exactly that and need the receipt.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
trissim added a commit to OpenHCSDev/toad that referenced this pull request Nov 7, 2026
Textual's Button refuses a press during the previous press's 0.2 s effect; the
test pressed Wrap and Latest twice inside it once the pilot stopped sleeping.
One press helper waits on Button.accepts_press (OpenHCSDev/textual#257) and
replaces the hand-written -active wait.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim
trissim merged commit e54d6c4 into main Nov 7, 2026
4 of 25 checks passed
trissim added a commit to OpenHCSDev/toad that referenced this pull request Nov 7, 2026
…on owed work)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
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.

2 participants