{"thread":{"id":"14433","subject":"[PATCH] bash completion: Improve responsiveness of git-log completion","startedAt":"2008-07-13T02:37:42Z","lastAt":"2008-07-13T22:12:02Z","messageCount":5,"participants":["Shawn O. Pearce","Petr Baudis","Johannes Schindelin","Junio C Hamano","Ingo Molnar"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"83120","messageId":"20080713023742.GA31760@spearce.org","threadId":"14433","inReplyTo":null,"subject":"[PATCH] bash completion: Improve responsiveness of git-log completion","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-07-13T02:37:42Z","receivedAt":"2008-07-13T02:37:42Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio noticed the bash completion has been taking a long time lately.\nPetr Baudis tracked it down to 72e5e989b (\"bash: Add space after\nunique command name is completed.\").  Tracing the code showed\nwe spent significant time inside of this loop within __gitcomp,\ndue to the string copying overhead.\n\n  [28.146109654] _git common over\n  [28.164791148] gitrefs in\n  [28.280302268] gitrefs dir out\n  [28.300939737] gitcomp in\n  [28.308378112] gitcomp pre-case\n* [28.313407453] gitcomp iter in\n* [28.701270296] gitcomp iter out\n  [28.713370786] out normal\n\nSince __git_refs avoids this string copying by forking and using\necho we use the same trick here when we need to finish generating\nthe names for the caller.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n\n Does this make things better?  Or worse?  I'm not seeing a huge\n difference on my own system.  Maybe its too fast these days...\n\n contrib/completion/git-completion.bash |   28 ++++++++++++++++------------\n 1 files changed, 16 insertions(+), 12 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 27332ed..61581fe 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -114,9 +114,20 @@ __git_ps1 ()\n \tfi\n }\n \n+__gitcomp_1 ()\n+{\n+\tlocal c IFS=' '$'\\t'$'\\n'\n+\tfor c in $1; do\n+\t\tcase \"$c$2\" in\n+\t\t--*=*) printf %s$'\\n' \"$c$2\" ;;\n+\t\t*.)    printf %s$'\\n' \"$c$2\" ;;\n+\t\t*)     printf %s$'\\n' \"$c$2 \" ;;\n+\t\tesac\n+\tdone\n+}\n+\n __gitcomp ()\n {\n-\tlocal all c s=$'\\n' IFS=' '$'\\t'$'\\n'\n \tlocal cur=\"${COMP_WORDS[COMP_CWORD]}\"\n \tif [ $# -gt 2 ]; then\n \t\tcur=\"$3\"\n@@ -124,21 +135,14 @@ __gitcomp ()\n \tcase \"$cur\" in\n \t--*=)\n \t\tCOMPREPLY=()\n-\t\treturn\n \t\t;;\n \t*)\n-\t\tfor c in $1; do\n-\t\t\tcase \"$c$4\" in\n-\t\t\t--*=*) all=\"$all$c$4$s\" ;;\n-\t\t\t*.)    all=\"$all$c$4$s\" ;;\n-\t\t\t*)     all=\"$all$c$4 $s\" ;;\n-\t\t\tesac\n-\t\tdone\n+\t\tlocal IFS=$'\\n'\n+\t\tCOMPREPLY=($(compgen -P \"$2\" \\\n+\t\t\t-W \"$(__gitcomp_1 \"$1\" \"$4\")\" \\\n+\t\t\t-- \"$cur\"))\n \t\t;;\n \tesac\n-\tIFS=$s\n-\tCOMPREPLY=($(compgen -P \"$2\" -W \"$all\" -- \"$cur\"))\n-\treturn\n }\n \n __git_heads ()\n-- \n1.5.6.2.393.g45096\n"},{"id":"83122","messageId":"20080713040248.GG10151@machine.or.cz","threadId":"14433","inReplyTo":"20080713023742.GA31760@spearce.org","subject":"Re: [PATCH] bash completion: Improve responsiveness of git-log completion","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2008-07-13T04:02:48Z","receivedAt":"2008-07-13T04:02:48Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Sun, Jul 13, 2008 at 02:37:42AM +0000, Shawn O. Pearce wrote:\n> Junio noticed the bash completion has been taking a long time lately.\n> Petr Baudis tracked it down to 72e5e989b (\"bash: Add space after\n> unique command name is completed.\").  Tracing the code showed\n> we spent significant time inside of this loop within __gitcomp,\n> due to the string copying overhead.\n> \n>   [28.146109654] _git common over\n>   [28.164791148] gitrefs in\n>   [28.280302268] gitrefs dir out\n>   [28.300939737] gitcomp in\n>   [28.308378112] gitcomp pre-case\n> * [28.313407453] gitcomp iter in\n> * [28.701270296] gitcomp iter out\n>   [28.713370786] out normal\n> \n> Since __git_refs avoids this string copying by forking and using\n> echo we use the same trick here when we need to finish generating\n> the names for the caller.\n> \n> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>\n\nI spent quite some time trying to optimize the run by heavy use of bash\narrays (either just in __gitcomp() and at later stage reusing\n__git_refs() array within __gitcomp()), but it turned out I have made\nfew simple mistakes and in the end, the array is slower than Shawn's\napproach - when reusing the array, the difference is even bigger.\nI think that if a[i]=x is slower than echo x, bash is doing something\nreal weird, but that seems to be the case.\n\n\nSomeone might find my simple and noisy benchmarker useful:\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 27332ed..31f97c1 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -45,6 +45,18 @@\n #       git@vger.kernel.org\n #\n \n+\n+benchmark_last=0\n+benchmark_delta=0\n+benchmark ()\n+{\n+\t#return\n+\tlocal now=$(date +%S%N)\n+\tbenchmark_diff=$(echo \"($now-$benchmark_last)/1000000-($benchmark_delta)\"|bc)\n+\techo \"[+${benchmark_diff}ms] $*\" >&2\n+\tbenchmark_last=$now\n+}\n+\n __gitdir ()\n {\n \tif [ -z \"$1\" ]; then\n@@ -116,6 +128,7 @@ __git_ps1 ()\n \n __gitcomp ()\n {\n+\tbenchmark \"gitcomp start\"\n \tlocal all c s=$'\\n' IFS=' '$'\\t'$'\\n'\n \tlocal cur=\"${COMP_WORDS[COMP_CWORD]}\"\n \tif [ $# -gt 2 ]; then\n@@ -138,6 +151,7 @@ __gitcomp ()\n \tesac\n \tIFS=$s\n \tCOMPREPLY=($(compgen -P \"$2\" -W \"$all\" -- \"$cur\"))\n+\tbenchmark \"gitcomp end\"\n \treturn\n }\n \n@@ -185,12 +199,16 @@ __git_tags ()\n \n __git_refs ()\n {\n+\tbenchmark \"gitrefs in\"\n \tlocal cmd i is_hash=y dir=\"$(__gitdir \"$1\")\"\n \tif [ -d \"$dir\" ]; then\n \t\tif [ -e \"$dir/HEAD\" ]; then echo HEAD; fi\n-\t\tfor i in $(git --git-dir=\"$dir\" \\\n+\t\tbenchmark \"gitrefs dir a\"\n+\t\tlocal a=($(git --git-dir=\"$dir\" \\\n \t\t\tfor-each-ref --format='%(refname)' \\\n-\t\t\trefs/tags refs/heads refs/remotes); do\n+\t\t\trefs/tags refs/heads refs/remotes))\n+\t\tbenchmark \"gitrefs dir r\"\n+\t\tfor i in \"${a[@]}\"; do\n \t\t\tcase \"$i\" in\n \t\t\t\trefs/tags/*)    echo \"${i#refs/tags/}\" ;;\n \t\t\t\trefs/heads/*)   echo \"${i#refs/heads/}\" ;;\n@@ -198,6 +216,7 @@ __git_refs ()\n \t\t\t\t*)              echo \"$i\" ;;\n \t\t\tesac\n \t\tdone\n+\t\tbenchmark \"gitrefs dir out\"\n \t\treturn\n \tfi\n \tfor i in $(git ls-remote \"$dir\" 2>/dev/null); do\n@@ -210,6 +229,7 @@ __git_refs ()\n \t\tn,*) is_hash=y; echo \"$i\" ;;\n \t\tesac\n \tdone\n+\tbenchmark \"gitrefs out\"\n }\n \n __git_refs2 ()\n@@ -324,7 +344,9 @@ __git_complete_revlist ()\n \t\t__gitcomp \"$cur.\"\n \t\t;;\n \t*)\n+\t\tbenchmark \"revlist simple in\"\n \t\t__gitcomp \"$(__git_refs)\"\n+\t\tbenchmark \"revlist simple out\"\n \t\t;;\n \tesac\n }\n@@ -756,6 +778,8 @@ _git_log ()\n {\n \t__git_has_doubledash && return\n \n+\tbenchmark \"doubledash over\"\n+\n \tlocal cur=\"${COMP_WORDS[COMP_CWORD]}\"\n \tcase \"$cur\" in\n \t--pretty=*)\n@@ -791,6 +815,7 @@ _git_log ()\n \t\treturn\n \t\t;;\n \tesac\n+\tbenchmark \"gitcomp over\"\n \t__git_complete_revlist\n }\n \n@@ -1303,6 +1328,9 @@ _git ()\n {\n \tlocal i c=1 command __git_dir\n \n+\tbenchmark \"in\"; benchmark_delta=0\n+\tbenchmark \"in netto\"; benchmark_delta=$benchmark_diff\n+\n \twhile [ $c -lt $COMP_CWORD ]; do\n \t\ti=\"${COMP_WORDS[c]}\"\n \t\tcase \"$i\" in\n@@ -1314,6 +1342,8 @@ _git ()\n \t\tc=$((++c))\n \tdone\n \n+\t#[ \"$__git_dir\" ] || __git_dir=\"$(__gitdir)\"\n+\n \tif [ -z \"$command\" ]; then\n \t\tcase \"${COMP_WORDS[COMP_CWORD]}\" in\n \t\t--*=*) COMPREPLY=() ;;\n@@ -1330,12 +1360,16 @@ _git ()\n \t\t\t;;\n \t\t*)     __gitcomp \"$(__git_commands) $(__git_aliases)\" ;;\n \t\tesac\n+\n+\t\tbenchmark \"out cut $(date +%S.%N)\"\n \t\treturn\n \tfi\n \n \tlocal expansion=$(__git_aliased_command \"$command\")\n \t[ \"$expansion\" ] && command=\"$expansion\"\n \n+\tbenchmark \"mark common\"\n+\n \tcase \"$command\" in\n \tam)          _git_am ;;\n \tadd)         _git_add ;;\n@@ -1374,6 +1408,8 @@ _git ()\n \twhatchanged) _git_log ;;\n \t*)           COMPREPLY=() ;;\n \tesac\n+\n+\tbenchmark \"out normal\"\n }\n \n _gitk ()\n"},{"id":"83151","messageId":"alpine.DEB.1.00.0807131554040.4816@eeepc-johanness","threadId":"14433","inReplyTo":"20080713023742.GA31760@spearce.org","subject":"Re: [PATCH] bash completion: Improve responsiveness of git-log completion","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-13T13:55:25Z","receivedAt":"2008-07-13T13:55:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 13 Jul 2008, Shawn O. Pearce wrote:\n\n>  Does this make things better?  Or worse?  I'm not seeing a huge\n>  difference on my own system.  Maybe its too fast these days...\n\nI see an incredible difference.  I do not have the means to measure at the \nmoment, but it jumped from several seconds (which qualifies for a synonym \nto \"eternity\" in Git speak, as you know) to almost instantaneous.\n\nGood job,\nDscho\n"},{"id":"83182","messageId":"7vy745piqy.fsf@gitster.siamese.dyndns.org","threadId":"14433","inReplyTo":"20080713023742.GA31760@spearce.org","subject":"Re: [PATCH] bash completion: Improve responsiveness of git-log completion","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-13T21:38:29Z","receivedAt":"2008-07-13T21:38:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" <spearce@spearce.org> writes:\n\n> Junio noticed the bash completion has been taking a long time lately....\n\nThe credit actually goes to Ingo.\n\n> Petr Baudis tracked it down to 72e5e989b (\"bash: Add space after\n> unique command name is completed.\").  Tracing the code showed\n> we spent significant time inside of this loop within __gitcomp,\n> due to the string copying overhead....\n> ...\n>  Does this make things better?  Or worse?  I'm not seeing a huge\n>  difference on my own system.  Maybe its too fast these days...\n\nIngo, I understand you have stopped using the completion long time ago due\nto this latency issue.  Together with d773c63 (bash: offer only paths\nafter '--', 2008-07-08) that already is in 'maint' and 'master', this\nhopefully would make the completion usable for you again?\n"},{"id":"83190","messageId":"20080713221202.GA4204@elte.hu","threadId":"14433","inReplyTo":"7vy745piqy.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] bash completion: Improve responsiveness of git-log completion","fromName":"Ingo Molnar","fromEmail":"mingo@elte.hu","sentAt":"2008-07-13T22:12:02Z","receivedAt":"2008-07-13T22:12:02Z","isPatch":true,"sender":{"key":"mingo@elte.hu","avatar":null},"body":"\n* Junio C Hamano <gitster@pobox.com> wrote:\n\n> \"Shawn O. Pearce\" <spearce@spearce.org> writes:\n> \n> > Junio noticed the bash completion has been taking a long time lately....\n> \n> The credit actually goes to Ingo.\n> \n> > Petr Baudis tracked it down to 72e5e989b (\"bash: Add space after\n> > unique command name is completed.\").  Tracing the code showed\n> > we spent significant time inside of this loop within __gitcomp,\n> > due to the string copying overhead....\n> > ...\n> >  Does this make things better?  Or worse?  I'm not seeing a huge\n> >  difference on my own system.  Maybe its too fast these days...\n> \n> Ingo, I understand you have stopped using the completion long time ago \n> due to this latency issue.  Together with d773c63 (bash: offer only \n> paths after '--', 2008-07-08) that already is in 'maint' and 'master', \n> this hopefully would make the completion usable for you again?\n\nyeah. I've checked out the latest version and applied Shawn's patch and \nadded the patched contrib/completion/git-completion.bash back to my \n.bashrc and i'm not seeing the latencies anymore.\n\nThanks! Please consider this fixed - will follow up if there's any \nproblem left.\n\n\tIngo\n"}]}