On Wed, 10 Dec 2008, Luben Tuikov wrote:
> --- On Tue, 12/9/08, Jakub Narebski <jnareb@xxxxxxxxx> wrote:
> > This patch attempts to migitate issue [of performance] a bit by
> > caching $parent_commit info in %metainfo, which makes gitweb to
> > call git-rev-parse only once per unique commit in blame output.
>
> Have you tested this patch that it gives the same commit chain
> as before it?
The only difference between precious version and this patch is that
now, if you calculate sha-1 of $long_rev^, it is stored in
$metainfo{$long_rev}{'parent'} and not calculated second time.
But I have checked that (at least for single example file) the blame
output is identical for before and after this patch.
> > That is what I have noticed during browsing git_blame()
> > code.
>
> What?
What I have noticed? I have noticed this inefficiency.
Why I was browsing git_blame? To write git_blame_incremental...
See also my reply to Nanako Shiraishi with simple benchmark.
> > We can change it to even more effective implementation
> > (like the ones proposed above in the commit message) later.
>
> Where?
jn> Unfortunately the implementation in 244a70e used one call for
jn> git-rev-parse to find parent revision per line in file, instead of
jn> using long lived "git cat-file --batch-check" (which might not existed
jn> then), or changing validate_refname to validate_revision and made it
jn> accept <rev>^, <rev>^^, <rev>^^^ etc. syntax.
One solution mentioned there is to open bidi pipe (like in Git::Repo
in gitweb caching by Lea Wiemann) to "git cat-file --batch-check"
(the '--batch-check' option was added to git-cat-file by Adam Roben
on Apr 23, 2008 in v1.5.6-rc0~8^2~9), feed it $long_rev^, and parse
its output of the form:
926b07e694599d86cec668475071b32147c95034 commit 637
see manpage for git-cat-file(1). Unfortunately it seems like
command_bidi_pipe doesn't work as _I_ expected...
Another solution mentioned there is to change validate_refname to
validate_revision when checking script parameters (CGI query or
path_info), with validate_revision being something like:
sub validate_revision {
my $rev = shift;
return validate_refname(strip_rev_suffixes($rev));
}
or something like that, so we don't need to calculate $long_rev^,
but can pass "$long_rev^" as 'hb' parameter ($long_rev can in turn
also end in '^').
--
Jakub Narebski
Poland
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
