Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Follow-on consolidation of git rev-parse calls #3227

Open
jwhitley opened this issue Jan 16, 2024 · 3 comments · May be fixed by #3284
Open

Follow-on consolidation of git rev-parse calls #3227

jwhitley opened this issue Jan 16, 2024 · 3 comments · May be fixed by #3284
Labels
enhancement New feature or request

Comments

@jwhitley
Copy link
Contributor

In #3183, @jesseduffield writes:

there's an opportunity to pack even more stuff into the git rev-parse command: e.g. in pkg/app/app.go we call VerifyInGitRepo which does git rev-parse --git-dir and IsBareRepo which does git rev-parse --is-bare-repository.

This is a follow-on issue to track the opportunity to further consolidate lazygit's info retrieval from git rev-parse. During #3183, Jesse observed that rev-parse can take multiple arguments and return the requested information on a line-by-line basis, which reduces overhead vs. multiple git invocations.

See GetRepoPaths() in pkg/commands/git_commands/repo_paths.go for the new centralized invocation of git rev-parse (on the #3183 branch, or on master once that's been merged.)
@jwhitley jwhitley added the enhancement New feature or request label Jan 16, 2024
@jesseduffield
Copy link
Owner

Thanks for making this issue @jwhitley . To clarify: the goal here is to reduce lazygit startup time as much as possible by reducing the number of git calls required (each git call takes ~10ms on my machine). Given that git rev-parse can take multiple arguments and return multiple values in one go, we want to try and have all the git rev-parse calls on startup condensed into a single call

@jwhitley
Copy link
Contributor Author

FWIW, I think it's also possible to consolidate VerifyInGitRepo(), which is another git rev-parse --git-dir call. The idea would be to just call GetRepoPaths() and either 1) return the error if not in a git repo or 2) cache the path results that much earlier. The real question there is the dataflow/architectural impact of doing that. I'll have a look at that, and see what might be done.

@jesseduffield
Copy link
Owner

Sounds good

This was referenced Jan 27, 2024
Closed
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Change RepoPaths to be acquired via RepoPathCache jwhitley/lazygit
2 participants
@jwhitley @jesseduffield