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

From: Jakub Narebski <jnareb@gmail.com>
Date: Jun 4, 2008, 12:31 UTC
Message-ID: <200806041431.07494.jnareb@gmail.com>
In-Reply-To: <4845E45E.9030504@gmail.com>

Lea Wiemann wrote:

Show 7 quoted lines

> 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...

Show 7 quoted lines

> > [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).

Show 6 quoted lines

> > 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