Re: [PATCH] Avoid errors from git-rev-parse in gitweb blame

[Jakub Narebski <jnareb@gmail.com>]

Lea Wiemann wrote:
> Jakub Narebski wrote:
> >
> > And snapshots [and blob_plain].  We certainly want to stream snapshots, as
> > they can be quite large.
> 
> Yup.  I suppose that those need to be cached on disk rather than in 
> memory, so they need a separate cache.

Or at least (in the first implementation) to avoid caching them in
memory-based cache (and serve them uncached).

Although I wonder how memory-based caches such as memcached or swifty,
and perhaps also mmap based cache (BerkeleyDB based cache is supposedly
fast because it fits into memory/caches in memory) deals with overly
large cache entries...

> > [Parents in blame output:]
> > It perhaps makes no difference performance wise (solution with
> > "git rev-list --parents --no-walk" has one fork more), but it might
> > make code unnecessarily more complicated.
> 
> A few lines.  *shrugs*  Probably actually easier than adding stuff to 
> git-blame's output, but I won't argue against the latter if you want it.

With modified (enhanced) git-blame output code would look like this
(rough pseudocode):

  while (<$fd>) {
    ...
    <parse 'parent' header>
    ...
  }

while using no-walk rev-list requires list of blamed parents upfront,
so the code would have to look like this

  @blame_data = <$fd>;
  @commitlist = map { <get sha1> } grep { <header line> } @blame_list;
  %commit_parents = get_parents(\@commitlist); # calls git-rev-list
  foreach (@commitlist) {
    ...
    ...
  }

Note that you read whole data into gitweb, inclreasing memory usage...
which we want to avoid, especially when using memcached or similar
caching backend (git-blame itself has to keep data in memory, but no
need to duplicate the amount).


Besides git-blame output needs to be extended/enhanced anyway for the
data mining / annotated file history navigation Luben wanted to be
really robust.  See my response to Linus email in this thread (to be
written).

> > use AJAX together with "git blame --incremental" to reduce latency.
> > It was done by having JavaScript check if browser is AJAX-capable,
> 
> Unfortunately there is no such check (and I doubt it's doable without 
> cookie or redirect trickery) -- you'll find that the blames on 
> repo.or.cz don't work without JavaScript.

I have in my git repository original version (well, one of original
versions) adding incremental blame output

  Message-ID: <20070825222404.16967.9402.stgit@rover>
  http://permalink.gmane.org/gmane.comp.version-control.git/56657

by Petr Baudis, tweaked version of Fredrik Kuivinen patch, and in the
commit message there is the floowing info:

    Compared to the original patch, this one works with pathinfo-ish URLs as
    well, and should play well with non-javascript browsers as well (the HTML
    points to the blame action, while javascript code rewrites the links to use
    the blame_incremental action; it is somewhat hackish but I couldn't think
    of a better solution).

Instead of rewriting links gitweb's JavaScript could use JavaScript
redirect trickery, using JavaScript (by setting location.href for
example) to redirect to blame_incremental action from blame action.


As to checking if browser is AJAX capable: you can at least check
if all methods needed are available.


P.S. You would probably want to remove old git-annotate based git_blame
(dead code, currently not used by any action), and rename git_blame2 to
git_blame.  A bit less code to check for caching problems etc,...
-- 
Jakub Narebski
Poland