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

[Jakub Narebski <jnareb@gmail.com>]

On Wed, 10 Dec 2008, Nanako Shiraishi wrote:
> 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