On Sun, Mar 02, 2025 at 03:34:07PM -0500, David Mandelberg wrote:
> Op 2025-03-02 om 09:17 schreef Phillip Wood:
> > Hi David
> >
> > On 02/03/2025 07:45, David Mandelberg via GitGitGadget wrote:
> > > From: David Mandelberg <david@xxxxxxxxxxxxxx>
> > >
> > > Previously, some calls to for-each-ref passed fixed numbers of path
> > > components to strip from refs, assuming that remote names had no slashes
> > > in them. This made completions like:
> > >
> > > git push github/dseomn :com<Tab>
> > >
> > > Result in:
> > >
> > > git push github/dseomn :dseomn/completion-remote-slash
> > >
> > > With this patch, it instead results in:
> > >
> > > git push github/dseomn :completion-remote-slash
> >
> > This sounds like a useful improvement and I like the idea, but I think
> > running "git for-each-ref" once for each remote is not going to scale
> > very well for people who have a lot of remotes. I think it would be
> > better to try and strip "refs/remote/$remote/" outside of "git for-each-
> > ref". I've not tested it but I think something like
>
> Good point, I hadn't thought of that. Do you have a rough estimate of what
> "a lot of remotes" is? 100ish, maybe?
5.
In Git for Windows fork()-ing subshells and fork()+exec()-ing
processes is rather costly, about an order of magnitude slower than on
Linux. The rough equivalent of the body of your loop, with two
subshells and a git process:
time { a=$(echo 1) ; b=$(echo 2) ; git for-each-ref >/dev/null ; }
takes on average 0.17s on a windows box I have access to (with fully
packed refs, and merely 4 refs in total). So guess at about 4-5
remotes it would take over a second to react to my TABs...
I would rather try to go in the opposite direction to see whether 'git
for-each-ref' could be taught to strip the "refs/remote/$remote/"
prefix with a format specifier option like '%(refname:strip=remote)'.
That would surely be faster than any shell filtering we might come up
with, and would also save us from the trouble of escaping glob and/or
regex metacharacters for shell/sed pattern matching.
> I'd like to do some testing to get
> actual performance numbers before trying to optimize this, because I think
> the optimization has some drawbacks, see below.
>
> If optimization is needed, another approach is to parallelize the forks:
>
> {
> local fer_pids=
> for ...
> do
> __git for-each-ref ... &
> fer_pids="$fer_pids $!"
> done
> test -z "$fer_pids" || wait $fer_pids
> } | sort | uniq -u
>
> That might cause spikes in cpu/memory/disk usage that aren't ideal though.
Please don't do this, the completion script must not forkbomb the
system.
