test(runner): pin the clock probe to once per run, not once per worker - #1363
Merged
Conversation
Resolving the clock implementation used to happen inside a `$( )`, so the resolved value died with that subshell and the main shell was left empty. Under --parallel every worker inherited that empty value and re-probed; on a shell without EPOCHREALTIME the probe forks `perl`, so the count scaled one-for-one with the tests. Measured here on a 500-test file, macOS arm64, bash 3.2.57: old init --no-parallel 3 --parallel 502 new init --no-parallel 2 --parallel 2 It fired even with per-test timing off, because deciding that timing is off is what asks whether the clock is expensive, and answering that resolves the impl. The behaviour is already fixed: #1358 converted clock::init to now_to_slot for its own reasons, which resolves the impl in the main shell before any worker is dispatched. Nothing pinned it there, so this adds the two guards. The unit test is the invariant itself -- init must leave _BASHUNIT_CLOCK_NOW_IMPL set in the caller -- and the acceptance test is the differential the issue asked for: eight times the tests must not cost more probes. Both fail against the old init (7 probes against 42). A differential rather than a budget, because a platform whose clock is EPOCHREALTIME or `date` forks no `perl` at all; comparing two sizes still fails loudly if the count starts tracking the test count. perf-fork-budget.md stated the parallel budget from a sequential census fixture, which cannot see a per-worker cost by construction. It now says so. Closes #1353 Claude-Session: https://claude.ai/code/session_01EXYWTGLjf7qM8Ru3GakDRm
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
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.
🤔 Background
Related #1353
Resolving the clock implementation happened inside a
$( ), so the resolved value died with that subshell and every--parallelworker re-probed. On a shell withoutEPOCHREALTIMEthe probe forksperl, so it scaled one-for-one with the tests — and fired even with per-test timing off, since deciding that timing is off is what asks whether the clock is expensive.The behaviour is already fixed. #1358 converted
clock::inittonow_to_slotfor its own reasons, which resolves the impl in the main shell before any worker is dispatched. Measured on a 500-test file, macOS arm64, bash 3.2.57:--no-parallel--parallel💡 Changes
initmust leave_BASHUNIT_CLOCK_NOW_IMPLset in the caller. The acceptance test is the differential the issue asked for: eight times the tests must not cost more probes. Both fail against the oldinit(7 probes against 42).EPOCHREALTIMEordateforks noperlat all — comparing two sizes still fails loudly if the count starts tracking the test count.perf-fork-budget.mdstated the parallel budget from a sequential census fixture, which cannot see a per-worker cost by construction. It now says so.https://claude.ai/code/session_01EXYWTGLjf7qM8Ru3GakDRm