perf(shell): cut remaining N+1 Git and CLI spawns - #544
Conversation
…at-file Spawn census on this repo (65 shebang-looking blobs) counted 131 git invocations per run: one ls-files --stage and one cat-file blob per candidate. One git grep -z -n plus one ls-files --stage, with exact-path mode lookup, cuts that to two git spawns (133 -> 4 total). Line 1 matching ^#! is the POSIX-text equivalent of the previous byte-0 cat-file check. Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
compare_settings parsed the same pair three times (missing plugins, extra plugins, marketplace diffs). One tagged jq pass matches compare_catalog and the earlier manifest yq consolidation. --compare jq spawns 5 -> 3. Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
… path install_plugins_from listed marketplaces and plugins per source, then inventory listed marketplaces again (five CLI round-trips every session). Share the listings, record installs locally, and re-list marketplaces only when this session registered a new one. Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Claude finished @kyle-sexton's task in 7m 35s —— View job Code review complete
Findings1 high-severity correctness issue, posted inline on The new shebang-detection loop assumes Note: I could not execute No other issues found. The |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1aa4c31b56
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Stock macOS bash 3.2 has no associative arrays. Replace declare -A with a POSIX awk join on the batched ls-files records (same awk dependency as heading-cites). Spawn census stays git=2; total still 4 (mktemp -d + awk). Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
The local-lane driver already prefilters to one candidate comment per call, then chp::scan_text spawned awk to re-extract comment lines. Match the comment-start pattern in-process for newline-free content so a tree scan no longer pays one awk per hit. Multi-line callers still use one awk. Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
PATH-prefix assignments on a shell function persist, and command -v can see a leftover census shim. command -p pins the standard awk so the counter cannot double-count a wrapper. Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
Five per-OS git greps each walked the index. One combined -E union plus in-process classification keeps the per-OS labels and the 20-line cap without a second grep or a display-only head. Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
An empty EXCLUDE used to pipe ls-files through cat. Split the pipeline so the exclude grep is the only extra stage when a consumer sets one. Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
git-grep(1) documents -z as delimiting pathnames, which a review read as path NUL lineno:text LF. Combined -z -n actually emits path NUL lineno NUL text LF (git 2.43 byte dump; the 100644 shebang case already fails closed). Pipe through tr so the two NULs are visible to assert_eq. Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
…rm path install_plugins_from listed the Claude plugin CLI twice (once per source) and spawned grep -qxF per entry. Share one marketplace list and one plugin list across fleet+repo, record new installs in-process, and skip with the same membership idiom cloud-bootstrap already uses. Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
check_one ran jq empty then jq -e per candidate. The success path now does one policy pass; jq empty on the candidate runs only when that pass fails, so parse errors stay exit 2 and policy misses stay exit 1. Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
CI scans 11 workflow files, each previously one yq. One eval --no-doc over the glob matches per-file records (filename operator; --- separators would be misread as hits). Multi-file eval still stops at the first parse error and has no try/catch (yq 4.53.3), so a failed batch falls back per-file. Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
2 participants
No related issue: remaining Bash spawn N+1 after #316/#361, ranked by spawn census rather than file size.
Summary
Measurement-first Bash performance pass, following the fleet
performance@melodic-softwareplugin (spawn count over wall clock, stable PATH shim, interleaved repeats) and the patterns already landed in #320/#358/#361.distribution/sync-manifest.shis already a Node wrapper; this change does not touch it.Highest-impact remaining subjects on this repo, in order:
check-exec-bit.sh: 133 → 4 process spawns (git 131 → 2) against 65 shebang-looking blobs.scan-comment-hygiene.sh: 81 → 3 (awk 77 → 0) — one awk per git-grep hit.check-machine-specific-paths.sh: 10 → 1 (git 5 → 1, head 5 → 0).scan-workflow-files.sh: yq 11 → 1 on the success path (CI glob is 11 workflow files).check-plugin-baseline.sh --compare: jq 5 → 3.setup.shplugin stage (fleet+repo, warm): 4 Claude list CLIs + N grep → 2 (same list-once shape as cloud-bootstrap).check-claude-settings-targets.sh --file: jq 3 → 2 on the success path.check-heading-cites.sh: drop the identitycatwhenEXCLUDEis empty.Host qualification (
bash -c true, n=20): spread 1.66× (measurable). Headline metric is still the drift-immune spawn counter.Fix
git grep --cached -z -nIE '^#!'(line 1 ≡ byte-0 shebang) plus onegit ls-files --stage -zwith exact-recorded-path mode lookup. POSIX awk join (bash 3.2: nodeclare -A). Combined-z -nemitspath NUL lineno NUL text LF(git 2.43 byte dump; git-grep(1) only mentions pathname delimiting). The 100644 shebang case fails closed; a contract test pins that record viatr '\0' '|'.chp::scan_textmatches the comment-start pattern in-process for newline-free content (the local-lane git-grep hit shape). Multi-line callers still use one awk.git grep -Eover the five OS/repo bodies; classify hits in-process so per-OS labels and the 20-line cap stay without a second grep orhead.EXCLUDEdoes not spawncat.compare_settings: one tagged jq pass, matchingcompare_catalogand the perf(distribution): validate the manifest in two yq passes, not 274 #320 yq consolidation..claude/cloud-bootstrap.shstays byte-identical to the component.marketplace listand oneplugin listacross fleet+repo; in-process membership instead ofgrep -qxFper entry; record new installs so the second source does not re-query. Toolchain tracks (apt/dotnet/nvm) are unchanged.jq emptyon the candidate runs only when that pass fails, so parse errors stay exit 2 and policy misses stay exit 1.yq eval --no-docover every existing workflow argument on the success path (filenameoperator; without--no-doc,---document separators are misread as records). Mike Farah yq 4.53.3 multi-fileevalstill stops at the first parse error and has notry/catch, so a failed batch falls back to per-filepcc::scan_textand still reports every file.Left unchanged after census (already at floor, tiny corpus, or I/O-dominated):
sync-manifestNode engine, setup.sh apt/dotnet/nvm, private-repo-inventory (tiny TOML),repin::ahead_of(release-time, small N).Verification
bash harness/shell/run-tests.sh— previously 19 passed, 0 failed, 16 skipped on the earlier commits; this revision adds spawn-census coverage inpin-comment-convention.test.sh.comment-hygiene.test.sh: 8 single-line scans awk=0; multi-line fixture awk=1local-lane-guards.test.sh: exec-bit 8-file git shim (grep=1, ls-files=1, cat-file=0); 100644 shebang fails closed;git grep -z -nlayoutbad.sh|1|#!/usr/bin/env bash; machine-paths git=1 head=0; comment-hygiene driver awk=0; start-of-line Linux home still labeledcheck-plugin-baseline.test.sh:--comparejq=3cloud-bootstrap.test.sh: warm path marketplace-list=1, plugin-list=1; copies identicalsetup.test.sh: fleet+repo warm path marketplace-list=1, plugin-list=1, marketplace-add=0, plugin-install=2 (beta+gamma);bash -nand stamp-after-wait contracts unchangedcheck-claude-settings-targets.test.sh:--filesuccess-path jq=2; malformed candidate still exits 2pin-comment-convention.test.sh: 8 conforming files yq=1; parse-error fallback still reports a later missing-comment (batch + per-file yq=4)check-exec-bit.shgit=131)git=2)scan-comment-hygiene.shawk=77)awk=0)check-machine-specific-paths.shgit=5head=5)git=1)scan-workflow-files.sh(CI glob, success)--compareidentical fleet listjq=5)jq=3)check-heading-cites.shcat=1)cat=0)setup.shplugin stage (fleet+repo, warm)grep=0)check-claude-settings-targets.sh --fileGrounding: GNU/Git
cat-file --batch/ls-files --stage/grep(batch vs per-object); Google Shell Style Guide “builtin vs external” and no UUOC; this repo’s #320/#361 spawn-census method; performance plugin harness-integrity (stable PATH, fail closed, commit before verify); yq file operators.Related