perf(runner): drop five process forks per hooked test - #1355
Merged
Conversation
A test in a file defining set_up or tear_down cost five process forks more than a hookless one: each hook minted its output file through bashunit::temp_file (mktemp) and removed it with rm -f, and the temp-owner marker mktemp leaves behind made the EXIT trap rm -rf the test's temp files even when the test created none. That is 14.1ms per test against 5.4ms hookless here, and this suite runs 1052 such tests. Name the file arithmetically in the run directory instead, the way discovery.sh already names its source-error capture: the folded file path (keeps two files sharing a basename apart, #959) plus the per-suite ordinal the dispatcher assigns before forking (#851), since a Bash 3 subshell inherits both $$ and the RANDOM state. The > redirect truncates, so nothing has to remove it and the run-dir cleanup takes it at the end. Hooked and hookless tests now cost the same. Reusing the run dir also inherits its known hazard, so extract the vanished-directory guard discovery.sh grew for #1137 and share it: a failed redirect otherwise reads as the hook itself failing. The census in perf-fork-budget.md called the per-test path fork-free. It measured a fixture that defines no hooks, which is why this never showed up; the rule now says so. Closes #1345 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
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.
🤔 Background
Related #1345
A test in a file that defines
set_uportear_downcost 3.3x a hookless one. Each hook minted its output file withmktempand removed it withrm, and the ownership marker that left behind made the runnerrm -rfthe test's temp files at exit — five forks per test, even for a test that created no temp file of its own.💡 Changes
bashunit::temp_file, keyed on the folded file path plus the per-suite ordinal so parallel workers cannot collide. The>redirect truncates, so nothing has to remove it.https://claude.ai/code/session_01EXYWTGLjf7qM8Ru3GakDRm