All of lore.kernel.org
 help / color / mirror / Atom feed
From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, "SZEDER Gábor" <szeder.dev@gmail.com>
Subject: [PATCH 07/12] completion: let 'for-each-ref' and 'ls-remote' filter matching refs
Date: Fri,  3 Feb 2017 03:54:00 +0100	[thread overview]
Message-ID: <20170203025405.8242-8-szeder.dev@gmail.com> (raw)
In-Reply-To: <20170203025405.8242-1-szeder.dev@gmail.com>

When completing refs, several __git_refs() code paths list all the
refs from the refs/{heads,tags,remotes}/ hierarchy and then
__gitcomp_nl() iterates over those refs in a shell loop to filter out
refs not matching the current word to be completed.  This comes with a
considerable performance penalty when a repository contains a lot of
refs but the current word can be uniquely completed or when only a
handful of refs match the current word.

Specify appropriate globbing patterns to 'git for-each-ref' and 'git
ls-remote' to list only those refs that match the current word to be
completed.  This reduces the number of iterations in __gitcomp_nl()
from the number of refs to the number of matching refs.

This speeds up refs completion considerably when there are a lot of
non-matching refs to be filtered out.  Uniquely completing a branch in
a repository with 100k local branches, all packed, best of five:

  On Linux, before:

    $ time __git_complete_refs --cur=maste

    real    0m0.831s
    user    0m0.808s
    sys     0m0.028s

  After:

    real    0m0.119s
    user    0m0.104s
    sys     0m0.008s

  On Windows, before:

    real    0m1.480s
    user    0m1.031s
    sys     0m0.060s

  After:

    real    0m0.377s
    user    0m0.015s
    sys     0m0.030s

Strictly speaking this is a fundamental behavior change in
__git_refs(), a helper function that is likely used in users' custom
completion scriptlets.  However, arguably this change should be
harmless, because:

  - __git_refs() was only ever intended to feed refs to Bash's
    completion machinery, for which non-matching refs have to be
    filtered out anyway.

  - There were already code paths that list only matching refs
    (listing unique remote branches for 'git checkout's tracking
    DWIMery and listing full remote refs via 'git ls-remote').

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 contrib/completion/git-completion.bash |  14 +++--
 t/t9902-completion.sh                  | 102 +++++++++++++++++++++++++++++++++
 2 files changed, 111 insertions(+), 5 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index d55eadfd1..615292f8b 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -394,7 +394,7 @@ __git_refs ()
 		case "$cur_" in
 		refs|refs/*)
 			format="refname"
-			refs="${cur_%/*}"
+			refs=("$cur_*" "$cur_*/**")
 			track=""
 			;;
 		*)
@@ -402,11 +402,13 @@ __git_refs ()
 				if [ -e "$dir/$i" ]; then echo $pfx$i; fi
 			done
 			format="refname:strip=2"
-			refs="refs/tags refs/heads refs/remotes"
+			refs=("refs/tags/$cur_*" "refs/tags/$cur_*/**"
+				"refs/heads/$cur_*" "refs/heads/$cur_*/**"
+				"refs/remotes/$cur_*" "refs/remotes/$cur_*/**")
 			;;
 		esac
 		__git_dir="$dir" __git for-each-ref --format="$pfx%($format)" \
-			$refs
+			"${refs[@]}"
 		if [ -n "$track" ]; then
 			# employ the heuristic used by git checkout
 			# Try to find a remote branch that matches the completion word
@@ -438,10 +440,12 @@ __git_refs ()
 		if [ "$list_refs_from" = remote ]; then
 			echo "HEAD"
 			__git for-each-ref --format="%(refname:strip=2)" \
-				"refs/remotes/$remote/" | sed -e "s#^$remote/##"
+				"refs/remotes/$remote/$cur_*" \
+				"refs/remotes/$remote/$cur_*/**" | sed -e "s#^$remote/##"
 		else
 			__git ls-remote "$remote" HEAD \
-				"refs/tags/*" "refs/heads/*" "refs/remotes/*" |
+				"refs/tags/$cur_*" "refs/heads/$cur_*" \
+				"refs/remotes/$cur_*" |
 			while read -r hash i; do
 				case "$i" in
 				*^{})	;;
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 7b42a686c..499be5879 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -837,6 +837,108 @@ test_expect_success '__git refs - exluding full refs' '
 	test_cmp expected "$actual"
 '
 
