perf(git): batch collect_status_local to <= 7 subprocesses per module - #13
Merged
Merged
Conversation
collect_status_local runs once per module inside status/update/check loops. Worst case today: 9 subprocesses (per-module .gitmodules config read + upstream/show-ref cascade). Pin budgets <= 7 / <= 6 (Red) and keep a behavior guard on the returned status dict.
collect_status_local runs once per module inside status/update/check loops. Two cuts: - the per-module `git config -f .gitmodules` branch read now comes from the parse_modules cache (mtime-cached); the direct read is kept only when parse_modules is unavailable (vim_dir without .git) so hostile branch values still reach sanitize_branch (#9 parity) - the sequential upstream/show-ref cascade (rev-parse @{upstream}, show-ref origin/main, origin/master, rev-parse origin/HEAD, show-ref origin/<branch>) collapses into one for-each-ref call with the same selection order; its sha listing also replaces the remote rev-parse for origin/* refs (verbatim .gitmodules branches keep their rev-parse - they resolve LOCAL refs, v2.2.10 behavior) Worst case per module: 9 subprocesses -> 7 (6 with .gitmodules branch). tests/perf.vader pins the budget and the returned status dict.
Tier B dead-code sweep, zero behavior change except where noted:
- async: drop never-read job state fields ('id', 'started', 'queued')
and a dead local (l:opts)
- plugin: drop g:plugin_manager_periodic_timer (written, never read)
- syntax: drop highlight keywords no code emits (Synced, Skipped,
timed out, Stashing changes); 'Pending' stays (update.vim uses it)
- ui: drop the 'pending' glyph key, the legacy numeric status branch of
get_status_glyph (all callers/tests pass keywords now), the
unreachable replace path of update_sidebar and the list branch of
log_detail
- delete ftdetect/pluginmanager.vim (only matched a literal file named
PluginManager; the sidebar sets its own filetype) and the Makefile.test
forwarding shim
- check: opts.force is now real (was documented, never read): force
fetches and leaves the cache untouched so the next startup check
re-fetches (tests/check.vader pins the contract)
health#execute was a single 200-line function of sequential if/else blocks. Checks are now a table of Funcrefs (s:checks) returning [status, label, detail] items; three custom checks (git version, log probe, submodule scan) moved to named helpers. Report order, messages and the both-problems submodule case are unchanged - pinned first by a characterization test (tests/health.vader: exact statuses on a healthy fixture).
…e-read
- sanitize_url/sanitize_cmd share one regex impl (s:sanitize_url_impl)
- log debug/warn/trace share s:leveled; the doubled level prefix
('DEBUG:c:DEBUG:msg') collapses to a single level marker - log line
format now '| comp | EXTERNAL | DEBUG:msg' (tests/log.vader pinned)
- backup: drop a redundant get_config('vim_dir') re-read inside
s:backup_vimrc_file
- rejected as behavior-changing (audit over-flagged): declare footer
dedup (sync/async messages differ), header+info extraction (zero net
lines), add.vim catch dedup (marginal), inline helptags -> helper
(install outcome on helptags failure would change)
The single-plugin and all-plugins update paths duplicated three blocks verbatim: the pinned-module handoff (s:handle_pin), the detached-HEAD / custom-branch skip text (s:skip_status, UI copy now pinned by tests/update.vader) and the pull command construction (s:pull_cmd). Path-specific logic that differs (pull-step UI text 'Pulling changes' vs 'Updating', stash bookkeeping, helptags+auto-commit in the single path, batch finalize) stays per-path: merging the flows would trade real behavior differences for churn. Full pipeline unification is deliberately NOT done (audit over-flagged it).
- CONTRIBUTING: drop ftdetect/ and Makefile.test from the tree, list the new perf.vader and health.vader characterization suites - CHANGELOG: [Unreleased] documents the subprocess batching, the health refactor, the single level prefix in debug/trace log lines, the now-real opts.force and the dead-code removals
…h setup Two CI-only failures, both git-environment dependent: - the fixture renamed HEAD to main AFTER the first commit; on stock images (init.defaultBranch unset -> master) that left HEAD an unborn symref, so `update-ref refs/remotes/origin/main HEAD` failed and the cascade found no remote branch. Point HEAD at main BEFORE the commit. - `git config -f` calls ran without -C: git does repo discovery from the cwd first, so a cwd inside an unrelated (or broken) repository could leak in or abort. Both parse_modules and the collect_status_local fallback now use `git -C <vim_dir> config -f ...`. Verified in an AlmaLinux 9 container (vim 8.2.2637, git 2.52): 214/214 Vader + 16/16 async smoke.
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.
Summary
Performance and dead-code pass with zero behavior change:
collect_status_local(runs once per module inside status/update/check loops) drops from 9 git subprocesses to 7 (6 with a declared branch), the 200-line health check becomes a table of check Funcrefs, never-read state and unreachable branches are removed, and the documented-but-ignoredopts.forceof the check command is now real. Public API surface untouched, no breaking change.Type of change
Related issue
n/a (performance + dead-code sweep from a repo audit; no issue tracking it)
How was this tested?
Vim 9.1 (Debian). TDD: the subprocess budget test (tests/perf.vader) was written RED against 2.2.10, then made GREEN by the batching. Characterization tests pinned exact statuses/messages before each refactor (health statuses, update skip messages, log line format).
make test-cipasses (214/214)make test-asyncpasses (16/16)make test-install-smokepasses (install/declare changes)(bug fixes: failing regression test before the fix)
Checklist
type(scope): subject)g:plugin_manager_*flag defaulting to off)v:version < 802untouched)