reduce stat calls during git discovery #6882
Conversation
pascalkuthe
added
C-enhancement
There was a problem hiding this comment.
I think it's worth noting that this way the git ceiling dir isn't respected, which might have been a silent benefit of the current discovery mechanism.
If it helps, in theory GitoxideLabs/gitoxide#832 should end up doing pretty much the same work that git does as alternative.
If ceilings aren't important, I think this is about as fast as it gets.
Right now we set the ceiling dir always to |
|
True! Maybe it's the best choice anyway as it puts control over how to handle ceilings and crossing of filesystems into On the other hand, maybe a future step could be to adjust |
| @@ -41,16 +48,8 @@ impl Git { | |||
| ..gix::Permissions::default_for_level(gix::sec::Trust::Full) | |||
| }); | |||
|
|
|||
| let mut open_options = gix::discover::upwards::Options::default(); | |||
| if let Some(ceiling_dir) = ceiling_dir { | |||
| open_options.ceiling_dirs = vec![ceiling_dir.to_owned()]; | |||
There was a problem hiding this comment.
I actually recommend to not go ahead with this PR and instead use the new dot_git_only option. With it set to true the discovery can be optimal for this usecase, while making it easy to setup ceiling dirs and handle cross-filesystem checks.
There was a problem hiding this comment.
thanks for taking this further, I will update this PR to use the new gitoxide version once that is released
pascalkuthe
added
S-waiting-on-pr
|
Closing in favor of #6890 |
This should help with #6867, I talked with @Byron and for our purposes checking
.gitshould be enough and take less stat calls.I still want to cache the repo lookup here eventually, but that needs some more considerations about resource cleanup (so we don't keep an infinite amout of repos open, probably some kind of LRU mechanism is needed). I also want to look into opening the repo asynchronously, but that's a bit awkward to synchronize, so I will look at that in a separate PR too.