-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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.
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
git: starship still unusable in git repos (with submodules?) #4275
Comments
Since starship is still snappy in other big repos like widelands, wesnoth or llvm-project, I assume this is some corner-case related to rust-langs submodules? 🤔 |
Hey @matthiaskrgr, there is a bug with submodules, #4266, but not sure if that is the same issue you are having here. |
Hm, right, when I am inside a repo that has a submodule (but not inside the submodule itself), it shows correct hash but wrong tag 🤔 |
I can't confirm the issue in my local copy of the rust repo as-is. I assume the suggested workaround for #4251, setting |
@matthiaskrgr And just to confirm: Do you also get the issue if you compile starship with a stable rust toolchain? |
Timings for me look like this with #4277 applied (even though I don't think the patch is related): Here are the timings of modules in your prompt (>=1ms or output):
git_status - 387ms - "�[1;31m[!]�[0m "
git_metrics - 298ms - "�[1;32m+8�[0m �[1;31m-8�[0m "
directory - <1ms - "�[1;36mrust�[0m "
git_branch - <1ms - "�[1;35m( master)�[0m "
python - <1ms - "via �[1;33m🐍 �[0m"
line_break - <1ms - "\n"
character - <1ms - "�[1;32m❯�[0m " Neither Thus I am also curious about your timings to see where all the time is spent. |
starship timings:
so looks like getting the git commit is the problem here, hmm. |
Using stable |
That looks like there's an issue with |
Added a bunch of debug prints for getting timings,
|
Hmm, this seems to be gitoxide code? /// Try to find a name for the configured commit id using all prior configuration, returning `Some(describe::Format)`
/// if one was found.
///
/// Note that there will always be `Some(format)`
pub fn try_format(&self) -> Result<Option<git_revision::describe::Format<'static>>, Error> {
self.try_resolve()?.map(|r| r.format()).transpose()
} |
@matthiaskrgr It is. |
Oooh, I noticed I had some commit checked out in the repo :) |
For the record:
|
@matthiaskrgr Does adding |
|
Thanks for debugging this! While I quietly watching how this unfolds I had a strong hunch that it's the describe functionality - it has a lot of potential for being slow in pathological cases, and simply doing a rust (14a459b 🏷 1.0.0-beta) via 🐍
❯ time git describe HEAD --debug
describe HEAD
No exact match on refs or tags, searching to describe
finished search at 7140d7c52bdf55daf0b978a19706d20c3bf7ee92
annotated 156705 1.0.0-beta
annotated 158575 1.0.0-alpha.2
annotated 160641 1.0.0-alpha
annotated 164398 0.12.0
annotated 169772 0.10
annotated 172074 0.9
annotated 174726 0.8
annotated 177831 0.7
annotated 180685 0.6
annotated 181375 0.11.0
traversed 181377 commits
1.0.0-beta-156705-g14a459bf37b
git describe HEAD --debug 1.10s user 0.02s system 99% cpu 1.118 total
rust (14a459b 🏷 1.0.0-beta) via 🐍
❯ time gix commit describe --statistics
traversed 181377 commits
1.0.0-beta-156705-g14a459bf37b
gix commit describe --statistics 1.85s user 0.01s system 100% cpu 1.858 total I paid a lot of attention to implement the algorithm to match Maybe there is still some performance to be eeked out, and I certainly hope so, as @davidkna Assuming the worst which is me not being able to get it significantly faster, what would be alternatives to mitigate this? I can imagine exposing a configuration setting to traverse the first parent only, or to shell it out to rust (14a459b 🏷 1.0.0-beta) via 🐍
❯ git commit-graph write
Finding commits for commit graph among packed objects: 100% (1964700/1964700), done.
Expanding reachable commits in commit graph: 201038, done.
rust (14a459b 🏷 1.0.0-beta) via 🐍 took 3s
❯ time git describe HEAD --debug
describe HEAD
No exact match on refs or tags, searching to describe
finished search at 7140d7c52bdf55daf0b978a19706d20c3bf7ee92
annotated 156705 1.0.0-beta
annotated 158575 1.0.0-alpha.2
annotated 160641 1.0.0-alpha
annotated 164398 0.12.0
annotated 169772 0.10
annotated 172074 0.9
annotated 174726 0.8
annotated 177831 0.7
annotated 180685 0.6
annotated 181375 0.11.0
traversed 181377 commits
1.0.0-beta-156705-g14a459bf37b
git describe HEAD --debug 0.06s user 0.02s system 97% cpu 0.084 total However, that is not usually available in typical people's repositories.
Passing That was a lot, what's your thoughts :)? |
Related to starship#4275 bringing performance to spitting distance compared to git.
Related to starship#4275 bringing performance to spitting distance compared to git.
So, I took another look, this time with a profiler, and noticed that 80% of the time is actually spent on the object database, which was unexpected. It turned out that the graph traversal of 'commit describe' benefits greatly from having an object cache, so that multiple accesses of the same commit can be cached. In the pending gitoxide upgrade I am setting a small object cache that is big enough to bring gitoxide into spitting distance to git. The same fix was applied to ❯ starship timings # with #4277 applied, down from ~1850ms
Here are the timings of modules in your prompt (>=1ms or output):
git_commit - 1172ms - "(14a459b 🏷 1.0.0-beta) "
git_status - 507ms - ""
git_metrics - 203ms - ""
directory - <1ms - "rust "
python - <1ms - "via 🐍 "
line_break - <1ms - "\n"
character - <1ms - "❯ "
rust (14a459b 🏷 1.0.0-beta) via 🐍
❯ time git describe HEAD
1.0.0-beta-156705-g14a459bf37b
git describe HEAD 1.10s user 0.01s system 99% cpu 1.114 total
rust (14a459b 🏷 1.0.0-beta) via 🐍
❯ time GITOXIDE_OBJECT_CACHE_MEMORY=4MB gix commit describe
1.0.0-beta-156705-g14a459bf37b
GITOXIDE_OBJECT_CACHE_MEMORY=1MB gix commit describe 1.16s user 0.02s system 99% cpu 1.191 total The cache size here is the smallest one that gave fast times time 2 in case this is good for bigger repositories, but I think with this change shelling out the operation isn't beneficial anymore. Further speedups are certainly possible, it looks like 20% of the time is still spent in parsing commits, and that could probably be faster if In any case, however, I feel that |
* upgrade `gitoxide` to v0.21 This release comes with lenient configuration handling by default, allowing to open repositories even their configuration values are invalid (even for git), as long as there are viable defaults. Furthermore this release adds the ability to open submodule repsitories. Fixes #4266 and fixes #4272 * Assure an object cache is set to speed up `commit.describe()` Related to #4275 bringing performance to spitting distance compared to git.
* upgrade `gitoxide` to v0.21 This release comes with lenient configuration handling by default, allowing to open repositories even their configuration values are invalid (even for git), as long as there are viable defaults. Furthermore this release adds the ability to open submodule repsitories. Fixes starship#4266 and fixes starship#4272 * Assure an object cache is set to speed up `commit.describe()` Related to starship#4275 bringing performance to spitting distance compared to git.
Bug Report
It looks like the point-release solved the hang in git repos, but starship still takes ~5 seconds to load inside a rust-lang/rust repository for me rendering it unusable. It used to take less than a second to load before 1.10.
Possible Solution
Can probably disable the git functionality or downgrade.
Environment
starship --version
]Relevant Shell Configuration
Starship Configuration
The text was updated successfully, but these errors were encountered: