Re: [PATCH 2/3] gitweb: Cache $parent_commit info in git_blame()

From: Jakub Narebski <jnareb@gmail.com>
Date: Dec 10, 2008, 13:39 UTC
Message-ID: <200812101439.18526.jnareb@gmail.com>
In-Reply-To: <20081210124901.6117@nanako3.lavabit.com>

On Wed, 10 Dec 2008, Nanako Shiraishi wrote:

Show 9 quoted lines

> Quoting Jakub Narebski <jnareb@gmail.com>:
> 
> > Unfortunately the implementation in 244a70e used one call for
> > git-rev-parse to find parent revision per line in file, instead of
> > using long lived "git cat-file --batch-check" (which might not existed
> > then), or changing validate_refname to validate_revision and made it
> > accept <rev>^, <rev>^^, <rev>^^^ etc. syntax.
> 
> Could you substantiate why this is "Unfortunate"?

Because it calls git-rev-parse once for _each line_, even if for lines
in the group of neighbour lines blamed by same commit $parent_commit
is the same, and even if you need to calculate $parent_commit only once
per unique individual commit present in blame output.

> Is the new implementation faster?  By how much?

File | L[1] | C[2] || Time0[3] | Before[4] | After[4] ==================================================================== blob.h | 18 | 4 || 0m1.727s | 0m2.545s | 0m2.474s GIT-VERSION-GEN | 42 | 13 || 0m2.165s | 0m2.448s | 0m2.071s README | 46 | 6 || 0m1.593s | 0m2.727s | 0m2.242s revision.c | 1923 | 121 || 0m2.357s | 0m30.365s | 0m7.028s gitweb/gitweb.perl | 6291 | 428 || 0m8.080s | 1m37.244s | 0m20.627s

File | L/C | Before/After ========================================= blob.h | 4.5 | 1.03 GIT-VERSION-GEN | 3.2 | 1.18 README | 7.7 | 1.22 revision.c | 15.9 | 4.32 gitweb/gitweb.perl | 14.7 | 4.71

As you can see the greater ratio of lines in file to unique commits in blame output, the greater gain from the new implementation.

Footnotes:
~~~~~~~~~~
[1] Lines: 
    $ wc -l <file>
[2] Individual commits in blame output:
    $ git blame -p <file> | grep author-time | wc -l
[3] Time for running "git blame -p" (user time, single run):
    $ time git blame -p <file> >/dev/null
[4] Time to run gitweb as Perl script from command line:
    $ gitweb-run.sh "p=git.git;a=blame;f=<file>" > /dev/null 2>&1

Appendix A: ~~~~~~~~~~~ #!/bin/bash

export GATEWAY_INTERFACE="CGI/1.1" export HTTP_ACCEPT="*/*" export REQUEST_METHOD="GET" export QUERY_STRING=""$1"" export PATH_INFO=""$2""

export GITWEB_CONFIG="/home/jnareb/git/gitweb/gitweb_config.perl"

perl -- /home/jnareb/git/gitweb/gitweb.perl

# end of gitweb-run.sh

> When "previous" commit information is available in the output from
> "git blame", can you make use of it?

Yes, I could; but I don't think it got implemented.

-- 
Jakub Narebski
Poland