Re: [PATCH] completion: improve ls-files filter performance
From: Johannes Schindelin <hidden>
Date: 2018-04-04 16:16:25
Hi drizzd, On Wed, 4 Apr 2018, Clemens Buchacher wrote:
From the output of ls-files, we remove all but the leftmost path component and then we eliminate duplicates. We do this in a while loop, which is a performance bottleneck when the number of iterations is large (e.g. for 60000 files in linux.git). $ COMP_WORDS=(git status -- ar) COMP_CWORD=3; time _git real 0m11.876s user 0m4.685s sys 0m6.808s Replacing the loop with the cut command improves performance significantly: $ COMP_WORDS=(git status -- ar) COMP_CWORD=3; time _git real 0m1.372s user 0m0.263s sys 0m0.167s The measurements were done with Msys2 bash, which is used by Git for Windows.
Those are nice numbers right there, so I am eager to get this into Git for Windows as quickly as it stabilizes (i.e. when it hits `next` or so). I was wondering about one thing, though:
quoted hunk ↗ jump to hunk
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 6da95b8..69a2d41 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash@@ -384,12 +384,7 @@ __git_index_files () local root="${2-.}" file __git_ls_files_helper "$root" "$1" | - while read -r file; do - case "$file" in - ?*/*) echo "${file%%/*}" ;;
This is a bit different from the `cut -f1 -d/` logic, as it does *not necessarily* strip a leading slash: for `/abc` the existing code would return the string unmodified, for `/abc/def` it would return an empty string! Now, I think that this peculiar behavior is most likely bogus as `git ls-files` outputs only relative paths (that I know of). In any case, reducing paths to an empty string seems fishy. I looked through the history of that code and tracked it all the way back to https://public-inbox.org/git/1357930123-26310-1-git-send-email-manlio.perillo@gmail.com/ (that is the reason why you are Cc:ed, Manlio). Manlio, do you remember why you put the `?` in front of `?*/*` here? I know, it's been more than five years... Out of curiosity, would the numbers change a lot if you replaced the `cut -f1 -d/` call by a `sed -e 's/^\//' -e 's/\/.*//'` one? I am not proposing to change the patch, though, because we really do not need to expect `ls-files` to print lines with leading slashes.
- *) echo "$file" ;; - esac - done | sort | uniq + cut -f1 -d/ | sort | uniq } # Lists branches from the local repository.
Ciao, Dscho