Re: [PATCHv3/RFC 4/5] gitweb: Create links leading to 'blame_incremental' using JavaScript

From: Jakub Narebski <jnareb@gmail.com>
Date: Nov 6, 2009, 18:05 UTC
Message-ID: <200911061905.52285.jnareb@gmail.com>
In-Reply-To: <20091105203316.GD17748@machine.or.cz>

On Thu, Nov 05, 2009, Petr Baudis wrote:

Show 25 quoted lines

> On Tue, Sep 01, 2009 at 01:39:19PM +0200, Jakub Narebski wrote:
> > @@ -4806,6 +4820,10 @@ sub git_tag {
> >  
> >  sub git_blame_common {
> >  	my $format = shift || 'porcelain';
> > +	if ($format eq 'porcelain' && $cgi->param('js')) {
> > +		$format = 'incremental';
> > +		$action = 'blame_incremental'; # for page title etc
> > +	}
> >  
> >  	# permissions
> >  	gitweb_check_feature('blame')
> 
>   I'm a bit concerned here. I have somewhat backed out of incremental
> blame myself because I have found (in accord with Junio's old findings)
> that in most cases, incremental blame can be actually slower than normal
> blame because of slow browsers where it takes long to update the page in
> each step.
> 
>   I'm sorry if I missed this in one of your mails, but how fast is
> incremental blame in your implementation? If this still might be an
> issue, I think it should be configurable whether to use incremental
> blame, or perhaps use some quick heuristic wrt. file size (negative
> bias) and history length (positive bias) [not sure if that information
> is quickly available].

Unfortunately I can't benchmark the speed of incremental blame well because of testing it on a single computer.

What I have found that incremental blame spares at least _server time_,
which means that the time to prepare starting view for incremental blame
(with the contents of file in blame format, unblamed) plus the time to
generate incremental blame data is usually about the same or faster
than the time to generate ordinary blame view.  Quite faster if file
have large number of blamed commits:
  $ git blame -p <file> | grep author-time | wc -l

But even if incremental blame turns out to be slower than incremental blame it still has the advantage of being _incremental_. You have at least some result soon. Even more with current implementation which includes progress report for incremental blame.

What needs to be addressed however is to remove totally unnecessary 
critical section / locking code, as JavaScript is single threaded.
We should take care however that JavaScript code of interactive blame
doesn't take all CPU, for example using technique presented in
  "Timed array processing in JavaScript" by Nicholas C. Zakas
  http://www.nczonline.net/blog/2009/08/11/timed-array-processing-in-javascript/

.....................................................................

If you want below there are very simple benchmark of blame and
incremental blame on a _single_ computer:
  AMD Athlon 1 GHz, 512MB RAM, Linux 2.6.14-11.1.aur.2, Apache 2.0.54
I don't remember however if it is for the most current code.

File | 'blame'[1] | 'blame_incremental'[2] ================================================================ blob.h | 2.346s | 0.443s + (2.244s / 2.921s) GIT-VERSION-GEN | 2.449s | 1.346s + (3.157s / 3.876s) README | 2.713s | 0.508s + (2.952s / 3.659s) revision.c | 19.964s | 4.872s + (11.306s / 32.124s) gitweb/gitweb.perl | 83.912s | 12.069s + (52.922s / 223.133s)

$ git blame --porcelain   gitweb/gitweb.perl >/dev/null
  0m11.437s user + 0m0.740s sys, 66300minor pagefaults
$ git blame --incremental gitweb/gitweb.perl >/dev/null
  0m11.477s user + 0m0.816s sys, 68945minor pagefaults

Footnotes: ~~~~~~~~~~ [1] Total wall-clock time as returned by gitweb in the page footer. [2] XXs + (XXs server blame_data / XXs client JavaScript).

-- 
Jakub Narebski
Poland