Skip to content

sysmon: wake the watchdog on stop instead of waiting out its tick - #13

Open
johng wants to merge 1 commit into
robertsdotpm:mainfrom
johng:fix/sysmon-prompt-stop
Open

johng wants to merge 1 commit into
robertsdotpm:mainfrom
johng:fix/sysmon-prompt-stop

Conversation

@johng

@johng johng commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Problem

mn_fini stops the sysmon watchdog by setting runloom_sysmon_stop and joining the thread. Between scans the watchdog sleeps in a bare nanosleep, so the join waits out whatever is left of the current tick.

By the end of a run the hubs are idle, and the adaptive idle backoff (9ad5f98) has stretched the tick to its cap (wedge_ns/2, 25 ms by default). So every runloom.run() pays ~20 ms of teardown latency:

watchdog on watchdog off
empty runloom.run() (18 hubs, median of 15) 21.7 ms 2.0 ms

The watchdog is on by default on free-threaded 3.13+ (preemption forces it on), and in migration mode since the deadlock-census change. The cost lands on any code that calls run() repeatedly: the test suite, sync wrappers around run(), and short benchmark samples.

Fix

The watchdog now waits out its tick on a condvar (runloom_cond_timedwait_ns) instead of sleeping, and stop_join signals it. This is the same pattern mn_fini already uses for the hubs' idle waits (BUG #10).

  • The stop flag is re-checked in the watchdog and set in stop_join under the same lock. A stop that lands between the loop test and the wait can't lose its wakeup.
  • The lock and condvar are initialised per spawn and destroyed after the join. No watchdog is alive at either point, and a fork child may have inherited the lock held.
  • Tick length, idle backoff, wedge detection, preemption and the stack auto-cap are unchanged.
  • The crash-handler freeze (runloom_sched_freeze_for_crash) still only sets the flag, so it stays async-signal-safe. The watchdog exits at its next tick as before.

Test

tests/test_mn_teardown.py::test_fini_does_not_wait_out_the_sysmon_tick times mn_fini with RUNLOOM_SYSMON_MS=2000, which gives an 80 ms backed-off tick.

The watchdog's ticks are phase-locked to mn_init, so a fixed idle time always lands at the same point in the tick. The first version of the test used a fixed 0.3 s idle and passed on unfixed code, measuring ~7 ms. The test therefore sweeps the idle time across one full tick and averages.

  • Unfixed: 43.9 ms, fails
  • Fixed: ~0.7 ms, passes

Also green: test_cov100_sysmon, test_sysmon_oracle, test_preempt_timeslicer, test_cov100_resume_preempt, test_cov100_init_fini, test_mn_teardown, test_teardown_park_matrix, test_mn_deadlock_detect, test_cov_go_deadlock_differential, test_fork_safety, test_fork_balance_lock, test_monkey_fork (via tests/run_isolated.py, 3.14.4t, macOS arm64).

Benchmark impact

benchmark/workflows (from the workflow-compare branch), 18-core M-series, 3.14.4t PGO+LTO, 18 hubs. Mean of 2 interleaved runs × 5 samples, before → after on a tree with this change applied:

workload stock runloom runloom-mig
mixed 56.9k → 63.7k (+11.9%) 52.3k → 56.6k (+8.3%)
cpu_parallel 182M → 195M (+6.9%) +1.1%
pipeline +3.7% +5.0%
worker_pool −0.9% (noisy) +5.3%
fanout_io +2.2% +2.7%
sleepers +3.8% −1.0% (noise)

This isn't a throughput change in the scheduler: the harness times the whole run(), and the gain tracks how short each sample is. Mixed samples are ~70 ms, so a fixed 20 ms is ~13% of each one. With the fix, what those rows measure is the workload rather than teardown.

It also explains an apparent migration-mode regression. Enabling sysmon under migration made that column start paying the same ~20 ms that stock already paid.

mn_fini stops the sysmon watchdog by setting a flag and joining it, but the
watchdog waited between scans in a bare nanosleep, so the join waited for
the rest of the current tick.  By the end of a run the hubs are idle and the
adaptive idle backoff has stretched the tick to its cap (wedge_ns/2, 25 ms
by default), so every runloom.run() paid ~20 ms of teardown latency: an
empty run() took ~21.7 ms with the watchdog on and ~2 ms without it.

Wait out the tick on a condvar instead and have stop_join signal it.  The
stop flag is re-checked and set under the same lock, so a stop that lands
between the loop test and the wait cannot lose its wakeup.  The lock and
cond are initialised per spawn (no watchdog is alive then, and a fork child
may have inherited the lock held) and destroyed after the join.  Tick
length, idle backoff, wedge detection and preemption are unchanged; the
crash-handler freeze still just sets the flag (async-signal-safe) and the
watchdog exits at its next tick as before.

The ~20 ms showed up as throughput in short benchmark samples: the
workflow benchmark times the whole run(), so a ~70 ms mixed sample lost
10-15% to it.

Test: test_mn_teardown.py::test_fini_does_not_wait_out_the_sysmon_tick
measures mn_fini with a long backed-off tick, sweeping the idle time across
one tick (the ticks are phase-locked to mn_init).  Unfixed: 43.9 ms, fails.
Fixed: ~0.7 ms.
@johng
johng marked this pull request as ready for review September 23, 2026 20:51
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