From: Lea Wiemann <lewiemann@gmail.com>
Date: Sat, 31 May 2008 12:39:58 GMT
Subject: Re: [PATCH] gitweb: use Git.pm, and use its parse_rev method for git_get_head_hash
Message-ID: <4841471E.2070302@gmail.com>
In-Reply-To: <m3lk1q24nb.fsf@localhost.localdomain>

Jakub Narebski wrote:
> But I'm not sure for example [that] parsing subroutines [that parse the
> output of the git commands should be in Git.pm].
> 
> [...] IMVHO caching command output is bad idea.  I'd rather have gitweb
> cache _parsed_ data (i.e. Perl structures).

Yes, I agree with you that parsed data should be cached, but for that 
reason I think that the subroutine that parse git's output should be in 
Git.pm, not in Gitweb:

If you want to cache parsed data, you need the caching layer in between 
the application (= Gitweb) and the repository access layer (= command 
output parsing), or you would have to insert cache code at every call to 
the repository access layer.  So you end up with these layers:

(Layer 0: Front-end [HTML] caching.)
Layer 1: Application (Gitweb)
Layer 2: Back-end caching
Layer 3: Repository access (command parsing)
Layer 4: Calls to the git binary

Layer 3 and 4 are application-independent (i.e. not Gitweb specific), 
and since they form a usable API, they might as well be written as a 
separate API rather than lumped together with Gitweb.  Git.pm is a start 
of such an API (it does layer 4 and a little bit of layer 3), so it 
seems natural for me to extend it.

Layer 2 is application-independent as well, so it can become an extra 
class in Git.pm or a separate module.  (It should stay independent of 
layers 3 and 4).

We may need to have an explicitly implemented layer 0 (front-end 
caching) in Gitweb for at least a subset of pages (like project pages), 
but we'll see if that's necessary.

> Caching git command output (something like IPC::Cmd::Cached?)
> would only repeat bad I/O patterns of git commands.

I hope you're not assuming that the back-end cache will reside on disk 
-- the problem is IO throughput, so having a cache on disk can really 
only work for a front-end cache.  For the back-end cache, we *will* have 
to use a memory cache (or no cache at all).

> P.S. Here it is what could go [from Gitweb] to Git.pm: [snip]

Good points, I agree those should eventually be moved to Git.pm.  (They 
are not my priority at the moment, since I really want to implement 
caching.  So unless they move naturally as part of my refactoring, I'll 
leave them in Gitweb for now.)

-- Lea