All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Sayers <andrew-git@pileofstuff.org>
To: "SZEDER Gábor" <szeder@ira.uka.de>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 19/19] bash prompt: alternative git prompt without command substitution
Date: Wed, 09 May 2012 20:38:22 +0100	[thread overview]
Message-ID: <4FAAC7AE.3020002@pileofstuff.org> (raw)
In-Reply-To: <1336524290-30023-20-git-send-email-szeder@ira.uka.de>

On 09/05/12 01:44, SZEDER Gábor wrote:
> __git_ps1() prints the branch name, status indicators, etc. to stdout,
> therefore it has to be included in $PS1 through a command substitution
> to display that information in the prompt.  The configuration is
> straightforward, but it imposes the overhead of fork()ing a subshell
> for the command substitution.
> 
> However, bash has the $PROMPT_COMMAND shell variable, which "if set,
> the value is executed as a command prior to issuing each primary
> prompt" (quoted from bash man page).  Its value isn't executed in a
> subshell but in the context of the "main" shell, hence (non-local)
> variables set in invoked shell functions are available when expanding
> $PS1.  We can use this facility to avoid that command substitution for
> __git_ps1().
> 
> So split out the meat of __git_ps1() into the new
> __git_prompt_command() function, which stores the branch name & co.
> in the $__git_ps1_string variable.  This function, as its name
> suggests, should be included in $PROMPT_COMMAND, and $__git_ps1_string
> should in turn be included in $PS1 with a bit of a twist to put the
> parentheses around it:
> 
>    PROMPT_COMMAND=__git_prompt_command

Rather than overwrite any existing PROMPT_COMMAND, it would be better to
do something like:

PROMPT_COMMAND="__git_prompt_command; $PROMPT_COMMAND"

>    PS1='[\u@\h \W${__git_ps1_string:+ ($__git_ps1_string)}]\$ '
> 
> Turn __git_ps1() into a wrapper around __git_prompt_command() such
> that it's functionality remains unaltered, so already configured
> prompts won't break.
> 
> The whole series speeds up the bash prompt on Windows/MinGW
> immensely, in many cases brings it down to around 10ms on my
> machine while in powersave mode.  Here are some timing results in
> three common scenarios (repeated 10 times, because the after cases
> were too fast to measure a single execution accurately with 'time'):
> 
> In my home directory, i.e. not in a git repository, before:
> 
>     /c/Users/szeder
>     $ time for i in {0..9} ; do prompt=$(__git_ps1) ; done
> 
>     real    0m0.952s
>     user    0m0.214s
>     sys     0m0.444s
> 
>   After:
> 
>     /c/Users/szeder
>     $ time for i in {0..9} ; do __git_prompt_command ;
>            prompt=${__git_ps1_string:+ ($__git_ps1_string)} ; done
> 
>     real    0m0.718s
>     user    0m0.136s
>     sys     0m0.354s
> 
>   After, with discovery across filesystems enabled:
> 
>     /c/Users/szeder
>     $ time for i in {0..9} ; do __git_prompt_command ;
>            prompt=${__git_ps1_string:+ ($__git_ps1_string)} ; done
> 
>     real    0m0.078s
>     user    0m0.016s
>     sys     0m0.062s
> 
> At the top of a work tree, before:
> 
>     /c/Users/szeder/repo (master)
>     $ time for i in {0..9} ; do prompt=$(__git_ps1) ; done
> 
>     real    0m2.901s
>     user    0m0.391s
>     sys     0m1.468s
> 
>   After:
> 
>     /c/Users/szeder/repo (master)
>     $ time for i in {0..9} ; do __git_prompt_command ;
>            prompt=${__git_ps1_string:+ ($__git_ps1_string)} ; done
> 
>     real    0m0.094s
>     user    0m0.047s
>     sys     0m0.047s
> 
> In a subdirectory, stash indicator enabled, before:
> 
>     /c/Users/szeder/repo/subdir (master $)
>     $ time for i in {0..9} ; do prompt=$(__git_ps1) ; done
> 
>     real    0m4.118s
>     user    0m0.468s
>     sys     0m2.056s
> 
>   After:
> 
>     /c/Users/szeder/repo/subdir (master $)
>     $ time for i in {0..9} ; do __git_prompt_command ;
>            prompt=${__git_ps1_string:+ ($__git_ps1_string)} ; done
> 
>     real    0m0.858s
>     user    0m0.152s
>     sys     0m0.322s
> 
>   After, discovery across filesystems enabled:
> 
>     /c/Users/szeder/repo/subdir (master $)
>     $ time for i in {0..9} ; do __git_prompt_command ;
>            prompt=${__git_ps1_string:+ ($__git_ps1_string)} ; done
> 
>     real    0m0.109s
>     user    0m0.047s
>     sys     0m0.063s
> 
> Well, that's about 97% improvement.
> 
> The performance gain on Linux is smaller, the latter case goes down
> from 0.264s to 0.047, but since it was fast enough to begin with I
> won't lengthen this commit message with further timing results on
> Linux.
> 
> Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
> ---
> 
> We had some discussions recently about putting user-facing functions into
> a separate "namespace".  This patch doesn't take that into account, but
> once a consensus is reached __git_prompt_command() should be put in that
> namespace.
> 
>  contrib/completion/git-completion.bash | 25 +++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 5ea19018..1c29f3d0 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -29,6 +29,11 @@
>  #       are currently in a git repository.  The %s token will be
>  #       the name of the current branch.
>  #
> +#       Alternatively, to make the above Bash prompt a bit faster:
> +#               PROMPT_COMMAND=__git_prompt_command

