perf(runner): remove avoidable subshell forks from the run_test hot path#770
Merged
Conversation
#764) detect_runtime_error was called once per attempt inside the retry loop and again after it with the same input, each call a $(...) fork. Convert it to the return-slot convention (_BASHUNIT_RUNNER_RUNTIME_ERROR_OUT) and reuse the final attempt's value after the loop, removing both the fork and the duplicate computation from the per-test hot path. No behaviour change.
…empty (#764) decode_subshell_output was wrapped in a $(...) capture on every test. Convert it to the return-slot convention (_BASHUNIT_RUNNER_SUBSHELL_OUTPUT_OUT) and short-circuit the empty/_BASHUNIT_EMPTY_ case inline, so a passing test with no captured output does no subshell fork at all. The non-empty path keeps the inherent base64 decode fork. No behaviour change.
…_name (#764) normalize_test_function_name forked one or two nested $(...) subshells per call to read the custom title and interpolated-name state via their accessor functions, which only echo reserved-namespace globals. Read the globals directly on this per-test hot path. Accessors are kept for external callers; no behaviour change.
…re fork (#764) generate_id was already internally fork-free but every caller wrapped it in a $(...) capture. Convert it to the return-slot convention (_BASHUNIT_HELPER_ID_OUT); the per-test export_test_identity path and the per-file script-id path now read the slot instead of forking. resolve_test_location already writes a global slot and its single inner subshell is deliberate (it contains the extdebug toggle), so it is left as-is. No behaviour change.
#764) retry_count was captured via $(...) once per test to read a value that is constant across the whole run. Add resolve_retry_count, which validates BASHUNIT_RETRY into the _BASHUNIT_RETRY_VALIDATED global in-shell (no fork); run_test reads the global. retry_count is kept as a thin echoing wrapper for external callers. No behaviour change.
…tf fork (#764) print_successful_test wrapped a $(printf ...) on every passing test to build its output line, though the format only did %s substitution. Concatenate the line directly (byte-identical, covered by the args and no-args console_results tests). No behaviour change.
…reated (#764) cleanup_testcase_temp_files ran rm -rf on every test's temp-file glob even though most tests create none, forking rm once per test (100/100 on the noop census). Probe the glob in pure bash first (a non-matching glob stays literal with nullglob off, so matches[0] does not exist) and rm only when a file matched. A global flag cannot be used here: temp_file/temp_dir run in a $(...) capture subshell, so a flag set there never reaches the EXIT-trap cleanup; the filesystem is the only reliable signal. Fork census on the 100-noop file: rm 102 -> 2. No behaviour change.
JesusValeraDev
approved these changes
Jul 11, 2026
bash 3.0 does not expand a compound array assignment attached to `local`: `local matches=(glob)` stores the literal string "(glob)" instead of the matched paths. The cleanup probe therefore never saw a match, [ -e ] was false, and per-test temp files were left behind, failing the two temp file/dir tests on the Bash 3.0 jobs while every newer bash passed. Declare and assign separately so the glob expands on 3.0 too.
bash 3.0 does not expand a compound array assignment attached to `local`. watch.sh collapsed "$@" into a single literal element "(a b c)", so watch mode forwarded its extra args as one broken argument; console_results.sh stored the literal "()" as element 0 instead of building an empty array. Both are the same defect just fixed in globals.sh, and both are silent on every bash >= 3.2. Declare and assign separately in each, and add a guard test that greps src/ for the pattern, since a behavioural test could only fail under bash 3.0.
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
2 participants
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 #764
run_testand its helpers forked ~12-14 pure-bash$(...)subshells per test plus anrmper test for temp-file cleanup, the single largest external cost on the framework's reference platform (Bash 3.2 macOS). Most of those forks measure or clean up nothing.💡 Changes
rmwhen the test actually created a temp file (rm102 -> 2 on a 100-test file; ~33% faster wall-clock on that file). A flag could not be used:temp_fileruns in a$(...)subshell, so it never reaches the EXIT-trap cleanup.resolve_test_location(its lone inner subshell deliberately contains anextdebugtoggle) and did not pre-filter thedeclare -Fcache inget_functions_to_run(shared by thetestandbenchprefixes; filtering the one builder would couple it to every consumer and removes no fork).shfmtv3.13.1 drift left untouched, added lines show none (as in perf(runner): cache data-provider lookup per file #767).