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

[Luben Tuikov <ltuikov@yahoo.com>]

--- On Tue, 12/9/08, Jakub Narebski <jnareb@gmail.com> wrote:
> From: Jakub Narebski <jnareb@gmail.com>
> Subject: [PATCH 2/3] gitweb: Cache $parent_commit info in git_blame()
> To: git@vger.kernel.org
> Cc: "Luben Tuikov" <ltuikov@yahoo.com>, "Jakub Narebski" <jnareb@gmail.com>
> Date: Tuesday, December 9, 2008, 2:48 PM
> Luben Tuikov changed 'lineno' link from leading to
> commit which lead
> to current version of given block of lines, to leading to
> parent of
> this commit in 244a70e (Blame "linenr" link jumps
> to previous state at
> "orig_lineno").  This supposedly made data mining
> possible (or just
> better).

Before 244a70e, clicking on linenr links would display
the same commit id as displayed to the left, which is no
different than the block of lines displayed, thus data
mining was impossible, i.e. I had to manually (commands)
go back in history to see how this line or block of lines
developed and/or changed.

244a70e didn't make data mining perfect, just possible.

> This patch attempts to migitate issue 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?

   Luben


> 
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
> That is what I have noticed during browsing git_blame()
> code.

What?

> We can change it to even more effective implementation
> (like the ones
> proposed above in the commit message) later.

Where?

> 
> Indenting is cause for artifically large diff
> 
>  gitweb/gitweb.perl |   16 +++++++++++-----
>  1 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 1b800f4..916396a 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -4657,11 +4657,17 @@ HTML
>  			              esc_html($rev));
>  			print "</td>\n";
>  		}
> -		open (my $dd, "-|", git_cmd(),
> "rev-parse", "$full_rev^")
> -			or die_error(500, "Open git-rev-parse
> failed");
> -		my $parent_commit = <$dd>;
> -		close $dd;
> -		chomp($parent_commit);
> +		my $parent_commit;
> +		if (!exists $meta->{'parent'}) {
> +			open (my $dd, "-|", git_cmd(),
> "rev-parse", "$full_rev^")
> +				or die_error(500, "Open git-rev-parse
> failed");
> +			$parent_commit = <$dd>;
> +			close $dd;
> +			chomp($parent_commit);
> +			$meta->{'parent'} = $parent_commit;
> +		} else {
> +			$parent_commit = $meta->{'parent'};
> +		}
>  		my $blamed = href(action => 'blame',
>  		                  file_name =>
> $meta->{'filename'},
>  		                  hash_base => $parent_commit);