Avoid unncessary git rev parse calls#313856
Open
dmitrivMS wants to merge 10 commits into
Open
Conversation
During initial scan and on file-watcher events, openRepository was invoked for every visible editor directory, sub-folder and watched path. The existing short-circuit only matched exact repo roots, so each call still spawned 'git rev-parse --show-toplevel'. On Windows this is ~200-700ms per call and dominates the initial scan time (see #313760). Fast-path: if the candidate path is contained in an opened repository (excluding submodules, via the existing getRepository lookup) and does not itself host a .git entry (so nested repositories are still discovered), skip the git call entirely. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes Git repository opening in the built-in Git extension by short-circuiting openRepository for paths that appear to be inside an already opened repository, reducing repeated git rev-parse --show-toplevel calls during workspace initialization.
Changes:
- Added an early-exit path in
Model.openRepositoryfor folders contained within an already opened repository. - Added a filesystem
.gitexistence check so actual nested repositories can still be opened. - Reduced the number of expensive
git rev-parse --show-toplevelsubprocess calls during initial repository scanning.
Show a summary per file
| File | Description |
|---|---|
extensions/git/src/model.ts |
Adds the new containment-based fast path before resolving the repository root with Git. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 1
Contributor
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Member
|
@dmitrivMS, we do this so that we can discover nested git repositories. |
Contributor
Author
I think it should still work with this change. What would be a good test to verify? If I create some folders under e.g. vscode repo and clone other repos in there - is that a good test? |
Member
|
@dmitrivMS, yes, you would need a folder structure like this:
I am not sure how your code would work if git repository (1) is discovered before git repository (2). |
Contributor
Author
|
@lszomoru Thanks, I'll build a repro with logs to verify (it should still detect folderB as repo 2 by checking for |
Contributor
Author
|
@lszomoru I have cloned |
Contributor
|
Base:
|
Contributor
Author
|
Posted custom build links on the issue, waiting for verification. |
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.
Fixes #321621
We make a lot of
git rev-parse --show-toplevelcalls when opening git workspaces. Those calls are ~100ms and there are many of them per repo.This change is a simple optimization to avoid calls for nested folders. E.g. for vscode repo this reduces number of calls to 2 for initial load.
Contributes towards #134921