diff --git a/.claude/rules/perf-fork-budget.md b/.claude/rules/perf-fork-budget.md index f0422416..3a5ee8d9 100644 --- a/.claude/rules/perf-fork-budget.md +++ b/.claude/rules/perf-fork-budget.md @@ -258,7 +258,15 @@ Verify such a rewrite by running both implementations over the same input that cannot fail proves nothing. **Parallel 10-test file run (CI's mode):** ~11 forks — 3 `mkdir`, 4 `rm`, -3 `awk` (#813; was 61). The per-test result file is named by a per-suite +3 `awk` (#813; was 61). That count was taken with a sequential census fixture +and so missed the one cost that only exists in this mode: every worker used to +re-probe the clock, because the probe resolved the implementation inside a +`$( )` and the resolved value died with that subshell. On a shell without +`EPOCHREALTIME` the probe forks `perl`, so it scaled one-for-one with the tests +— 502 execs for a 500-test file against 2 — and it fired even with per-test +timing off, since deciding that timing is off is what asks whether the clock is +expensive (#1353). **Measure the parallel budget with a parallel fixture**: a +per-worker cost is invisible to a sequential census by construction. The per-test result file is named by a per-suite ordinal the single-threaded dispatcher assigns just before each `&` (the fork inherits it), so it costs **no** `mktemp` + `mv` per test (#851; was 10 `mktemp` + 10 `mv`). This replaced the old sanitized-test-name scheme, whose diff --git a/CHANGELOG.md b/CHANGELOG.md index ec407078..2ea7192b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased ### Changed +- Performance: a `--parallel` run on a shell without `EPOCHREALTIME` no longer forks `perl` once per test. Every worker re-probed the clock, because the probe resolved the implementation inside a command substitution and the resolved value died with it: 502 `perl` execs for a 500-test file, now 2. It happened even with per-test timing off, since deciding that timing is off is what asks whether the clock is expensive. Fixed by the `clock::init` change in #1358; this adds the guards that keep it fixed (#1353) - A helper can now ship a faster body for a newer Bash and keep the Bash 3.0 one as a fallback, chosen once at load time. The floor does not move: 3.0 keeps working and keeps being tested. The compatibility rules now record each construct's minimum version instead of only banning it, and allow one only inside a matching version gate — never for a construct that is a parse error on the floor, since those kill the file even in a branch that shell never takes. See `adrs/adr-013-bash-version-gated-fast-paths.md` (#1352) - Performance: a file defining `set_up_before_script` or `tear_down_after_script` costs about 5.6ms less on Bash 3.2, and a test rendered with `--show-execution-time` about 0.74ms less. Four clock reads per file and the per-test padding still went through a capture subshell, next to return-slot variants that were already there (#1348) - Performance: startup is faster on suites of plain test files. Printing "Running N tests" sourced every file a second time and re-ran every data provider before the run began. A file whose functions the provider scan can already see is counted from that scan instead: over this repo's 241 files the counting pass went from 1.72s to 1.51s, and 124 of them no longer source or run a provider twice. Files with a data provider, a heredoc, a multi-line string, an `eval`, a nested `source` or a conditional definition keep the old path, so the count can never disagree with the run (#1347) diff --git a/tests/acceptance/bashunit_run_forks_test.sh b/tests/acceptance/bashunit_run_forks_test.sh index fd23913d..8d9c2db2 100644 --- a/tests/acceptance/bashunit_run_forks_test.sh +++ b/tests/acceptance/bashunit_run_forks_test.sh @@ -335,3 +335,65 @@ function test_test_hooks_do_not_fork_mktemp_or_rm_per_test() { # The run's own scratch-dir cleanup, and nothing per test. assert_less_or_equal_than 1 "$rm_forks" } + +# Regression guard for the parallel clock probe. Resolving the clock +# implementation used to happen inside a `$( )`, so the resolved value died +# with that subshell and every --parallel worker re-probed. On a shell without +# EPOCHREALTIME the probe forks `perl`, so the count scaled one-for-one with +# the tests: 502 execs for a 500-test file against 2 (#1353). It fired even +# with per-test timing off, because deciding that timing is off is what asks +# whether the clock is expensive, which resolves the impl. +# +# Asserted as a differential rather than a budget: on a platform whose clock is +# `EPOCHREALTIME` or `date` this forks no `perl` at all, and comparing two sizes +# still fails loudly if the count ever starts tracking the test count. +function test_parallel_clock_probes_do_not_scale_with_the_test_count() { + if bashunit::check_os::is_windows; then + bashunit::skip "PATH shims are unreliable under Git Bash" && return + fi + + local dir + dir="$(bashunit::temp_dir)" + local count_file="$dir/perl_calls" + local real_perl + real_perl="$(command -v perl)" + if [ -z "$real_perl" ]; then + bashunit::skip "no perl on this machine to shim" && return + fi + + { + echo '#!/usr/bin/env bash' + echo "echo x >>\"$count_file\"" + echo "exec \"$real_perl\" \"\$@\"" + } >"$dir/perl" + chmod +x "$dir/perl" + + local few="$dir/few_test.sh" + local many="$dir/many_test.sh" + local i=0 + : >"$few" + while [ $i -lt 5 ]; do + echo "function test_f$i() { assert_true true; }" >>"$few" + i=$((i + 1)) + done + i=0 + : >"$many" + while [ $i -lt 40 ]; do + echo "function test_m$i() { assert_true true; }" >>"$many" + i=$((i + 1)) + done + + : >"$count_file" + PATH="$dir:$PATH" ./bashunit --parallel "$few" >/dev/null 2>&1 + local few_calls + few_calls="$(grep -c . "$count_file" || true)" + + : >"$count_file" + PATH="$dir:$PATH" ./bashunit --parallel "$many" >/dev/null 2>&1 + local many_calls + many_calls="$(grep -c . "$count_file" || true)" + + # Eight times the tests must not cost more probes. Equality, not a budget: + # the run resolves the clock once whatever the size. + assert_same "$few_calls" "$many_calls" +} diff --git a/tests/unit/util/clock_test.sh b/tests/unit/util/clock_test.sh index d288c392..61d7b94d 100644 --- a/tests/unit/util/clock_test.sh +++ b/tests/unit/util/clock_test.sh @@ -220,3 +220,41 @@ function test_clock_is_expensive_false_for_native_impls() { done assert_same " shell:no date:no date-seconds:no" "$result" } + +# Resolving the clock implementation writes _BASHUNIT_CLOCK_NOW_IMPL, and the +# whole point is that it survives into the caller. `init` used to resolve it +# inside `$( )`, so the assignment died with that subshell and the main shell +# was left empty. Under --parallel every worker then re-probed, and on a shell +# without EPOCHREALTIME the probe forks `perl`: 502 execs for a 500-test file +# against 2 (#1353). +function test_clock_init_resolves_the_impl_in_the_calling_shell() { + local saved_impl=${_BASHUNIT_CLOCK_NOW_IMPL:-} + local saved_start=${_BASHUNIT_START_TIME:-} + _BASHUNIT_CLOCK_NOW_IMPL="" + + bashunit::clock::init + + local resolved=$_BASHUNIT_CLOCK_NOW_IMPL + + _BASHUNIT_CLOCK_NOW_IMPL=$saved_impl + _BASHUNIT_START_TIME=$saved_start + + assert_not_empty "$resolved" +} + +# is_expensive answers a property of the resolved impl, and is asked once per +# test through BASHUNIT_SHOW_EXECUTION_TIME=auto. With the impl already +# resolved it must be a variable read, not another probe. +function test_is_expensive_does_not_reprobe_a_resolved_impl() { + local saved_impl=${_BASHUNIT_CLOCK_NOW_IMPL:-} + bashunit::clock::now_to_slot + + local before=$_BASHUNIT_CLOCK_NOW_IMPL + bashunit::clock::is_expensive || true + local after=$_BASHUNIT_CLOCK_NOW_IMPL + + _BASHUNIT_CLOCK_NOW_IMPL=$saved_impl + + assert_not_empty "$before" + assert_same "$before" "$after" +}