Subject: Re: [RFC PATCH 00/10] gitweb: Simple file based output caching

From: "J.H." <[email-masked]>
Date: Fri, 05 Feb 2010 16:51:17 -0800

> Table of contents:
> ~~~~~~~~~~~~~~~~~~
>  [RFC PATCH 01/10] gitweb: Print to explicit filehandle (preparing
>                    for caching)

This looks fine, I did some quick testing to verify that this would work
- and it does.

The only caveat that needs to be aware is that if the layer is going to
output binary data it needs to flip the whole stream to :raw before
outputting (this is going to be more specific to the caching layer).

One advantage to having the file handles separate is that it's easier to
distinguish if the data is going to need to be binary data that will
need to be flipped properly.

Also means you could cache the binary data differently than textual data.

I.E. binary data gets saved to disk, but page data gets saved to memcached.

Just food for thought, I'm not sure which way makes more sense
personally, though I would have a tendency to err on the side of
flexibility and have both.

>  [RFC PATCH 02/10] gitweb: href(..., -path_info => 0|1)

note: delaying additional comment till I've finished reading through the
basics of the following patches.

>  [RFC PATCH 03/10] gitweb/cache.pm - Very simple file based caching

Ok this is quite the departure from what I had, I'm unsure that it's the
right way to go, but it obviously has merits (I.E. much simpler addition
of any Cache::Cache or CHI compatible caching layer)

This patch itself looks fine, and as it states it borrows heavily from
my basic implementation - just wraps it differently.  I might have some
thoughts on extending this a bit to be a bit more flushed out from a
basic standpoint.

Need to dig through it some more, but I'm generally ok with it.

>  [RFC PATCH 04/10] gitweb/cache.pm - Stat-based cache expiration

Looks fine to me, though the note about getting the errors should get
moved to previous patch, as it says.

Note: I'm going to stop here as the following are WIP and I want to play
around with this particular direction on my own a little more before
further comment.  There's some ideas running around I want to try and
get down in code first.  Me moving on and trying these other ideas is
not a reflection on the following patches, just some alternative
thinking before I discuss some other ideas on the following patches.

Also I've been sitting on this e-mail in this state for almost a week
while I've been playing with this and having to fight other fires and I
know that Jakub has been looking for commentary on this.

>  [RFC PATCH 05/10] gitweb: Use Cache::Cache compatibile (get, set)
>                    output caching (WIP)
>  [RFC PATCH 06/10] gitweb/cache.pm - Adaptive cache expiration time (WIP)
>  [RFC PATCH 07/10] gitweb: Use CHI compatibile (compute method) caching (WIP)
>  [RFC PATCH 08/10] gitweb/cache.pm - Use locking to avoid 'stampeding herd'
>                    problem (WIP)
>  [RFC PATCH 09/10] gitweb/cache.pm - Serve stale data when waiting for
>                    filling cache (WIP)
>  [RFC PATCH 10/10] gitweb: Show appropriate "Generating..." page when
>                    regenerating cache (WIP)