+test_expect_success 'setup for filtering matching refs' '
+	git branch matching/branch &&
+	git tag matching/tag &&
+	git -C otherrepo branch matching/branch-in-other &&
+	git fetch --no-tags other &&
+	rm -f .git/FETCH_HEAD
+'
+
+test_expect_success '__git_refs - only matching refs' '
+	cat >expected <<-EOF &&
+	HEAD
+	matching-branch
+	matching/branch
+	matching-tag
+	matching/tag
+	EOF
+	(
+		cur=mat &&
+		__git_refs >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - only matching refs - full refs' '
+	cat >expected <<-EOF &&
+	refs/heads/matching-branch
+	refs/heads/matching/branch
+	EOF
+	(
+		cur=refs/heads/mat &&
+		__git_refs >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - only matching refs - remote on local file system' '
+	cat >expected <<-EOF &&
+	HEAD
+	master-in-other
+	matching/branch-in-other
+	EOF
+	(
+		cur=ma &&
+		__git_refs otherrepo >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - only matching refs - configured remote' '
+	cat >expected <<-EOF &&
+	HEAD
+	master-in-other
+	matching/branch-in-other
+	EOF
+	(
+		cur=ma &&
+		__git_refs other >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - only matching refs - remote - full refs' '
+	cat >expected <<-EOF &&
+	refs/heads/master-in-other
+	refs/heads/matching/branch-in-other
+	EOF
+	(
+		cur=refs/heads/ma &&
+		__git_refs other >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success '__git_refs - only matching refs - checkout DWIMery' '
+	cat >expected <<-EOF &&
+	HEAD
+	matching-branch
+	matching/branch
+	matching-tag
+	matching/tag
+	matching/branch-in-other
+	EOF
+	for remote_ref in refs/remotes/other/ambiguous \
+		refs/remotes/remote/ambiguous \
+		refs/remotes/remote/branch-in-remote
+	do
+		git update-ref $remote_ref master &&
+		test_when_finished "git update-ref -d $remote_ref"
+	done &&
+	(
+		cur=mat &&
+		__git_refs "" 1 >"$actual"
+	) &&
+	test_cmp expected "$actual"
+'
+
+test_expect_success 'teardown after filtering matching refs' '
+	git branch -d matching/branch &&
+	git tag -d matching/tag &&
+	git update-ref -d refs/remotes/other/matching/branch-in-other
+'
+
 test_expect_success '__git_complete_refs - simple' '
 	sed -e "s/Z$//g" >expected <<-EOF &&
 	HEAD Z
-- 
2.11.0.555.g967c1bcb3


  parent reply	other threads:[~2017-02-03  2:54 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-03  2:53 [PATCH 00/12] completion: speed up refs completion SZEDER Gábor
2017-02-03  2:53 ` [PATCH 01/12] completion: remove redundant __gitcomp_nl() options from _git_commit() SZEDER Gábor
2017-02-03  2:53 ` [PATCH 02/12] completion: wrap __git_refs() for better option parsing SZEDER Gábor
2017-02-03  2:53 ` [PATCH 03/12] completion: support completing full refs after '--option=refs/<TAB>' SZEDER Gábor
2017-02-03  2:53 ` [PATCH 04/12] completion: support excluding full refs SZEDER Gábor
2017-02-03  2:53 ` [PATCH 05/12] completion: don't disambiguate tags and branches SZEDER Gábor
2017-02-03  2:53 ` [PATCH 06/12] completion: don't disambiguate short refs SZEDER Gábor
2017-02-03  2:54 ` SZEDER Gábor [this message]
2017-02-03  2:54 ` [PATCH 08/12] completion: let 'for-each-ref' strip the remote name from remote branches SZEDER Gábor
2017-02-03  2:54 ` [PATCH 09/12] completion: let 'for-each-ref' filter remote branches for 'checkout' DWIMery SZEDER Gábor
2017-02-03  2:54 ` [PATCH 10/12] completion: let 'for-each-ref' sort " SZEDER Gábor
2017-02-03  2:54 ` [PATCH 11/12] completion: list only matching symbolic and pseudorefs when completing refs SZEDER Gábor
2017-02-03  2:54 ` [PATCH 12/12] completion: fill COMPREPLY directly " SZEDER Gábor
2017-02-06 18:15   ` [PATCH] squash! " SZEDER Gábor
2017-02-10 21:44     ` Junio C Hamano
2017-02-13 19:32       ` SZEDER Gábor
2017-02-13 20:24         ` Junio C Hamano
2017-02-03  4:15 ` [PATCH 00/12] completion: speed up refs completion Jacob Keller
2017-02-04  3:15   ` Jacob Keller
2017-02-04  6:21     ` Junio C Hamano
2017-02-06 18:31     ` Jacob Keller
2017-02-06 19:36       ` SZEDER Gábor
2017-02-06 23:55         ` Jacob Keller

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=20170203025405.8242-8-szeder.dev@gmail.com \
    --to=szeder.dev@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.