Re: [PATCH 0/6] Gitweb caching changes v2

From: Jakub Narebski <jnareb@gmail.com>
Date: Dec 11, 2009, 18:01 UTC
Message-ID: <200912111901.35781.jnareb@gmail.com>
In-Reply-To: <4B226D56.7000004@kernel.org>

On Fri, 11 Dec 2009, J.H. (John 'Warthog9' Hawley) wrote:

> Jakub Narebski wrote:
>> "John 'Warthog9' Hawley" <warthog9@kernel.org> writes:

Show 24 quoted lines

>>> John 'Warthog9' Hawley (6):
>>>   GITWEB - Load Checking
>>>   GITWEB - Missmatching git w/ gitweb
>>>   GITWEB - Add git:// link to summary pages
>>>   GITWEB - Makefile changes
>>>   GITWEB - File based caching layer
>> 
>> This patch didn't made it to git mailing list.  I suspect that you ran
>> afoul vger anti-SPAM filter.
>> 
>> Does this "File based caching layer" have anything common with GSoC
>> 2008 project, available at git://repo.or.cz/git/gitweb-caching.git ?
> 
> Yeah, it does seem that way (like I said eaten by a grue), it 
> *currently* has nothing to do with Lea's GSoC code but it is still my 
> intention, long term, to integrate the two.
> 
> The patch, in all it's glory can be viewed at: 
> http://git.kernel.org/?p=git/warthog9/gitweb.git;a=commitdiff;h=42641b1e3bfae14d5cc2e0150355e89cb87951db
> 
> It is anything but a small patch to gitweb, the patch is 117K and 
> comprises 3539 lines (including git header commit information).  There's 
> not any real good way to break it up as it's a bit of an all or nothing 
> patch.

First, why do you reinvent the wheel instead of using one of existing caching interfaces like CHI or Cache::Cache (perhaps creating a custom backend or middle layer which incorporates required features, like being load-aware)? This way changing from file-based cache to e.g. mmap based one or to memcached would be very simple. And you would avoid pitfals in doing your own cache management. perl-Cache-Cache should be available package in extras repositories.

If module is no available this would simply mean no caching, like in many (or not so many) other cases with optional features in gitweb.

Second, if you can't use CGI::Cache directly, you can always steal the idea from it, then the change to gitweb itself would be minimal:

  "Internally, the CGI::Cache module ties the output file descriptor
  (usually STDOUT) to an internal variable to which all output is saved."

P.S. I'll postpone critique of the patch itself for now. The above issues are much more important.

-- 
Jakub Narebski
Poland