Re: [PATCH v2 10/13] bash prompt: combine 'git rev-parse' executions
On Tue, Jun 18, 2013 at 02:05:35AM -0400, Jeff King wrote: > On Tue, Jun 18, 2013 at 04:17:03AM +0200, SZEDER Gábor wrote: > > > The whole series speeds up the bash prompt on Windows/MSysGit > > considerably. Here are some timing results in two scenarios, repeated > > 10 times: > > > > At the top of the work tree, before: > > > > $ time for i in {0..9} ; do prompt="$(__git_ps1)" ; done > > > > real0m1.716s > > user0m0.301s > > sys 0m0.772s > > > > After: > > > > real0m0.686s > > user0m0.075s > > sys 0m0.287s > > > > In a subdirectory, during rebase, stash status indicator enabled, > > before: > > > > real0m3.557s > > user0m0.495s > > sys 0m1.767s > > > > After: > > > > real0m0.702s > > user0m0.045s > > sys 0m0.409s > > Very nice speedup (or perhaps it is a testament to how bad fork() is on > msys). Well, it seems it's not just fork() & friends. The latter case on Linux, before: $ time for i in {0..99}; do prompt="$(__git_ps1)" ; done real0m2.819s user0m0.180s sys 0m0.272s After: real0m0.787s user0m0.000s sys 0m0.044s If you look solely at speedup (Win/MSys: 80%, Linux: 72%), Linux isn't that much better either, but overall it's about an order of magnitude faster to begin with (100 repetitions vs. 10). Btw, it could still be a bit faster in you would care to change your prompt to run from $PROMPT_COMMAND, because it would avoid that final $(__git_ps1) command sunstitution, too. But I didn't measured that because I find the interface awful ;) (Hmm, speaking of which, the patch reading HEAD might break setups using $PROMPT_COMMAND, because it might do a simple return without updating $PS1...) > Reading patches 8 and 9, I can't help but feel that "git status" > is letting us down a little by making us parse all of this data > ourselves. In theory, __git_ps1() could just be something like: > > eval "$(git status --shell)" > printf ... > > and the heavy lifting could be done in a single C process which does not > have to worry about fork overhead. But that is quite far from where we > are now, so while it might be an interesting place to go in the future, > I do not think such dreams would want to hold up current work. Yeah, that would be one way to go. It would have the added benefit that it could support 'core.abbrev' in the non-describable detached HEAD case without noticable overhead. OTOH it still requires a command substitution and a git command, so it's less than ideal. The previous version of this series more than a year ago did some tricky things to create a prompt without a single fork(), let alone exec(), e.g. looking for .git directory with bash builtins, comparing pwd's prefix with path to .git dir to find out whether inside .git dir, etc. As a result even that subdir+rebase+stash case could be done in 10ms on Windows. Too bad that that "inside .git dir" check could misfire on case insensitive file systems, so we have to do that check with '$(git rev-parse)'. http://thread.gmane.org/gmane.comp.version-control.git/197432/focus=197450 Gábor -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 10/13] bash prompt: combine 'git rev-parse' executions
On Tue, Jun 18, 2013 at 04:17:03AM +0200, SZEDER Gábor wrote: > The whole series speeds up the bash prompt on Windows/MSysGit > considerably. Here are some timing results in two scenarios, repeated > 10 times: > > At the top of the work tree, before: > > $ time for i in {0..9} ; do prompt="$(__git_ps1)" ; done > > real0m1.716s > user0m0.301s > sys 0m0.772s > > After: > > real0m0.686s > user0m0.075s > sys 0m0.287s > > In a subdirectory, during rebase, stash status indicator enabled, > before: > > real0m3.557s > user0m0.495s > sys 0m1.767s > > After: > > real0m0.702s > user0m0.045s > sys 0m0.409s Very nice speedup (or perhaps it is a testament to how bad fork() is on msys). Reading patches 8 and 9, I can't help but feel that "git status" is letting us down a little by making us parse all of this data ourselves. In theory, __git_ps1() could just be something like: eval "$(git status --shell)" printf ... and the heavy lifting could be done in a single C process which does not have to worry about fork overhead. But that is quite far from where we are now, so while it might be an interesting place to go in the future, I do not think such dreams would want to hold up current work. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 10/13] bash prompt: combine 'git rev-parse' executions
From: SZEDER Gábor >From the four '$(git rev-parse --)' command substitutions __git_ps1() executes three in its main code path: - the first to get the path to the .git directory ('--git-dir'), - the second to check whether we're inside the .git directory ('--is-inside-git-dir'), - and the last, depending on the results of the second, either * to check whether it's a bare repo ('--is-bare-repository'), or * to check whether inside a work tree ('--is-inside-work-tree'). Naturally, this imposes the overhead of fork()ing three subshells and fork()+exec()ing three git commands. Combine these four 'git rev-parse' queries into one and use bash parameter expansions to parse the combined output, i.e. to separate the path to the .git directory from the true/false of '--is-inside-git-dir', etc. This way we can eliminate two of the three subshells and git commands. The whole series speeds up the bash prompt on Windows/MSysGit considerably. Here are some timing results in two scenarios, repeated 10 times: At the top of the work tree, before: $ time for i in {0..9} ; do prompt="$(__git_ps1)" ; done real0m1.716s user0m0.301s sys 0m0.772s After: real0m0.686s user0m0.075s sys 0m0.287s In a subdirectory, during rebase, stash status indicator enabled, before: real0m3.557s user0m0.495s sys 0m1.767s After: real0m0.702s user0m0.045s sys 0m0.409s Signed-off-by: SZEDER Gábor --- contrib/completion/git-prompt.sh | 18 +- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index 10ec6902..b73cebf7 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -311,8 +311,9 @@ __git_ps1 () ;; esac - local g= - if ! g="$(git rev-parse --git-dir 2>/dev/null)"; then + local repo_info= + if ! repo_info="$(git rev-parse --git-dir --is-inside-git-dir \ + --is-bare-repository --is-inside-work-tree 2>/dev/null)"; then if [ $pcmode = yes ]; then #In PC mode PS1 always needs to be set PS1="$ps1pc_start$ps1pc_end" @@ -320,6 +321,13 @@ __git_ps1 () return fi + local inside_worktree="${repo_info##*$'\n'}" + repo_info="${repo_info%$'\n'*}" + local bare_repo="${repo_info##*$'\n'}" + repo_info="${repo_info%$'\n'*}" + local inside_gitdir="${repo_info##*$'\n'}" + local g="${repo_info%$'\n'*}" + local r="" local b="" local step="" @@ -396,13 +404,13 @@ __git_ps1 () local c="" local p="" - if [ "true" = "$(git rev-parse --is-inside-git-dir 2>/dev/null)" ]; then - if [ "true" = "$(git rev-parse --is-bare-repository 2>/dev/null)" ]; then + if [ "true" = "$inside_gitdir" ]; then + if [ "true" = "$bare_repo" ]; then c="BARE:" else b="GIT_DIR!" fi - elif [ "true" = "$(git rev-parse --is-inside-work-tree 2>/dev/null)" ]; then + elif [ "true" = "$inside_worktree" ]; then if [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" ] && [ "$(git config --bool bash.showDirtyState)" != "false" ] then -- 1.8.3.1.487.g8f4672d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html