perf(runtime): stop reverse-resolving the bind address, and stop paying for idle waits - #236
Merged
Conversation
The macOS test leg ran 415s against Ubuntu's 92s, and the whole quality gate waited on it. Almost none of that was work: profiled locally, the suite burned 135s of wall clock for 60s of CPU. Three things were charging for nothing, none of them macOS-specific in cause — only in price, because process creation and scheduling are what macOS makes dearest. `serve_forever()` was called bare, so `socketserver` polled for shutdown every 0.5s and each `shutdown()` blocked until the accept loop next woke. The suite stands up about a hundred servers; that alone was 28s of `test_http_api`'s 44s, all of it asleep. Test servers now poll at 5ms, and the shipped serve path keeps the stock interval — the trade is right in a test and wrong in a daemon. `PageJsHarness` spawned one node per check, 425 of them. Measured, node startup was 40ms of each check's 44ms: the spawn was the bill, not the 260KB of page script it ran. One long-lived worker now runs every check in a fresh `vm` context, which is the same isolation a fresh process gave. And a real defect behind the third: every POST that read its body and then refused it re-drained bytes already gone, so `read1` blocked until REJECT_DRAIN_SECONDS gave up. A validation 400 on `/api/ask` cost 252ms of its handler thread, on the shipped route, not only under test. It now costs 0.4ms. Pinned both ways — at the handler, by counting reads the drain attempts, and over a socket, against the deadline it used to wait out. Local: 135s to 60s, 1943 tests green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Jared Scott <jared.scott@variable.team>
Contributor
CoverageThreshold: |
Every check compiles the 260KB page script into its own vm context, and V8 reclaims those lazily: the worker sat at 430MB after the suite's 425 checks and climbed to 592MB when the same checks ran three times over. Nothing leaks that a restart cannot clear, so the process is replaced every 150 checks — two extra 40ms spawns, and a flat 261MB however many page tests the suite comes to hold. A recycled worker's pipes were left to the finalizer, which reported them as an unclosed-file ResourceWarning in the middle of an unrelated test. Closed on the way out instead, after joining the stderr drain. The drain thread is now handed its pipe rather than reading `self._proc`, which a restart rebinds — it would otherwise have ended up reading the replacement's stderr alongside its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Jared Scott <jared.scott@variable.team>
Attributes the macOS/Ubuntu gap to specific tests instead of guessing at it from a wall of progress dots. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Jared Scott <jared.scott@variable.team>
…rge" This reverts commit dc19296.
`HTTPServer.server_bind` sets `server_name` from `socket.getfqdn()` — a reverse DNS lookup, on the startup path, for a value nothing in this codebase reads. A resolver with no answer for 127.0.0.1 does not fail fast, it waits. Measured on the macOS CI runner at roughly 17.5s per bind. Seven tests that bind, or spawn something that binds, were 285s of that leg's 316s; the other 1936 finished in 31s. Two of them do nothing but construct servers on 127.0.0.1 and still took 35s each, which is what identified the call. Ubuntu answers the same lookup from /etc/hosts instantly, which is why the leg looked like a macOS problem rather than a lookup nobody needed. It is the same stall a person starting the dashboard on a machine with a slow resolver would sit through before the page came up, so this is a startup fix that happens to also be a CI fix. `server_name` and `server_port` are still set, because the base class promises them; the name is the host as given, and the port is read off the bound socket so a requested 0 reports what the OS actually handed out. Pinned by patching the lookup rather than by timing it: on a machine with a working resolver the call is instant, so a duration assertion would be a test that cannot fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Jared Scott <jared.scott@variable.team>
…rf/macos-test-runtime
gcko
changed the title
`docs/design-daemon.md` gains a rejected entry for inheriting `HTTPServer.server_bind`, because the override looks like something to tidy away and restoring the `super()` call reinstates a 17.5s-per-bind stall. It cost a CI probe round-trip to find; nobody should have to find it twice. `CONTRIBUTING.md`'s known-flake note described page tests shelling out to node per check with a 30-second subprocess timeout. They now share one worker, so the failure it told contributors to re-run past no longer exists, and the two failures that replace it look nothing like it. The harness bullet gains what a fresh vm context does not give back: the process no longer restarts, so anything a check leaves on a timer outlives it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Jared Scott <jared.scott@variable.team>
Three tests refuse twelve `--host` values between them, and each refusal goes through `parser.error()`, which writes a usage block plus the message to stderr. Nothing was catching it, so a green run buried the CI log for the whole suite under seventy-odd lines that report nothing wrong and read exactly like a failure. The suite's stderr goes from 64 lines to 5. `test_lifecycle` already redirects stderr around the same argparse exit; this brings the three stragglers to that convention, via one helper. Captured rather than discarded, because the text was worth something: each caller now asserts the refusal is about `--host`, where before any SystemExit from any parse error would have passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Jared Scott <jared.scott@variable.team>
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
1 participant
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.
What
The macOS test leg ran 415s against Ubuntu's 92s, and the whole quality gate waited on it.
The errors flooding that job's log were not the cause: they are 12
--hostargparse refusals, the same 12 Ubuntu emits, and they cost no time. They are still worth killing, because a green run that reads like a failure is its own problem, and that is cause 5.Five causes, found by measurement rather than inspection. The first is the big one, and it is a bug users hit.
1. A reverse DNS lookup on every bind
HTTPServer.server_bindsetsserver_namefromsocket.getfqdn()— a reverse DNS lookup, on the startup path, for a value nothing in this codebase reads. A resolver with no answer for127.0.0.1does not fail fast, it waits.On the macOS runner that was ~17.5s per bind. A per-test timing probe pushed to the macOS leg found seven tests were 285s of that leg's 316s, while the other 1936 finished in 31s:
The clustering at ~35s is what identified it — that is a fixed stall, not slowness.
test_the_listener_takes_its_reuse_policy_from_the_configdoes nothing but construct two servers on127.0.0.1and assert a boolean, and it took 35s: two binds, ~17.5s each. Ubuntu answers the same lookup from/etc/hostsinstantly, which is why this looked like a macOS problem rather than a lookup nobody needed.This is a startup fix, not only a CI fix. It is the same stall a person starting the dashboard on a machine with a slow resolver sits through before the page comes up.
2.
serve_forever()called baresocketserverpolls for shutdown every 0.5s by default, andshutdown()blocks until the accept loop next wakes and notices. The suite stands up about a hundred servers. 28s oftest_http_api's 44s, all asleep. Test servers now poll at 5ms; the shipped serve path keeps the stock interval, because the trade is right in a test and wrong in a daemon.3. One node process per page-JS check, 425 of them
Measured: node startup was 40ms of each check's 44ms — the spawn was the bill, not the 260KB of page script it ran. A single long-lived worker now runs every check in a fresh
vmcontext, which is the same isolation a fresh process gave. The page script uses norequire, noprocessand no node module machinery, which is what makes a bare context enough.The worker is recycled every 150 checks: each check compiles the page into its own context and V8 reclaims those lazily, so it sat at 430MB after 425 checks and climbed to 592MB over three times that. Recycling costs two 40ms spawns and holds it flat at 261MB.
4. A rejected POST re-draining a body it already read
Every POST that read its body and then refused it re-drained bytes already gone:
_drain_bodyreadsContent-Lengthagain, the peer has nothing left to send, andread1blocks untilREJECT_DRAIN_SECONDSgives up. A validation 400 on/api/askcost 252ms of its handler thread — on the shipped route, not only under test. Now 0.4ms.5. The log noise
Three tests refuse twelve
--hostvalues between them, and each refusal goes throughparser.error(), which writes a usage block plus the message to stderr. Nothing caught it, so agreen run buried the whole suite's log under seventy-odd lines that report nothing wrong and read
exactly like a failure.
test_lifecyclealready redirected stderr around the same argparse exit;these three never did.
Captured now rather than discarded, because the text was worth something: each caller asserts the
refusal is about
--host, where before anySystemExitfrom any parse error would have passed. Thesuite's stderr goes from 64 lines to 5, and the macOS job's test step from 13.8KB to 3.9KB.
Verification
Measured on CI, same workflow, before vs after:
test_http_api/api/askmacOS is no longer the long pole; Windows is. An intermediate CI run with causes 2-4 fixed but not
the bind stall measured 319s, which is what identified the bind stall as the dominant remainder.
Every new test was confirmed red against the unfixed code:
socket.getfqdnto raise — asserted by patching rather than by timing, because on a machine with a working resolver the call is instant and a duration assertion could not fail;BytesIOanswers a read past the end immediately, so a byte-count assertion cannot see the bug that only a socket exhibits.Full pre-PR suite run locally against the merged tree: ruff,
ruff format --check,mypy --strict,lint_embedded,validate_plugins,bump_version --current, no version fields moved since the merge base, and both suites under coverage (90.8%, threshold 73). 1959 tests green.What this does not touch
The remaining slow tests are the launcher characterization ones that genuinely spawn
server.py. Cutting those would cut real coverage, so they stay.🤖 Generated with Claude Code