Repository navigation
perf(hasher): reduce Git file enumeration checks - #1254
Open
djordjem88 wants to merge 1 commit into
Open
djordjem88 wants to merge 1 commit into
djordjem88 wants to merge 1 commit into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b26a45b-25aa-45e3-95b4-f9a1b55dcb58
Comment on lines
+62
to
92
| const [lsFilesResults, lsOtherResults] = await Promise.all([ | ||
| execa("git", ["ls-files", "--cached", "--deleted", "--stage", "-t", "-z"], { cwd: root }), | ||
| includeUntracked ? execa("git", ["ls-files", "-o", "-z", "--exclude-standard"], { cwd: root }) : undefined, | ||
| ]); | ||
|
|
||
| const files: string[] = []; | ||
| const deletedFiles = new Set<string>(); | ||
| for (const entry of lsFilesResults.stdout.split("\0")) { | ||
| if (!entry) { | ||
| continue; | ||
| } | ||
|
|
||
| if (lsFilesResults.exitCode === 0) { | ||
| const files = lsFilesResults.stdout.split("\0").filter((f) => Boolean(f) && fs.existsSync(path.join(root, f))); | ||
| this.addToPackageTree(files); | ||
| // Staged records separate metadata from the unquoted path with a tab. | ||
| const file = entry.slice(entry.indexOf("\t") + 1); | ||
| if (entry.startsWith("R ")) { | ||
| // Deleted paths also have cached records; exclude them after collecting both. | ||
| deletedFiles.add(file); | ||
| } else if (entry.startsWith("S ") || entry.slice(2, 8) === "120000") { | ||
| // Git skips deletion checks for skip-worktree entries and does not follow symlinks. | ||
| if (fs.existsSync(path.join(root, file))) { | ||
| files.push(file); | ||
| } | ||
| } else { | ||
| files.push(file); | ||
| } | ||
| } | ||
| this.addToPackageTree(files.filter((file) => !deletedFiles.has(file))); | ||
|
|
||
| if (includeUntracked) { | ||
| // Also get all untracked files in the workspace according to git | ||
| const lsOtherResults = await execa("git", ["ls-files", "-o", "-z", "--exclude-standard"], { cwd: root }); | ||
| if (lsOtherResults.exitCode === 0) { | ||
| const files = lsOtherResults.stdout.split("\0").filter(Boolean); | ||
| this.addToPackageTree(files); | ||
| } | ||
| if (lsOtherResults) { | ||
| this.addToPackageTree(lsOtherResults.stdout.split("\0").filter(Boolean)); | ||
| } |
Member
There was a problem hiding this comment.
This is great, thanks! Even better, this can be combined into one command. With -t, untracked records have a ? prefix but no stage metadata or tab separator, so they need their own parsing branch.
Suggested change
| const [lsFilesResults, lsOtherResults] = await Promise.all([ | |
| execa("git", ["ls-files", "--cached", "--deleted", "--stage", "-t", "-z"], { cwd: root }), | |
| includeUntracked ? execa("git", ["ls-files", "-o", "-z", "--exclude-standard"], { cwd: root }) : undefined, | |
| ]); | |
| const files: string[] = []; | |
| const deletedFiles = new Set<string>(); | |
| for (const entry of lsFilesResults.stdout.split("\0")) { | |
| if (!entry) { | |
| continue; | |
| } | |
| if (lsFilesResults.exitCode === 0) { | |
| const files = lsFilesResults.stdout.split("\0").filter((f) => Boolean(f) && fs.existsSync(path.join(root, f))); | |
| this.addToPackageTree(files); | |
| // Staged records separate metadata from the unquoted path with a tab. | |
| const file = entry.slice(entry.indexOf("\t") + 1); | |
| if (entry.startsWith("R ")) { | |
| // Deleted paths also have cached records; exclude them after collecting both. | |
| deletedFiles.add(file); | |
| } else if (entry.startsWith("S ") || entry.slice(2, 8) === "120000") { | |
| // Git skips deletion checks for skip-worktree entries and does not follow symlinks. | |
| if (fs.existsSync(path.join(root, file))) { | |
| files.push(file); | |
| } | |
| } else { | |
| files.push(file); | |
| } | |
| } | |
| this.addToPackageTree(files.filter((file) => !deletedFiles.has(file))); | |
| if (includeUntracked) { | |
| // Also get all untracked files in the workspace according to git | |
| const lsOtherResults = await execa("git", ["ls-files", "-o", "-z", "--exclude-standard"], { cwd: root }); | |
| if (lsOtherResults.exitCode === 0) { | |
| const files = lsOtherResults.stdout.split("\0").filter(Boolean); | |
| this.addToPackageTree(files); | |
| } | |
| if (lsOtherResults) { | |
| this.addToPackageTree(lsOtherResults.stdout.split("\0").filter(Boolean)); | |
| } | |
| const lsFilesResults = await execa( | |
| "git", | |
| ["ls-files", "--cached", "--deleted", "--stage", "-t", "-z", ...(includeUntracked ? ["--others", "--exclude-standard"] : [])], | |
| { cwd: root } | |
| ); | |
| const files: string[] = []; | |
| const deletedFiles = new Set<string>(); | |
| for (const entry of lsFilesResults.stdout.split("\0")) { | |
| if (!entry) { | |
| continue; | |
| } | |
| if (entry.startsWith("? ")) { | |
| files.push(entry.slice(2)); | |
| continue; | |
| } | |
| // Staged records separate metadata from the unquoted path with a tab. | |
| const file = entry.slice(entry.indexOf("\t") + 1); | |
| if (entry.startsWith("R ")) { | |
| // Deleted paths also have cached records; exclude them after collecting both. | |
| deletedFiles.add(file); | |
| } else if (entry.startsWith("S ") || entry.slice(2, 8) === "120000") { | |
| // Git skips deletion checks for skip-worktree entries and does not follow symlinks. | |
| if (fs.existsSync(path.join(root, file))) { | |
| files.push(file); | |
| } | |
| } else { | |
| files.push(file); | |
| } | |
| } | |
| this.addToPackageTree(files.filter((file) => !deletedFiles.has(file))); |
This branch has not been deployed
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
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
Requested reviewers: Nemanja Tesic (@nemanjatesic) and Elizabeth Craig (@ecraig12345).
Use
git ls-files --cached --deleted --stage -t -zto enumerate tracked inputs and filter deleted paths without an individual JavaScriptexistsSynccall for every regular tracked file. Retain existence checks for skip-worktree and indexed symlink entries. Run the tracked and optional untracked queries concurrently while retaining tracked-before-untracked insertion order.NUL-delimited parsing preserves spaces, tabs, and Unicode paths. Git failures continue to reject through execa. Git still performs filesystem work; this removes separate JavaScript checks, not all stat I/O.
Part of the broader Lage performance effort explored in #1238. This change is self-contained and based directly on current
main. The dependency-queue cursor originally implemented alongside it is deliberately excluded into its own PR.Validation
mainatcf155216; extracted source and test files exactly match the previously validated implementation.