[PATCH v3] Speed up bash completion loading

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

On this slow laptop, this decreases the time to load
git-completion.bash from about 500 ms to about 175 ms.

Suggested-by: Kirill Smelkov <kirr@xxxxxxxxxx>
Acked-by: Shawn O. Pearce <spearce@xxxxxxxxxxx>
Cc: Stephen Boyd <bebarino@xxxxxxxxx>
Cc: SZEDER Gábor <szeder@xxxxxxxxxx>
Cc: Sverre Rabbelier <srabbelier@xxxxxxxxx>
Signed-off-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
---
I do not know whether it is kosher to carry over an ack like this.
The interdiff is small, for what it’s worth:

	$ git branch jn/faster-completion-startup ee41299
	$ git diff jn/faster-completion-startup
	diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
	index bd22802..f67254d 100755
	--- a/contrib/completion/git-completion.bash
	+++ b/contrib/completion/git-completion.bash
	@@ -330,7 +330,7 @@ __git_list_merge_strategies ()
		}'
	 }
	 
	-unset __git_merge_strategies
	+__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
	@@ -501,10 +501,10 @@ __git_list_all_commands ()
		done
	 }
	 
	-unset __git_all_commands
	+__git_all_commands=
	 __git_compute_all_commands ()
	 {
	-	: ${__git_all_commands=$(__git_list_all_commands)}
	+	: ${__git_all_commands:=$(__git_list_all_commands)}
	 }
	 
	 __git_list_porcelain_commands ()
	@@ -593,11 +593,11 @@ __git_list_porcelain_commands ()
		done
	 }
	 
	-unset __git_porcelain_commands
	+__git_porcelain_commands=
	 __git_compute_porcelain_commands ()
	 {
		__git_compute_all_commands
	-	: ${__git_porcelain_commands=$(__git_list_porcelain_commands)}
	+	: ${__git_porcelain_commands:=$(__git_list_porcelain_commands)}
	 }
	 
	 __git_aliases ()

Please let me know if I have commited a faux pas.

Stephen Boyd wrote:
>> Junio C Hamano wrote:

>>> Wouldn't
>>>
>>>	: ${__gms:=$(command)}
>>>
>>> run command only until it gives a non-empty string?
> 
> Why not do this for all of the lists and not just the merge strategies?

I generally find set-if-unset a little more intuitive.  But some users
might install and try out git completion before realizing the need to
install git, and in this case set-if-empty gives better behavior.

Thanks.

 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..1223a07 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)
+
+__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 ()
+__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)"
+
+__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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]