git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Stephen Boyd <bebarino@gmail.com>,
	Kirill Smelkov <kirr@mns.spb.ru>,
	Sverre Rabbelier <srabbelier@gmail.com>,
	"Shawn O. Pearce" <spearce@spearce.org>,
	git@vger.kernel.org
Subject: [PATCH v2] Speed up bash completion loading
Date: Sun, 15 Nov 2009 04:29:13 -0600	[thread overview]
Message-ID: <20091115102912.GA4100@progeny.tock> (raw)
In-Reply-To: <7vd43krwd0.fsf@alter.siamese.dyndns.org>

On my slow laptop (P3 600MHz), system-wide bash completions take
too much time to load (> 2s), and a significant fraction of this
time is spent loading git-completion.bash:

$ time bash -c ". ./git-completion.bash"  # hot cache, before this patch

real    0m0.509s
user    0m0.310s
sys     0m0.180s

Kirill noticed that most of the time is spent warming up the
merge_strategy, all_command and porcelain_command caches.

Since git is not used in each and every interactive xterm, it
seems best to load completion support with cold caches and then
load each needed thing lazily.  This has most of the speed
advantage of pre-generating everything at build time, without the
complication of figuring out at build time what commands will be
available at run time.

The result is that loading completion is significantly faster
now:

$ time bash -c ". ./git-completion.bash"  # cold cache, after this patch

real    0m0.171s
user    0m0.060s
sys     0m0.040s

Suggested-by: Kirill Smelkov <kirr@mns.spb.ru>
Cc: Shawn O. Pearce <spearce@spearce.org>
Cc: Stephen Boyd <bebarino@gmail.com>
Cc: SZEDER Gábor <szeder@ira.uka.de>
Cc: Sverre Rabbelier <srabbelier@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
This version incorporates the small improvements previously sent, plus the
following nice one (which makes caching merge strategies safe again).
Thanks for the advice, everyone.

Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
> 
> >  __git_compute_merge_strategies ()
> >  {
> > -	: ${__git_merge_strategies=$(__git_list_merge_strategies)}
> > +	__git_merge_strategies=$(__git_list_merge_strategies)
> 
> Wouldn't
> 
> 	: ${__gms:=$(command)}
> 
> run command only until it gives a non-empty string?

Yes. :)

 contrib/completion/git-completion.bash |   76 +++++++++++++++++---------------
 1 files changed, 41 insertions(+), 35 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index e3ddecc..43d76b7 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -21,13 +21,7 @@
 #    2) Added the following line to your .bashrc:
 #        source ~/.git-completion.sh
 #
-#    3) You may want to make sure the git executable is available
-#       in your PATH before this script is sourced, as some caching
-#       is performed while the script loads.  If git isn't found
-#       at source time then all lookups will be done on demand,
-#       which may be slightly slower.
-#
-#    4) Consider changing your PS1 to also show the current branch:
+#    3) Consider changing your PS1 to also show the current branch:
 #        PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
 #
 #       The argument to __git_ps1 will be displayed only if you
@@ -324,12 +318,8 @@ __git_remotes ()
 	done
 }
 
-__git_merge_strategies ()
+__git_list_merge_strategies ()
 {
-	if [ -n "${__git_merge_strategylist-}" ]; then
-		echo "$__git_merge_strategylist"
-		return
-	fi
 	git merge -s help 2>&1 |
 	sed -n -e '/[Aa]vailable strategies are: /,/^$/{
 		s/\.$//
@@ -339,8 +329,17 @@ __git_merge_strategies ()
 		p
 	}'
 }
-__git_merge_strategylist=
-__git_merge_strategylist=$(__git_merge_strategies 2>/dev/null)
+
+unset __git_merge_strategies
+# 'git merge -s help' (and thus detection of the merge strategy
+# list) fails, unfortunately, if run outside of any git working
+# tree.  __git_merge_strategies is set to the empty string in
+# that case, and the detection will be repeated the next time it
+# is needed.
+__git_compute_merge_strategies ()
+{
+	: ${__git_merge_strategies:=$(__git_list_merge_strategies)}
+}
 
 __git_complete_file ()
 {
@@ -474,27 +473,24 @@ __git_complete_remote_or_refspec ()
 
 __git_complete_strategy ()
 {
+	__git_compute_merge_strategies
 	case "${COMP_WORDS[COMP_CWORD-1]}" in
 	-s|--strategy)
-		__gitcomp "$(__git_merge_strategies)"
+		__gitcomp "$__git_merge_strategies"
 		return 0
 	esac
 	local cur="${COMP_WORDS[COMP_CWORD]}"
 	case "$cur" in
 	--strategy=*)
-		__gitcomp "$(__git_merge_strategies)" "" "${cur##--strategy=}"
+		__gitcomp "$__git_merge_strategies" "" "${cur##--strategy=}"
 		return 0
 		;;
 	esac
 	return 1
 }
 
-__git_all_commands ()
+__git_list_all_commands ()
 {
-	if [ -n "${__git_all_commandlist-}" ]; then
-		echo "$__git_all_commandlist"
-		return
-	fi
 	local i IFS=" "$'\n'
 	for i in $(git help -a|egrep '^  [a-zA-Z0-9]')
 	do
@@ -504,17 +500,18 @@ __git_all_commands ()
 		esac
 	done
 }
-__git_all_commandlist=
-__git_all_commandlist="$(__git_all_commands 2>/dev/null)"
 
