Re: [PATCH] git-completion: Add git help completion for aliases

From: SZEDER Gábor <szeder@ira.uka.de>
Date: Mar 22, 2011, 20:25 UTC
Message-ID: <20110322202518.GA30604@neumann>
In-Reply-To: <AANLkTik8BaAJCnCRdDNTdu=nV1Q7cFLiwOuV_4wdXUE6@mail.gmail.com>

On Tue, Mar 22, 2011 at 11:28:01AM +0100, Erik Faye-Lund wrote:

Show 53 quoted lines

> 2011/3/22 SZEDER Gábor <szeder@ira.uka.de>:
> > On Tue, Mar 22, 2011 at 10:16:16AM +0100, Erik Faye-Lund wrote:
> >> 2011/3/22 SZEDER Gábor <szeder@ira.uka.de>:
> >> > On Tue, Mar 22, 2011 at 12:53:43AM -0700, Junio C Hamano wrote:
> >> >> This is a constructive tangent but if we are going to run $(__git_aliases)
> >> >> every time we run _git_help, perhaps it would want a hack similar to the
> >> >> way the value for $__git_all_commands is generated just once?
> >> >
> >> > I think this is not necessary.  We already run __git_aliases() every
> >> > time after 'git <TAB>', and it was not an issue so far.  And indeed, I
> >> > just created 50 aliases, and the time required for __git_aliases()
> >> > seems to be negligible:
> >> >
> >> >  $ time __git_aliases
> >> >  <bunch of aliases>
> >> >
> >> >  real    0m0.028s
> >> >  user    0m0.016s
> >> >  sys     0m0.004s
> >> >
> >>
> >> Unfortunately, the situation is not quite so good on Windows:
> >> $ time __git_aliases
> >> <bunch of aliases>
> >>
> >> real    0m0.112s
> >> user    0m0.030s
> >> sys     0m0.015s
> >>
> >> This is with 50 aliases, with 0 aliases I get this:
> >> $ time __git_aliases
> >> test
> >>
> >> real    0m0.063s
> >> user    0m0.015s
> >> sys     0m0.015s
> >
> > I see.  However, on Windows everything git-related tends to be slow,
> > so this is nothing new.
> 
> That's not the case. Every thing Git-related isn't slow on Windows,
> but there are some things in Git that is.
> 
> > The question is whether the slowness of a known slow platform would
> > justify the regression on all platforms.
> >
> 
> Windows isn't slow. Get over this way of thinking, it's just wrong.
> Windows has some different performance characteristics for some
> operations than e.g Linux, but saying that it's slow is just wrong.
> However, _Bash for Windows_ is quite slow, much due to Windows' lack
> of fork(), which means that some very involved emulation needs to be
> performed.

I meant the above only in the context of git, based on my last -- it seems outdated -- experiences with msysgit about (more than?) a year ago, when I found such builtins like commit and checkout to be quite noticeably slow. I'm glad to hear that things are changing for good.

Anyway, with the following silly "benchmarking" on top of the thread-starter patch[*] ...

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index d2b8746..12aa613 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2742,7 +2742,7 @@ _git ()
 	fi
 
 	local completion_func="_git_${command//-/_}"
-	declare -f $completion_func >/dev/null && $completion_func && return
+	declare -f $completion_func >/dev/null && time $completion_func && return
 
 	local expansion=$(__git_aliased_command "$command")
 	if [ -n "$expansion" ]; then


... the time required for 'git help <TAB>' with msysgit is around
120-140ms.

With the caching of aliases for 'git help' ...


diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 12aa613..466578b 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -963,6 +963,12 @@ __git_aliases ()
 	done
 }
 
+__git_aliases=
+__git_compute_aliases ()
+{
+	: ${__git_aliases:=$(__git_aliases)}
+}
+
 # __git_aliased_command requires 1 argument
 __git_aliased_command ()
 {
@@ -1508,7 +1514,8 @@ _git_help ()
 		;;
 	esac
 	__git_compute_all_commands
-	__gitcomp "$__git_all_commands $(__git_aliases)
+	__git_compute_aliases
+	__gitcomp "$__git_all_commands $__git_aliases
 		attributes cli core-tutorial cvs-migration
 		diffcore gitk glossary hooks ignore modules
 		repository-layout tutorial tutorial-2

... it goes down to around 50ms.

Looking at the numbers it seems to be a significant improvement, but
in practice I'm not sure I noticed anything; perhaps it's more
noticeable on slower hardware.  Just for the record, the numbers on
linux are ~50ms without and ~40ms with alias caching.

> But even so, at least 25% of the git user base is on Windows,
> according to the latest Git User Survey. That makes this stuff matter.

The other point of view is that it will cause a regression without a
compensating benefit for maybe more than 75% of the user base ;)


Best,
Gábor


[*] - off topic: Was it just me, or that simple patch really couldn't
be applied with a simple 'git am' on today's master?  What exactly
caused the conflict?  I looked quite hard, but couldn't find
anything...