gitweb: Simple file based output caching TODO (was: Re: [RFC PATCHv3 00/10] gitweb: Simple file based output caching)

From: Jakub Narebski <jnareb@gmail.com>
Date: Feb 28, 2010, 02:54 UTC
Message-ID: <201002280354.51366.jnareb@gmail.com>
In-Reply-To: <1266349005-15393-1-git-send-email-jnareb@gmail.com>

On Tue, 16 Feb 2010, Jakub Narebski wrote:

> This 10 patches long patch series is intended (yet) as preliminary version
> for splitting large 'gitweb: File based caching layer (from git.kernel.org)'
> mega-patch by John 'Warthog9' Hawley aka J.H., by starting small and
> adding features piece by piece.

[...]

Show 13 quoted lines

> Shortlog:
> ~~~~~~~~~
> Jakub Narebski (10):
>   gitweb: href(..., -path_info => 0|1)
>   gitweb/cache.pm - Very simple file based cache
>   gitweb/cache.pm - Stat-based cache expiration
>   gitweb: Use Cache::Cache compatibile (get, set) output caching
>   gitweb/cache.pm - Adaptive cache expiration time
>   gitweb: Use CHI compatibile (compute method) caching
>   gitweb/cache.pm - Use locking to avoid 'cache miss stampede' problem
>   gitweb/cache.pm - Serve stale data when waiting for filling cache
>   gitweb/cache.pm - Regenerate (refresh) cache in background
>   gitweb: Show appropriate "Generating..." page when regenerating cache

Here is the list of things that needs to be addressed in the future next (v4) version of this series, hopefully finally not an RFC.

* The caching engine (GitwebCache::SimpleFileCache) starts with default
  expire time of "never" (-1), while later it uses gitweb defaults when
  adaptive caching lifetime is added (20 / 1200 seconds).  This (slight)
  inconsistency should be fixed, either by using default of "never", or
  by using gitweb defaults for caching engine defaults in both patches:
    gitweb/cache.pm - Stat-based cache expiration
    gitweb/cache.pm - Adaptive cache expiration time

  Note that caching engine defaults are *independent* of gitweb's 
  defaults in %cache_options.

* Describe (better than it is now) in comments or in commit message
  why Temp::File is used for 'atomic write' _without_ locking (even
  when there might be more than one process (re)generating the same
  cache entry simultaneously).

* [improvement].  When using locking after
    gitweb/cache.pm - Use locking to avoid 'cache miss stampede' problem
  it is ensured that only one process would (re)generate cache entry.
  Therefore Temp::File is not needed for temporary file; the temporary
  file can have constant name.  This should improve performance a bit.

  But this might be unnecessary complication.

* Show information about when page was generated in the footer always
  when caching is enabled; currently it is shown only if caching *and*
  'timed' feature is enabled 
    gitweb: Use Cache::Cache compatibile (get, set) output caching

* Actually check that using alternate caching engine works.  It can be
  done (what is described in gitweb/README) by setting $cache to either
  cache engine class (package) name, or to cache object (instantiated
  cache).

* [cleanup] Remove commented out alternate solutions (commented out
  code).

* Benchmark overhead of caching, and performance of caching, perhaps for
  different caching engines including original patch by J.H.

John, Pasky, do you have any further comments / requests?

-- 
Jakub Narebski
Poland