-__git_porcelain_commands ()
+unset __git_all_commands
+__git_compute_all_commands ()
+{
+	: ${__git_all_commands=$(__git_list_all_commands)}
+}
+
+__git_list_porcelain_commands ()
 {
-	if [ -n "${__git_porcelain_commandlist-}" ]; then
-		echo "$__git_porcelain_commandlist"
-		return
-	fi
 	local i IFS=" "$'\n'
-	for i in "help" $(__git_all_commands)
+	__git_compute_all_commands
+	for i in "help" $__git_all_commands
 	do
 		case $i in
 		*--*)             : helper pattern;;
@@ -595,8 +592,13 @@ __git_porcelain_commands ()
 		esac
 	done
 }
-__git_porcelain_commandlist=
-__git_porcelain_commandlist="$(__git_porcelain_commands 2>/dev/null)"
+
+unset __git_porcelain_commands
+__git_compute_porcelain_commands ()
+{
+	__git_compute_all_commands
+	: ${__git_porcelain_commands=$(__git_list_porcelain_commands)}
+}
 
 __git_aliases ()
 {
@@ -1088,7 +1090,8 @@ _git_help ()
 		return
 		;;
 	esac
-	__gitcomp "$(__git_all_commands)
+	__git_compute_all_commands
+	__gitcomp "$__git_all_commands
 		attributes cli core-tutorial cvs-migration
 		diffcore gitk glossary hooks ignore modules
 		repository-layout tutorial tutorial-2
@@ -1444,7 +1447,8 @@ _git_config ()
 		return
 		;;
 	pull.twohead|pull.octopus)
-		__gitcomp "$(__git_merge_strategies)"
+		__git_compute_merge_strategies
+		__gitcomp "$__git_merge_strategies"
 		return
 		;;
 	color.branch|color.diff|color.interactive|\
@@ -1545,7 +1549,8 @@ _git_config ()
 	pager.*)
 		local pfx="${cur%.*}."
 		cur="${cur#*.}"
-		__gitcomp "$(__git_all_commands)" "$pfx" "$cur"
+		__git_compute_all_commands
+		__gitcomp "$__git_all_commands" "$pfx" "$cur"
 		return
 		;;
 	remote.*.*)
@@ -2142,7 +2147,8 @@ _git ()
 			--help
 			"
 			;;
-		*)     __gitcomp "$(__git_porcelain_commands) $(__git_aliases)" ;;
+		*)     __git_compute_porcelain_commands
+		       __gitcomp "$__git_porcelain_commands $(__git_aliases)" ;;
 		esac
 		return
 	fi
-- 
1.6.5.2

  reply	other threads:[~2009-11-15 10:18 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-11  9:34 excerpts from tomorrow's "What's cooking" draft Junio C Hamano
2009-11-11 16:33 ` Ben Walton
2009-11-11 17:07 ` Johan Herland
2009-11-11 17:57 ` Sverre Rabbelier
2009-11-11 18:45   ` Daniel Barkalow
2009-11-15  2:07     ` Sverre Rabbelier
2009-11-15  2:49       ` Daniel Barkalow
2009-11-11 22:08   ` ks/precompute-completion Jonathan Nieder
2009-11-13  6:40     ` ks/precompute-completion Stephen Boyd
2009-11-13  7:06       ` ks/precompute-completion Jonathan Nieder
2009-11-13  7:12         ` ks/precompute-completion Stephen Boyd
2009-11-13  8:50           ` [PATCH] Speed up bash completion loading Jonathan Nieder
2009-11-13  9:03             ` Jonathan Nieder
2009-11-13 10:29               ` Jonathan Nieder
2009-11-13 20:43               ` Stephen Boyd
2009-11-14 10:35                 ` Jonathan Nieder
2009-11-14 11:01                 ` Jonathan Nieder
2009-11-14 14:43                   ` SZEDER Gábor
2009-11-14 19:33                     ` Jonathan Nieder
2009-11-14 23:46                   ` Stephen Boyd
2009-11-15  6:50                     ` Jonathan Nieder
2009-11-15  9:05                   ` Junio C Hamano
2009-11-15 10:29                     ` Jonathan Nieder [this message]
2009-11-16  1:55                       ` [PATCH v2] " Shawn O. Pearce
2009-11-16  8:28                       ` Stephen Boyd
2009-11-18  0:49                         ` [PATCH v3] " Jonathan Nieder
2009-11-18  0:59                           ` Shawn O. Pearce
2009-11-11 18:42 ` excerpts from tomorrow's "What's cooking" draft Nicolas Sebrecht
2009-11-11 19:50   ` Nicolas Pitre
2009-11-11 21:07     ` Petr Baudis
2009-11-11 21:19       ` Nicolas Pitre
2009-11-11 21:26         ` Nicolas Sebrecht
2009-11-11 21:42         ` Petr Baudis
2009-11-11 22:04           ` Nicolas Pitre
2009-11-11 22:24           ` [PATCH] give priority to progress messages Nicolas Pitre
2009-11-11 18:54 ` excerpts from tomorrow's "What's cooking" draft Jakub Narebski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20091115102912.GA4100@progeny.tock \
    --to=jrnieder@gmail.com \
    --cc=bebarino@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=kirr@mns.spb.ru \
    --cc=spearce@spearce.org \
    --cc=srabbelier@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).