perf: improve git I/O operation management#1242
Merged
Conversation
…anup - Add ROOT_PATHS handling in pathExistsImpl and getFilesPathCached to correctly resolve '', '.', './' paths against the tree index - Add GitAdapter.closeAll() to ensure all batch cat-file processes are cleaned up, including those from alternate-revision instances - Move batch process cleanup from IOExecutor to main.ts for centralized lifecycle management
- Wrap main.ts processing in try/finally to ensure GitAdapter.closeAll() runs on error paths, preventing orphaned git cat-file processes - Add NaN guard in GitBatchCatFile._processBuffer to reject malformed headers instead of silently corrupting data - Clear static instances map in closeAll() to release tree index memory - Add closeAll() and malformed header tests for 100% coverage - Update DESIGN.md to reflect tree index and batch cat-file architecture
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1242 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 58 60 +2
Lines 1603 1731 +128
Branches 213 249 +36
==========================================
+ Hits 1603 1731 +128 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Raise jest coverage thresholds to 100% for all metrics - Add tests for cached batch cat-file reuse, empty dir listing, and closeAll with active instances - Remove unreachable defensive guard in listDirAtRevision
…edly Add close event handler to GitBatchCatFile that rejects all queued promises if the process exits with non-zero code, preventing indefinite hangs. Document unbounded memory characteristic in DESIGN.md.
scolladon
changed the title
Accept string | string[] for the path parameter in gitGrep, spreading the array as multiple pathspec arguments to git grep. Existing callers passing a single string are unaffected.
Mirror the GitAdapter.gitGrep signature change, allowing callers to pass multiple pathspecs through the fsHelper ACL.
Replace readDirs(config.source) + extension filter with grepContent() using pathspec globs. This finds only translation files containing flowDefinitions without building or querying the tree index, removing FlowTranslationProcessor's dependency on the full source file listing.
Introduce GitDirCopy kind for directory copies (used by ContainedDecomposedHandler). IOExecutor now handles GitCopy as direct single-file cat-file without querying the tree index, and GitDirCopy as directory enumeration via getFilesPath. This decouples standard metadata types from the tree index entirely.
Add public method to pre-populate the tree index cache with a scoped git ls-tree call. When called before handler processing, all subsequent buildTreeIndex calls for the same revision hit the cache, avoiding a full-repo ls-tree.
Analyze diff lines against the metadata registry to determine which type directories need tree index access. Standard types are excluded, bundle types scope to component directories, and all others scope to type directories.
After computing diff lines, analyze which metadata type directories need tree index access and pre-build a scoped git ls-tree covering only those directories. When --include is set, falls back to scoping by config.source. When generateDelta is false, no tree index is built.
- Add per-file ignore check in _executeGitDirCopy for child files - Add GitAdapter.closeAll() in test beforeEach to prevent singleton leak
|
Published under $ sf plugins install sfdx-git-delta@dev-1242 |
|
Shipped in release $ sf plugins install sfdx-git-delta@latest-rc
# Or
$ sf plugins install sfdx-git-delta@v6.33.1💡 Enjoying sfdx-git-delta? |
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.
Explain your changes
Performance optimization of git I/O operations to reduce heap pressure and subprocess overhead.
Key changes:
git showsubprocess spawns with a single long-livedgit cat-file --batchchild process per adapter instance, using a FIFO queue for stdin/stdout coordinationgit ls-tree -rwith scopedgit ls-tree -r <rev> -- <path1> <path2>targeting only metadata directories that handlers actually need. Scope is computed from diff lines + metadata registry analysispreBuildTreeIndex— no lazy fallback via unscopedls-tree.pathExistsImpl,getFilesPathCached, andlistDirAtRevisionreturn empty results if no index was pre-builtpreBuildTreeIndexforconfig.toandconfig.fromrun concurrently viaPromise.allpreBuildTreeIndexcall pattern inmain.tsby defaulting scope toconfig.sourceand overriding when no include flags are setgit cat-filewithout requiring the tree indexgetFilesPath(uses tree index) then copy each file via cat-filegit difffor A/M/D filters concurrently instead of sequentiallygit grepwith pathspec globs instead of tree index to find translation filessrc/service/tosrc/adapter/as it is an I/O adapter, not a serviceSafety:
Orphaned
git cat-fileprocesses are prevented viaGitAdapter.closeAll()infinallyblock andcloseevent handlers that reject pending promisesConcurrency bounded by
getConcurrencyThreshold()throughoutJest tests added to cover the fix.
NUT tests added to cover the fix.
E2E tests added to cover the fix.
Any particular element that can be tested locally
No CLI-facing changes. Internal optimization only. Run
sf sgd source deltaas usual — behavior is identical, execution should be faster on large repos with reduced heap allocation.Any other comments