Skip to content

Commit 2de660f

Browse files
ChemaclassChemaclass
authored
perf(runner): resolve a test's source location only when needed (#1356)
1 parent f9fbde9 commit 2de660f

8 files changed

Lines changed: 135 additions & 4 deletions

File tree

‎.claude/rules/perf-fork-budget.md‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,11 @@ them:
191191
that does used to cost five more forks per test (#1345), which is why that
192192
never showed up here. A census fixture only measures the path it exercises.
193193
- **Not the capture subshell.** A bare `$( )` costs ~0.46 ms here, about 6% of
194-
the 7.8 ms. The rest is bash work in the per-test machinery.
194+
the 7.8 ms. The rest is bash work in the per-test machinery. That 0.46 ms
195+
predates arm64 and must not be reused for estimates: the same measurement on
196+
macOS arm64 (bash 3.2.57) is 1.1-1.4 ms, and a subshell that also runs
197+
`shopt`/`declare` in it, 1.08 ms (#1346). A subshell per test is worth
198+
removing on that hardware even when this note says it is not the bottleneck.
195199
- **Not quadratic.** Per-test cost is 7.17 ms at 100 tests and 8.06 ms at 1000
196200
-- +12% over a 10x range, so the #830 fn-accumulation fix still holds. A
197201
regression there would show as per-test cost climbing with suite size.

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
## Unreleased
44

55
### Changed
6+
- Performance: a sequential run is about 0.7ms faster per test — a 500-test file went from 2.75s to 2.40s on macOS arm64, bash 3.2. Every test forked a subshell to read its own definition line, for a `<file>:<line>` that only a failure message and a report row ever use. It is resolved on demand now (#1346)
7+
- A standalone `bashunit assert <fn> …` no longer reports a source location belonging to another process. Launched from inside a test, it inherited that test's exported location and printed it as its own (#1346)
68
- Performance: a test in a file that defines `set_up` or `tear_down` is about 2.6x faster, which brings it level with a hookless test (14.1ms to 5.4ms per test on macOS arm64, bash 3.2). Each hook minted its output file with `mktemp` and removed it with `rm`, and the ownership marker that left behind made the runner `rm -rf` the test's temp files at exit — five forks per test, even for a test that created no temp file of its own (#1345)
79

810
### Fixed

‎src/console/test_line.sh‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ function bashunit::console_results::print_successful_test() {
4646
# is unknown. Used to append source context to failure output.
4747
##
4848
function bashunit::console_results::test_location_suffix() {
49+
bashunit::runner::ensure_test_location
4950
local location=${_BASHUNIT_TEST_LOCATION:-}
5051
if [ -z "$location" ]; then
5152
return 0

‎src/reports/collect.sh‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,10 @@ function bashunit::reports::add_test() {
100100

101101
# Capture the line number from the current test location ("file:line"),
102102
# but only when it belongs to this test's file, so a stale location from a
103-
# prior test never mislabels this entry.
103+
# prior test never mislabels this entry. Resolved here rather than per test:
104+
# reports are opt-in and this function has already returned when they are off
105+
# (#1346).
106+
bashunit::runner::ensure_test_location
104107
local line=""
105108
case "${_BASHUNIT_TEST_LOCATION:-}" in
106109
"$file":*) line="${_BASHUNIT_TEST_LOCATION##*:}" ;;

‎src/runner/context.sh‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,14 +62,50 @@ function bashunit::runner::export_test_identity() {
6262
local fn_name=$2
6363
bashunit::helper::generate_id "$fn_name"
6464
export BASHUNIT_CURRENT_TEST_ID="$_BASHUNIT_HELPER_ID_OUT"
65-
bashunit::runner::resolve_test_location "$test_file" "$fn_name"
65+
# Carry the inputs; do not resolve. Reading the definition line costs a
66+
# subshell, and only a failure message and a report row ever ask for it
67+
# (#1346). Cleared per test so a previous test's line cannot answer for this
68+
# one.
69+
#
70+
# The inputs are deliberately NOT exported. Every consumer runs in a fork of
71+
# this shell, which inherits them anyway, while an exec'd process gets a name
72+
# its own shell never defined. That is how a standalone `bashunit -a` used to
73+
# report the location of whichever test had launched it.
74+
_BASHUNIT_TEST_LOCATION=""
75+
_BASHUNIT_TEST_LOCATION_FILE=$test_file
76+
_BASHUNIT_TEST_LOCATION_FN=$fn_name
6677
export _BASHUNIT_TEST_LOCATION
6778
if [ "${_BASHUNIT_COVERAGE_ON:-0}" = 1 ]; then
6879
export _BASHUNIT_COVERAGE_CURRENT_TEST_FILE="$test_file"
6980
export _BASHUNIT_COVERAGE_CURRENT_TEST_FN="$fn_name"
7081
fi
7182
}
7283

84+
##
85+
# Resolves the running test's location, once, if anything asks for it.
86+
#
87+
# The lookup below forks a subshell, which measured 1.08ms per call on macOS
88+
# arm64 (bash 3.2.57) -- about 2.7s across this suite when every test paid it
89+
# up front. A passing test never reads the result, and reports are opt-in, so
90+
# the cost now falls only where the line is actually rendered (#1346).
91+
#
92+
# A caller inside `$( )` loses the assignment with its subshell, which only
93+
# means the next one resolves again: correct either way, and the failure path
94+
# is not hot.
95+
##
96+
function bashunit::runner::ensure_test_location() {
97+
if [ -n "${_BASHUNIT_TEST_LOCATION:-}" ]; then
98+
return 0
99+
fi
100+
101+
if [ -z "${_BASHUNIT_TEST_LOCATION_FN:-}" ]; then
102+
return 0
103+
fi
104+
105+
bashunit::runner::resolve_test_location \
106+
"${_BASHUNIT_TEST_LOCATION_FILE:-}" "$_BASHUNIT_TEST_LOCATION_FN"
107+
}
108+
73109
##
74110
# Resolves "<test_file>:<line>" for a test function and writes it into the
75111
# global _BASHUNIT_TEST_LOCATION, using `declare -F` under `extdebug` to read

‎tests/acceptance/snapshots/bashunit_direct_fn_call_advanced_test_sh.test_bashunit_direct_fn_call_failure.snapshot‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
11
✗ Failed: assert same
22
Expected 'foo'
33
but got  'bar'
4-
at tests/acceptance/bashunit_direct_fn_call_advanced_test.sh:16

‎tests/unit/console/results_test.sh‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -798,16 +798,22 @@ function test_test_location_suffix_when_set() {
798798
export _BASHUNIT_TEST_LOCATION="$original"
799799
}
800800

801+
# The suffix resolves the location on demand now (#1346), so "unknown" means
802+
# the inputs are gone too -- clearing only the cached value would just make it
803+
# resolve the test that is running.
801804
function test_test_location_suffix_empty_when_unset() {
802805
local original=${_BASHUNIT_TEST_LOCATION:-}
806+
local original_fn=${_BASHUNIT_TEST_LOCATION_FN:-}
803807
unset _BASHUNIT_TEST_LOCATION
808+
unset _BASHUNIT_TEST_LOCATION_FN
804809

805810
local output
806811
output="$(bashunit::console_results::test_location_suffix)"
807812

808813
assert_empty "$output"
809814

810815
export _BASHUNIT_TEST_LOCATION="$original"
816+
export _BASHUNIT_TEST_LOCATION_FN="$original_fn"
811817
}
812818

813819
# --- print_tap_line -----------------------------------------------------------

‎tests/unit/runner/context_test.sh‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,3 +69,83 @@ function test_restore_workdir_aborts_loudly_when_the_directory_is_gone() {
6969
assert_contains "cannot restore the working directory" "$output"
7070
assert_contains "$gone" "$output"
7171
}
72+
73+
# --- test location ------------------------------------------------------------
74+
75+
# Resolving "<file>:<line>" reads the definition line with
76+
# `$(shopt -s extdebug; declare -F …)`, a subshell that costs 1.08ms on macOS
77+
# arm64 — ~2.7s over this suite. Only a failure message and a report row ever
78+
# read it, so the identity carries the inputs and nothing resolves up front
79+
# (#1346).
80+
function test_export_test_identity_leaves_the_location_unresolved() {
81+
local orig_id=${BASHUNIT_CURRENT_TEST_ID:-}
82+
local orig_location=${_BASHUNIT_TEST_LOCATION:-}
83+
local orig_file=${_BASHUNIT_TEST_LOCATION_FILE:-}
84+
local orig_fn=${_BASHUNIT_TEST_LOCATION_FN:-}
85+
86+
bashunit::runner::export_test_identity "some_test.sh" "test_not_defined_here"
87+
88+
assert_empty "$_BASHUNIT_TEST_LOCATION"
89+
assert_same "some_test.sh" "$_BASHUNIT_TEST_LOCATION_FILE"
90+
assert_same "test_not_defined_here" "$_BASHUNIT_TEST_LOCATION_FN"
91+
92+
export BASHUNIT_CURRENT_TEST_ID="$orig_id"
93+
export _BASHUNIT_TEST_LOCATION="$orig_location"
94+
export _BASHUNIT_TEST_LOCATION_FILE="$orig_file"
95+
export _BASHUNIT_TEST_LOCATION_FN="$orig_fn"
96+
}
97+
98+
function test_ensure_test_location_resolves_on_demand() {
99+
local orig_location=${_BASHUNIT_TEST_LOCATION:-}
100+
local orig_file=${_BASHUNIT_TEST_LOCATION_FILE:-}
101+
local orig_fn=${_BASHUNIT_TEST_LOCATION_FN:-}
102+
_BASHUNIT_TEST_LOCATION=""
103+
_BASHUNIT_TEST_LOCATION_FILE="mine.sh"
104+
_BASHUNIT_TEST_LOCATION_FN="test_ensure_test_location_resolves_on_demand"
105+
106+
bashunit::runner::ensure_test_location
107+
108+
assert_matches "^mine\.sh:[0-9]+$" "$_BASHUNIT_TEST_LOCATION"
109+
110+
export _BASHUNIT_TEST_LOCATION="$orig_location"
111+
export _BASHUNIT_TEST_LOCATION_FILE="$orig_file"
112+
export _BASHUNIT_TEST_LOCATION_FN="$orig_fn"
113+
}
114+
115+
# Resolved once per test: the failure path renders the suffix up to three
116+
# times, and each would otherwise pay the subshell again.
117+
function test_ensure_test_location_keeps_an_already_resolved_location() {
118+
local orig_location=${_BASHUNIT_TEST_LOCATION:-}
119+
local orig_file=${_BASHUNIT_TEST_LOCATION_FILE:-}
120+
local orig_fn=${_BASHUNIT_TEST_LOCATION_FN:-}
121+
_BASHUNIT_TEST_LOCATION="already/resolved.sh:7"
122+
_BASHUNIT_TEST_LOCATION_FILE="mine.sh"
123+
_BASHUNIT_TEST_LOCATION_FN="test_ensure_test_location_keeps_an_already_resolved_location"
124+
125+
bashunit::runner::ensure_test_location
126+
127+
assert_same "already/resolved.sh:7" "$_BASHUNIT_TEST_LOCATION"
128+
129+
export _BASHUNIT_TEST_LOCATION="$orig_location"
130+
export _BASHUNIT_TEST_LOCATION_FILE="$orig_file"
131+
export _BASHUNIT_TEST_LOCATION_FN="$orig_fn"
132+
}
133+
134+
# Nothing to resolve from, so nothing is claimed: an empty location renders no
135+
# "at …" suffix at all.
136+
function test_ensure_test_location_stays_empty_without_a_function_name() {
137+
local orig_location=${_BASHUNIT_TEST_LOCATION:-}
138+
local orig_file=${_BASHUNIT_TEST_LOCATION_FILE:-}
139+
local orig_fn=${_BASHUNIT_TEST_LOCATION_FN:-}
140+
_BASHUNIT_TEST_LOCATION=""
141+
_BASHUNIT_TEST_LOCATION_FILE=""
142+
_BASHUNIT_TEST_LOCATION_FN=""
143+
144+
bashunit::runner::ensure_test_location
145+
146+
assert_empty "$_BASHUNIT_TEST_LOCATION"
147+
148+
export _BASHUNIT_TEST_LOCATION="$orig_location"
149+
export _BASHUNIT_TEST_LOCATION_FILE="$orig_file"
150+
export _BASHUNIT_TEST_LOCATION_FN="$orig_fn"
151+
}

0 commit comments

Comments
 (0)