> Shortlog: > ~~~~~~~~~ > Jakub Narebski (10): > gitweb: href(..., -path_info => 0|1) This still looks fine to me. > gitweb/cache.pm - Very simple file based cache This looks fine to me, and I'm much more keen on using the CHI interface. > gitweb/cache.pm - Stat-based cache expiration - I think having expires time at -1 (never) is a dangerous default. I think having it as an option is fine, but setting this to something like 6 months may be a better option. I wouldn't expect this to be an issue in reality, but I can just see someone screwing up and having a cache system that *never* expires. I think I'd rather see something long vs. never. But that's just my opinion. - This does have the problem (though this gets fixed later on, again) that it would return invalid if there's a process already updating the cache. This might need to be fixed later to understand the locking structures on what is/isn't valid (this is mostly a note for me while I read through the patches) > gitweb: Use Cache::Cache compatibile (get, set) output caching - It might be worth (in a later patch) enabling the PerlIO layers as there was a significant speed improvement in using them, despite their non-standardness. - What if you want to use a different caching library but don't want cache.pm ? That first bit of "if ($caching_enabled) {" might need to be wrapped to figure out if we are using our inbuilt caching or an external caching system. (just thinking out loud) Looks fine to me. > gitweb/cache.pm - Adaptive cache expiration time is the commented: #return &{$self->{'check_load'}}(); intended in check_load() ? otherwise this looks fine to me. > gitweb: Use CHI compatibile (compute method) caching Looks fine to me. I think it's fine where it's at in the order myself. > gitweb/cache.pm - Use locking to avoid 'cache miss stampede' problem Looks fine to me. > gitweb/cache.pm - Serve stale data when waiting for filling cache Looks fine to me. > gitweb/cache.pm - Regenerate (refresh) cache in background Looks good to me. > gitweb: Show appropriate "Generating..." page when regenerating cache I've got a couple of things that will need to be added on top of this (mainly to try and guess if the client is dumb or smart) so that if it won't support generating... as expected it doesn't get it. But that can come in a later patch. This looks fine to me. > > Diffstat: > ~~~~~~~~~ > gitweb/README | 70 ++++ > gitweb/cache.pm | 655 ++++++++++++++++++++++++++++++++ > gitweb/gitweb.perl | 321 +++++++++++++++- > t/gitweb-lib.sh | 2 + > t/t9500-gitweb-standalone-no-errors.sh | 19 + > t/t9503-gitweb-caching.sh | 32 ++ > t/t9503/test_cache_interface.pl | 404 ++++++++++++++++++++ > t/test-lib.sh | 3 + > 8 files changed, 1486 insertions(+), 20 deletions(-) > create mode 100644 gitweb/cache.pm > create mode 100755 t/t9503-gitweb-caching.sh > create mode 100755 t/t9503/test_cache_interface.pl > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html