As above, I'd recommend a simple documentation change:
PROMPT_COMMAND="__git_prompt_command; $PROMPT_COMMAND"
(to show people how to chain any other prompt commands they have)

> +#               PS1='[\u@\h \W${__git_ps1_string:+ ($__git_ps1_string)}]\$ '
> +#               GIT_DISCOVERY_ACROSS_FILESYSTEM=true
> +#
>  #       In addition, if you set GIT_PS1_SHOWDIRTYSTATE to a nonempty
>  #       value, unstaged (*) and staged (+) changes will be shown next
>  #       to the branch name.  You can configure this per-repository
> @@ -258,11 +263,12 @@ __git_ps1_show_upstream ()
>  }
>  
>  
> -# __git_ps1 accepts 0 or 1 arguments (i.e., format string)
> -# returns text to add to bash PS1 prompt (includes branch name)
> -__git_ps1 ()
> +# Stores the text to be added to the bash prompt (branch name, status
> +# indicators, etc.) in the $__git_ps1_string variable.
> +__git_prompt_command ()
>  {
>  	local __git_dir=""
> +	__git_ps1_string=""
>  	__gitdir >/dev/null
>  	if [ -z "$__git_dir" ]; then
>  		return
> @@ -365,7 +371,18 @@ __git_ps1 ()
>  	fi
>  
>  	local f="$w$i$s$u"
> -	printf -- "${1:- (%s)}" "$c${b##refs/heads/}${f:+ $f}$r$p"
> +	__git_ps1_string="$c${b##refs/heads/}${f:+ $f}$r$p"
> +}
> +
> +# __git_ps1 accepts 0 or 1 arguments (i.e., format string)
> +# returns text to add to bash PS1 prompt (includes branch name)
> +__git_ps1 ()
> +{
> +	local __git_ps1_string
> +	__git_prompt_command
> +	if [ -n "$__git_ps1_string" ]; then
> +		printf -- "${1:- (%s)}" "$__git_ps1_string"
> +	fi

How hard/appropriate would it be to export individual parts of the
prompt here?  Something like:

__git_ps1_string_dirtystate="$i"
__git_ps1_string_untrackedfiles="$u"

There have been requests in the past to let people individually
colourise different bits of the prompt, which this would make practical.

>  }
>  
>  __gitcomp_1 ()

  reply	other threads:[~2012-05-09 19:38 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-09  0:44 [PATCH 00/19] Bash prompt speedup SZEDER Gábor
2012-05-09  0:44 ` [PATCH 01/19] tests: move code to run tests under bash into a helper library SZEDER Gábor
2012-05-09  0:44 ` [PATCH 02/19] tests: add tests for the bash prompt functions in the completion script SZEDER Gábor
2012-05-09  8:07   ` Johannes Sixt
2012-05-09 18:08     ` Junio C Hamano
2012-05-10  6:09       ` Johannes Sixt
2012-05-09 18:36   ` Junio C Hamano
2012-05-09 20:33     ` SZEDER Gábor
2012-05-09  0:44 ` [PATCH 03/19] completion: use __gitdir() in _git_log() SZEDER Gábor
2012-05-09 18:41   ` Junio C Hamano
2012-05-09 19:01     ` SZEDER Gábor
2012-05-09  0:44 ` [PATCH 04/19] completion: respect $GIT_DIR SZEDER Gábor
2012-05-09  8:09   ` Johannes Sixt
2012-05-09 18:54   ` Junio C Hamano
2012-05-09  0:44 ` [PATCH 05/19] bash prompt: don't show the prompt when .git/HEAD is unreadable SZEDER Gábor
2012-05-09 19:32   ` Junio C Hamano
2012-05-09 19:45     ` SZEDER Gábor
2012-05-09  0:44 ` [PATCH 06/19] bash prompt: return early from __git_ps1() when not in a git repository SZEDER Gábor
2012-05-09  0:44 ` [PATCH 07/19] completion: make __gitdir() store repository path in $__git_dir SZEDER Gábor
2012-05-09 19:36   ` Junio C Hamano
2012-05-09  0:44 ` [PATCH 08/19] completion: use $__git_dir instead of $(__gitdir) SZEDER Gábor
2012-05-09 19:43   ` Junio C Hamano
2012-05-09 20:22     ` SZEDER Gábor
2012-05-09 20:56       ` Junio C Hamano
2012-05-09 21:36         ` SZEDER Gábor
2012-05-09  0:44 ` [RFC PATCH 09/19] completion: platform-specific helper function to get physical path SZEDER Gábor
2012-05-09  7:37   ` Johannes Sixt
2012-05-09  0:44 ` [PATCH 10/19] completion: use bash builtins to search for repository SZEDER Gábor
2012-05-09 19:52   ` Junio C Hamano
2012-05-09 22:34     ` SZEDER Gábor
2012-05-09 22:59       ` Junio C Hamano
2012-05-09  0:44 ` [PATCH 11/19] bash prompt: use bash builtins to find out current branch SZEDER Gábor
2012-05-09 20:02   ` Junio C Hamano
2012-05-09 21:11     ` SZEDER Gábor
2012-05-09 21:25       ` Junio C Hamano
2012-05-09 21:45         ` SZEDER Gábor
2012-05-09 21:50           ` Junio C Hamano
2012-05-09  0:44 ` [PATCH 12/19] bash prompt: use bash builtins to check whether inside git dir SZEDER Gábor
2012-05-09  8:07   ` Johannes Sixt
2012-05-09 20:06     ` Junio C Hamano
2012-05-09  0:44 ` [PATCH 13/19] bash prompt: check whether inside the worktree only when necessary SZEDER Gábor
2012-05-09  0:44 ` [PATCH 14/19] bash prompt: use bash builtins to find out current branch during rebase SZEDER Gábor
2012-05-09  0:44 ` [PATCH 15/19] bash prompt: use bash builtins to get detached HEAD abbrev. object name SZEDER Gábor
2012-05-09  0:44 ` [PATCH 16/19] bash prompt: display stash and upstream state even inside the repository SZEDER Gábor
2012-05-09  0:44 ` [PATCH 17/19] bash prompt: use bash builtins to check stash state SZEDER Gábor
2012-05-09  0:44 ` [RFC PATCH 18/19] bash prompt: avoid command substitution when checking for untracked files SZEDER Gábor
2012-05-09 20:32   ` Junio C Hamano
2012-05-09  0:44 ` [PATCH 19/19] bash prompt: alternative git prompt without command substitution SZEDER Gábor
2012-05-09 19:38   ` Andrew Sayers [this message]
2012-05-09 22:08     ` SZEDER Gábor

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=4FAAC7AE.3020002@pileofstuff.org \
    --to=andrew-git@pileofstuff.org \
    --cc=git@vger.kernel.org \
    --cc=szeder@ira.uka.de \
    /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.