{"thread":{"id":"22584","subject":"[RFC PATCHv2 00/10] gitweb: Simple file based output caching","startedAt":"2010-02-09T10:30:17Z","lastAt":"2010-02-10T20:32:54Z","messageCount":18,"participants":["Jakub Narebski","Petr Baudis"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"134015","messageId":"1265711427-15193-1-git-send-email-jnareb@gmail.com","threadId":"22584","inReplyTo":null,"subject":"[RFC PATCHv2 00/10] gitweb: Simple file based output caching","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-09T10:30:17Z","receivedAt":"2010-02-09T10:30:17Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"This 10 patches long patch series is intended as preliminary version\nfor splitting large 'gitweb: File based caching layer (from git.kernel.org)'\nmega-patch by John 'Warthog9' Hawley aka J.H., by starting small and\nadding features piece by piece.\n\nThis is second version (second release) of this series; previous is\navailable at http://repo.or.cz/w/git/jnareb-git.git as 'gitweb/cache-kernel'\nbranch. It was sent as:\n* [RFC PATCH 00/10] gitweb: Simple file based output caching\n Message-Id: \n http://thread.gmane.org/gmane.comp.version-control.git/136913/focus=136917\n(note however that v1 series of emails is lacking one of patches because\nit was over VGER anti-spam size limit for messages).\n\nThis version tries to do without \n gitweb: Print to explicit filehandle (preparing for caching)\npatch, by capturing output using either PerlIO layers manipulated\nusing PerlIO::Util if this module is available, or direct manipulation\nof *STDOUT if PerlIO::Util isn't available. One of the goals of this\nseries is then decide whether it is worth to have explicit filehandle\nin print statements in gitweb, or not; if the complexity is worth not\nhaving to deal with straightforward but quite intrusive (and large)\npatch.\n\nAs the earlier version was inspired by file-based caching in\nCache::Cache, this one is inspired by file-based caching in more\nmodern CHI (unified cache interface).\n\nIt still lacks POD for gitweb/cache.pm (would it be needed, or would\ncomments be enough), and gitweb/cache.pm still ties rather heavily\ninto gitweb (following still what was in original J.H. (mega)patch).\n\nIt *does* have quite detailed commit messages, as opposed to v1 of\nthis series, where some commits were described only in comment section\nof emails containing them. It is also very configurable (Pasky, this\nwould probably be of interest to you, as you didn't want to have \n\"Generating...\" pages enabled), even more than in original patch\nby J.H.\n\n\nNOTE: there are quite a bit of _API_ tests, but I have not tested gitweb\noutput with caching enabled extensively (thats how bug in \"Generating...\"\nslipped through - for details see comments in last patch). I have tested\nthat caching works around 4th patch in series, in that it doesn't cause\nerrors and displays page (here the lack of error handling is decidely\nunhelpful), and that it displays the time when page was generated. As I\nhave installed PerlIO::Util using local::lib, i.e. locally in ~/perl5,\nI think that what I have been testing was the \"*STDOUT munging\" method\nof capturing gitweb output. (See \"Capturing gitweb output\" section\nin PATCHv2 04/10).\n\n\nThis series is based on commit 8424981934c415bd20643de9cc932bd348dfb115:\n(in the 'master' branch of git.git repository)\n Jeff King (1):\n Fix invalid read in quote_c_style_counted\n\nand is available in the git repository at:\n\n git://repo.or.cz/git/jnareb-git.git gitweb/cache-kernel-v2\n\nJakub Narebski (10):\n gitweb: href(..., -path_info => 0|1)\n gitweb/cache.pm - Very simple file based caching\n gitweb/cache.pm - Stat-based cache expiration\n gitweb: Use Cache::Cache compatible (get, set) output caching\n gitweb/cache.pm - Adaptive cache expiration time\n gitweb: Use CHI compatible (compute method) caching\n gitweb/cache.pm - Use locking to avoid 'cache miss stampede' problem\n gitweb/cache.pm - Serve stale data when waiting for filling cache\n gitweb/cache.pm - Regenerate (refresh) cache in background\n gitweb: Show appropriate \"Generating...\" page when regenerating cache\n\nNote that compared to previous version of this series, this version\nlacks initial commit.\n gitweb: Print to explicit filehandle (preparing for caching)\nThis is a bit of an experiment if we can do caching without large patch\nto gitweb upfront, and to decide whether tradeoff (more complicated\ncapturing) is worth it.\n\nAlso, one of the commits:\n gitweb/cache.pm - Serve stale data when waiting for filling cache (WIP)\nwas split into two separate commits:\n gitweb/cache.pm - Serve stale data when waiting for filling cache\n gitweb/cache.pm - Regenerate (refresh) cache in background\none serving stale data (in processes waiting for cache to be filled, \naka readers), and one adding background cache regeneration. \n\nAfter previous series I have sent additional (PATCH 11/10) patch:\n gitweb: Ajax-y \"Generating...\" page when regenerating cache (WIP)\nThis patch would require rework to apply to this new series.\n\nDiffstat:\n~~~~~~~~~\n gitweb/README | 70 +++++\n gitweb/cache.pm | 530 ++++++++++++++++++++++++++++++++\n gitweb/gitweb.perl | 305 +++++++++++++++++-\n t/gitweb-lib.sh | 2 +\n t/t9500-gitweb-standalone-no-errors.sh | 19 ++\n t/t9503-gitweb-caching.sh | 32 ++\n t/t9503/test_cache_interface.pl | 380 +++++++++++++++++++++++\n t/test-lib.sh | 3 +\n 8 files changed, 1325 insertions(+), 16 deletions(-)\n create mode 100644 gitweb/cache.pm\n create mode 100755 t/t9503-gitweb-caching.sh\n create mode 100755 t/t9503/test_cache_interface.pl\n"},{"id":"134018","messageId":"1265711427-15193-2-git-send-email-jnareb@gmail.com","threadId":"22584","inReplyTo":"1265711427-15193-1-git-send-email-jnareb@gmail.com","subject":"[RFC PATCHv2 01/10] gitweb: href(..., -path_info => 0|1)","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-09T10:30:18Z","receivedAt":"2010-02-09T10:30:18Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"If named boolean option -path_info is passed to href() subroutine, use\nits value to decide whether to generate path_info URL form. If this\noption is not passed, href() queries 'pathinfo' feature to check\nwhether to generate path_info URL (if generating path_info link is\npossible at all).\n\nhref(-replay=>1, -path_info=>0) is meant to be used to generate a key\nfor caching gitweb output; alternate solution would be to use freeze()\nfrom Storable (core module) on %input_params hash (or its reference),\ne.g.:\n $key = freeze \\%input_params;\nor other serialization technique.\n\nWhile at it document extra options/flags to href().\n\nSigned-off-by: Jakub Narebski \n---\nThere is no change in the patch with v2; only the explanation in the\ncommit message got changed a bit.\n\n\nThe reason behind -path_info=>0 is that we want to have the same cache\nentry (the same cache file) regardless of whether we use path_info\nURL, or non-path_info URL.\n\nJ.H. recommends using href(...,full=>1), to be able to have two gitweb\ninstallations (or one gitweb installation but with $projects_root \ndepending on virtual server used) to share cache without worrying\nthat they would stomp over each other cache entries. On the other\nhand this means that moving to another server [name], or changin\n[virtual] location of gitweb, means that you need to re-cache\neverything. The discussion can be found in:\n http://thread.gmane.org/gmane.comp.version-control.git/136913/focus=137061\n\nNote that in the caching patch by J.H. from \"Gitweb caching v5\" thread\n(and top commit in git://git.kernel.org/pub/scm/git/warthog9/gitweb.git,\ngitweb-ml-v5 branch) the key was generated as \"$my_url?\".$ENV{'QUERY_STRING'}\nwhich wouldn't work with path_info URLs, but on the other hand gitweb\nat git.kernel.org doesn't use path_info URLs (perhaps even doesn't\nsupport them).\n\nUsing href(replay=>1,full=>1,path_info=>0) has additional advantage\nover using $cgi->self_url() in that it also does not depend on\nordering of parameters in handcrafted URLs.\n\n gitweb/gitweb.perl | 7 ++++++-\n 1 files changed, 6 insertions(+), 1 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 1f6978a..97ea3ec 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -970,6 +970,10 @@ exit;\n ## ======================================================================\n ## action links\n \n+# possible values of extra options\n+# -full => 0|1 - use absolute/full URL ($my_uri/$my_url as base)\n+# -replay => 1 - start from a current view (replay with modifications)\n+# -path_info => 0|1 - don't use/use path_info URL (if possible)\n sub href {\n \tmy %params = @_;\n \t# default is to use -absolute url() i.e. $my_uri\n@@ -986,7 +990,8 @@ sub href {\n \t}\n \n \tmy $use_pathinfo = gitweb_check_feature('pathinfo');\n-\tif ($use_pathinfo and defined $params{'project'}) {\n+\tif (defined $params{'project'} &&\n+\t (exists $params{-path_info} ? $params{-path_info} : $use_pathinfo)) {\n \t\t# try to put as many parameters as possible in PATH_INFO:\n \t\t# - project name\n \t\t# - action\n-- \n1.6.6.1\n"},{"id":"134021","messageId":"1265711427-15193-3-git-send-email-jnareb@gmail.com","threadId":"22584","inReplyTo":"1265711427-15193-1-git-send-email-jnareb@gmail.com","subject":"[RFC PATCHv2 02/10] gitweb/cache.pm - Very simple file based caching","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-09T10:30:19Z","receivedAt":"2010-02-09T10:30:19Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"This is first step towards implementing file based output (response)\ncaching layer that is used on such large sites as kernel.org.\n\nThis patch introduces GitwebCaching::SimpleFileCache package, which\nfollows Cache::Cache / CHI interface, although do not implement it\nfully. The intent of following established convention is to be able\nin the future to replace our simple file based cache e.g. by one using\nmemcached.\n\nLike in original patch by John 'Warthog9' Hawley (J.H.) (the one this\ncommit intends to be incremental step to), the data is stored in the\ncase as-is, without adding metadata (like expiration date), and\nwithout serialization (which means only scalar data).\n\nTo be implemented (from original patch by J.H.):\n* cache expiration (based on file stats, current time and global\n expiration time); currently elements in cache do not expire\n* actually using this cache in gitweb, except error pages\n* adaptive cache expiration, based on average system load\n* optional locking interface, where only one process can update cache\n (using flock)\n* server-side progress indicator when waiting for filling cache,\n which in turn requires separating situations (like snapshots and\n other non-HTML responses) where we should not show 'please wait'\n message\n\nPossible extensions (beyond what was in original patch):\n* (optionally) show information about cache utilization\n* AJAX (JavaScript-based) progress indicator\n* JavaScript code to update relative dates in cached output\n* make cache size-aware (try to not exceed specified maximum size)\n* utilize X-Sendfile header (or equivalent) to show cached data\n (optional, as it makes sense only if web server supports sendfile\n feature and have it enabled)\n* variable expiration feature from CHI, allowing items to expire a bit\n earlier than the stated expiration time to prevent cache miss\n stampedes (although locking, if available, should take care of\n this).\n\nThe code of GitwebCaching::SimpleFileCache package in gitweb/cache.pm\nwas heavily based on file-based cache in Cache::Cache package, i.e.\non Cache::FileCache, Cache::FileBackend and Cache::BaseCache, and on\nfile-based cache in CHI, i.e. on CHI::Driver::File and CHI::Driver\n(including implementing atomic write, something that original patch\nlacks).\n\nThis patch does not yet enable output caching in gitweb (it doesn't\nhave all required features yet); on the other hand it includes tests,\ncurrently testing only cache Perl API.\n\nInspired-by-code-by: John 'Warthog9' Hawley \nSigned-off-by: Jakub Narebski \n---\nPrevious version of this patch had large parts of this code are based\n_heavily_ on Cache::FileCache implementation (including Cache::FileBackend\nand Cache::BaseCache) from Cache::Cache distribution (which is dual licensed\nusing (Perl) Artistic License and GNU General Public License, like Perl\nitself).\n\nThis version is based on file-based caching from CHI (Unified Caching\nInterface), which is more modern package, and I think better engineered\n(with better design). Nevertheless gitweb/cache.pm is meant to use minimal\ndependencies, like gitweb itself, so it won't use Moose / Mouse for OOP\n(Object Oriented Programming) like CHI does. CHI is also licensed \"under\nthe same terms as Perl itself\", which means dual Artistic/GPL licensed.\n\nNote that GitwebCache::SimpleFileCache does implement neither CHI interface\nnor Cache::Cache interface fully; it is means to be very simple, but\nhopefully fast cache.\n\nDifferences from v1:\n* Allow for alternate CHI-compatible names for parameters passed to\n GitwebCache::SimpleFileCache constructor: 'cache_root' / 'root_dir',\n 'cache_depth' / 'depth' (not sure if it is worth it).\n* Accessors (getters and setters, i.e. ->get_depth() / ->set_depth($depth)\n methods) are generated, instead of using lot of boilerplate code. The\n names of fields changed a bit, too.\n* Change the design of \"engine\" part from inspired by older Cache::Cache\n to inspired by newer and more modern CHI. Among others this means that\n there are no 'private' subroutines (with '_'-prefixed names), and that\n inner (engine) methods no longer pass namespace as parameter - for example\n in place of ->restore($namespace, $key) there is ->fetch($key).\n* ->fetch() and ->store() methods use fast slurp (from CHI::Driver::File,\n which in turn has it adapted from File::Slurp), which uses sys{read,write}.\n* ->fetch() and ->store() uses ':raw' (binary) mode to read and write data.\n* ->set() has protection against writing undefined data (which would fail).\n* Does not use new make_path, but older mkpath interface from File::Path;\n this way we do not require File::Path version >= 2.0.\n\nDifferences from relevant parts of J.H. patch:\n* Does not use make_path, but mkpath from File::Path (see above)\n* Uses catfile/catdir from File::Spec (see below)\n* Writes to cache file atomically, by writing to unique temporary file\n and then renaming/moving this file to the cache file. This is to make\n caching more robust, by preventing readers from getting partial data.\n The issue this prevents should be very rare, as we write whole data\n (whole page) at once, after it is generated in full, so this situation\n can theoretically happen only in the case of heavy load, many clients,\n and very large page over the size of (file)system cache.\n* Currently it does not do any error reporting, as opposed to J.H. patch,\n in which errors are reported via die_error from gitweb (entangling\n cache.pm and gitweb.perl even more). This is planned to be fixed in the\n final version of this patch series.\n* The depth of cache hierarchy is configurable, while J.H. patch uses\n hardcoded depth of 1 subdirectory. It uses unpack rather than substr.\n For 06b90e786e... key digest it uses 06/b90e786e... (with cache_depth = 1)\n rather than 06/06b90e786e... in J.H. patch. It does not have binary\n and non-binary cache data separately; it does not use '.bin' suffix\n (extension) for binary-data cache files.\n* GitwebCache::SimpleFileCache uses standard ->get, ->set, ->compute\n methods, which should allow easy replacing it with CHI or Cache::Cache\n cache (see later commit, adding caching support to gitweb).\n* Tests of caching API in t/t9503-gitweb-caching.sh\n* Better Perl style, more consistent with what gitweb itself uses.\n\nPossible improvements:\n* CHI uses fast_catpath from CHI::Util, which for Unix reduces to simple\n join(\"/\", @_). GitwebCache::SimpleFileCache uses catpath (remainder of\n Cache::FileCache-based code), but gitweb itself uses just \"$root/$path\",\n so this over-portability just wastes performance.\n* We use File::Temp to generate temporary file to write data. CHI uses\n instead unique ids (Data::UUID to generate initial unique id, then suffix\n it with serial number, in hexadecimal) for efficiency.\n Note that with locking (introduced later in series) we could write to\n temporary file with pre-defined (not randomized/incremental) name.\n* GitwebCache::SimpleFileCache over-uses accessors in its methods, where\n referring directly to object fields would be (usually) enough. Also\n the fields probably should be just named 'field_key' rather than \n '_Field_key' (again its remainder of old-style Cache::FileCache-based\n code).\n* Separate change to t/test-lib.sh, adding exporting TEST_DIRECTORY and\n TRASH_DIRECTORY in test_external to allow its use in external tests,\n to a separate commit (it is required for t9503 to find cache.pm).\n* I should probably update copyright statement in cache.pm (just noticed)\n\n gitweb/cache.pm | 269 +++++++++++++++++++++++++++++++++++++++\n t/t9503-gitweb-caching.sh | 32 +++++\n t/t9503/test_cache_interface.pl | 84 ++++++++++++\n t/test-lib.sh | 3 +\n 4 files changed, 388 insertions(+), 0 deletions(-)\n create mode 100644 gitweb/cache.pm\n create mode 100755 t/t9503-gitweb-caching.sh\n create mode 100755 t/t9503/test_cache_interface.pl\n\ndiff --git a/gitweb/cache.pm b/gitweb/cache.pm\nnew file mode 100644\nindex 0000000..aebab01\n--- /dev/null\n+++ b/gitweb/cache.pm\n@@ -0,0 +1,269 @@\n+# gitweb - simple web interface to track changes in git repositories\n+#\n+# (C) 2006, John 'Warthog9' Hawley \n+#\n+# This program is licensed under the GPLv2\n+\n+#\n+# Gitweb caching engine\n+#\n+\n+{\n+# Minimalistic cache that stores data in the filesystem, without serialization\n+# and currently without any kind of cache expiration (all keys last forever till\n+# they got explicitely removed)\n+#\n+# It follows Cache::Cache and CHI interface (but does not implement it fully)\n+\n+package GitwebCache::SimpleFileCache;\n+\n+use strict;\n+use warnings;\n+\n+use File::Path qw(mkpath);\n+use File::Spec;\n+use File::Temp;\n+use Digest::MD5 qw(md5_hex);\n+\n+# by default, the cache nests all entries on the filesystem two\n+# directories deep\n+\n+our $DEFAULT_CACHE_DEPTH = 2;\n+\n+# by default, the root of the cache is located in 'cache'.\n+\n+our $DEFAULT_CACHE_ROOT = \"cache\";\n+\n+# ......................................................................\n+# constructor\n+\n+# The options are set by passing in a reference to a hash containing\n+# any of the following keys:\n+# * 'namespace'\n+# The namespace associated with this cache. This allows easy separation of\n+# multiple, distinct caches without worrying about key collision. Defaults\n+# to '' (which does not allow for simple implementation of clear() method).\n+# * 'cache_root' (Cache::FileCache compatibile),\n+# 'root_dir' (CHI::Driver::File compatibile),\n+# The location in the filesystem that will hold the root of the cache.\n+# Defaults to 'cache', relative to gitweb.cgi directory.\n+# * 'cache_depth' (Cache::FileCache compatibile),\n+# 'depth' (CHI::Driver::File compatibile),\n+# The number of subdirectories deep to cache object item. This should be\n+# large enough that no cache directory has more than a few hundred objects.\n+# Defaults to 1 unless explicitly set.\n+sub new {\n+\tmy ($proto, $p_options_hash_ref) = @_;\n+\n+\tmy $class = ref($proto) || $proto;\n+\tmy $self = {};\n+\t$self = bless($self, $class);\n+\n+\tmy ($root, $depth, $ns);\n+\tif (defined $p_options_hash_ref) {\n+\t\t$root =\n+\t\t\t$p_options_hash_ref->{'cache_root'} ||\n+\t\t\t$p_options_hash_ref->{'root_dir'};\n+\t\t$depth =\n+\t\t\t$p_options_hash_ref->{'cache_depth'} ||\n+\t\t\t$p_options_hash_ref->{'depth'};\n+\t\t$ns = $p_options_hash_ref->{'namespace'};\n+\t}\n+\t$root = $DEFAULT_CACHE_ROOT unless defined($root);\n+\t$depth = $DEFAULT_CACHE_DEPTH unless defined($depth);\n+\t$ns = '' unless defined($ns);\n+\n+\t$self->set_root($root);\n+\t$self->set_depth($depth);\n+\t$self->set_namespace($ns);\n+\n+\treturn $self;\n+}\n+\n+# ......................................................................\n+# accessors\n+\n+# http://perldesignpatterns.com/perldesignpatterns.html#AccessorPattern\n+\n+foreach my $i (qw(depth root namespace)) {\n+\tmy $field = $i;\n+\tno strict 'refs';\n+\t*{\"get_$field\"} = sub {\n+\t\tmy $self = shift;\n+\t\treturn $self->{'_'.ucfirst($field)};\n+\t};\n+\t*{\"set_$field\"} = sub {\n+\t\tmy ($self, $value) = @_;\n+\t\t$self->{'_'.ucfirst($field)} = $value;\n+\t};\n+}\n+\n+# ----------------------------------------------------------------------\n+# utility functions and methods\n+\n+# Return root dir for namespace (lazily built, cached)\n+sub path_to_namespace {\n+\tmy ($self) = @_;\n+\n+\tif (!exists $self->{'_Path_to_namespace'}) {\n+\t\t$self->{'_Path_to_namespace'} = File::Spec->catfile(\n+\t\t\t$self->get_root(),\n+\t\t\t$self->get_namespace()\n+\t\t);\n+\t}\n+\treturn $self->{'_Path_to_namespace'};\n+}\n+\n+# Take an human readable key, and return file path\n+sub path_to_key {\n+\tmy ($self, $key, $dir_ref) = @_;\n+\n+\tmy @paths = ( $self->path_to_namespace() );\n+\n+\t# Create a unique (hashed) key from human readable key\n+\tmy $filename = md5_hex($key); # or $digester->add($key)->hexdigest()\n+\n+\t# Split filename so that it have DEPTH subdirectories,\n+\t# where each subdirectory has a two-letter name\n+\tpush @paths, unpack(\"(a2)\".($self->{'_Depth'} || 1).\" a*\", $filename);\n+\t$filename = pop @paths;\n+\n+\t# Join paths together, computing dir separately if $dir_ref was passed.\n+\tmy $filepath;\n+\tif (defined $dir_ref && ref($dir_ref)) {\n+\t\tmy $dir = File::Spec->catdir(@paths);\n+\t\t$filepath = File::Spec->catfile($dir, $filename);\n+\t\t$$dir_ref = $dir;\n+\t} else {\n+\t\t$filepath = File::Spec->catfile(@paths, $filename);\n+\t}\n+\n+\treturn $filepath;\n+}\n+\n+# ----------------------------------------------------------------------\n+# worker methods\n+\n+sub fetch {\n+\tmy ($self, $key) = @_;\n+\n+\tmy $file = $self->path_to_key($key);\n+\treturn undef unless (defined $file && -f $file);\n+\n+\t# Fast slurp, adapted from File::Slurp::read, with unnecessary options removed\n+\t# via CHI::Driver::File (from CHI-0.33)\n+\tmy $buf = '';\n+\topen my $read_fh, '<', $file\n+\t\tor return undef;\n+\tbinmode $read_fh, ':raw';\n+\n+\tmy $size_left = -s $read_fh;\n+\n+\twhile ($size_left > 0) {\n+\t\tmy $read_cnt = sysread($read_fh, $buf, $size_left, length($buf));\n+\t\treturn undef unless defined $read_cnt;\n+\n+\t\tlast if $read_cnt == 0;\n+\t\t$size_left -= $read_cnt;\n+\t\t#last if $size_left <= 0;\n+\t}\n+\n+\treturn $buf;\n+}\n+\n+sub store {\n+\tmy ($self, $key, $data) = @_;\n+\n+\tmy $dir;\n+\tmy $file = $self->path_to_key($key, \\$dir);\n+\treturn undef unless (defined $file && defined $dir);\n+\n+\t# ensure that directory leading to cache file exists\n+\tmkpath($dir, 0, 0777) if !-d $dir;\n+\n+\t# generate a temporary file\n+\tmy $temp = File::Temp->new(\n+\t\t#DIR => $dir,\n+\t\tTEMPLATE => \"${file}_XXXXX\",\n+\t\tSUFFIX => \".tmp\"\n+\t);\n+\n+\t# Fast spew, adapted from File::Slurp::write, with unnecessary options removed\n+\t# via CHI::Driver::File (from CHI-0.33)\n+\tmy $write_fh = $temp;\n+\tbinmode $write_fh, ':raw';\n+\n+\tmy $size_left = length($data);\n+\tmy $offset = 0;\n+\n+\twhile ($size_left > 0) {\n+\t\tmy $write_cnt = syswrite($write_fh, $data, $size_left, $offset);\n+\t\treturn undef unless defined $write_cnt;\n+\t\t\n+\t\t$size_left -= $write_cnt;\n+\t\t$offset += $write_cnt; # == length($data);\n+\t}\n+\n+\tclose($temp);\n+\trename($temp, $file);\n+}\n+\n+sub get_size {\n+\tmy ($self, $key) = @_;\n+\n+\tmy $path = $self->path_to_key($key)\n+\t\tor return undef;\n+\tif (-f $path) {\n+\t\treturn -s $path;\n+\t}\n+\treturn 0;\n+}\n+\n+# ......................................................................\n+# interface methods\n+\n+# Removing and expiring\n+\n+sub remove {\n+\tmy ($self, $key) = @_;\n+\n+\tmy $file = $self->path_to_key($key)\n+\t\tor return undef;\n+\tunlink($file);\n+}\n+\n+# Getting and setting\n+\n+sub set {\n+\tmy ($self, $key, $data) = @_;\n+\n+\treturn unless (defined $key && defined $data);\n+\n+\t$self->store($key, $data);\n+}\n+\n+sub get {\n+\tmy ($self, $key) = @_;\n+\n+\tmy $data = $self->fetch($key)\n+\t\tor return undef;\n+\n+\treturn $data;\n+}\n+\n+sub compute {\n+\tmy ($self, $p_key, $p_coderef) = @_;\n+\n+\tmy $data = $self->get($p_key);\n+\tif (!defined $data) {\n+\t\t$data = $p_coderef->($self, $p_key);\n+\t\t$self->set($p_key, $data);\n+\t}\n+\n+\treturn $data;\n+}\n+\n+1;\n+} # end of package GitwebCache::SimpleFileCache;\n+\n+1;\ndiff --git a/t/t9503-gitweb-caching.sh b/t/t9503-gitweb-caching.sh\nnew file mode 100755\nindex 0000000..768080c\n--- /dev/null\n+++ b/t/t9503-gitweb-caching.sh\n@@ -0,0 +1,32 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2010 Jakub Narebski\n+#\n+\n+test_description='caching interface to be used in gitweb'\n+#test_description='caching interface used in gitweb, gitweb caching\n+#\n+#This test checks cache (interface) used in gitweb caching, caching\n+#infrastructure and gitweb response (output) caching (the last by\n+#running gitweb as CGI script from commandline).'\n+\n+# for now we are running only cache interface tests\n+. ./test-lib.sh\n+\n+# this test is present in gitweb-lib.sh\n+if ! test_have_prereq PERL; then\n+\tsay 'perl not available, skipping test'\n+\ttest_done\n+fi\n+\n+\"$PERL_PATH\" -MTest::More -e 0 >/dev/null 2>&1 || {\n+\tsay 'perl module Test::More unavailable, skipping test'\n+\ttest_done\n+}\n+\n+# ----------------------------------------------------------------------\n+\n+test_external 'GitwebCache::* Perl API (in gitweb/cache.pm)' \\\n+\t\"$PERL_PATH\" \"$TEST_DIRECTORY\"/t9503/test_cache_interface.pl\n+\n+test_done\ndiff --git a/t/t9503/test_cache_interface.pl b/t/t9503/test_cache_interface.pl\nnew file mode 100755\nindex 0000000..9714f72\n--- /dev/null\n+++ b/t/t9503/test_cache_interface.pl\n@@ -0,0 +1,84 @@\n+#!/usr/bin/perl\n+use lib (split(/:/, $ENV{GITPERLLIB}));\n+\n+use warnings;\n+use strict;\n+\n+use Test::More;\n+use Data::Dumper;\n+\n+# test source version; there is no installation target for gitweb\n+my $cache_pm = \"$ENV{TEST_DIRECTORY}/../gitweb/cache.pm\";\n+\n+unless (-f \"$cache_pm\") {\n+\tplan skip_all => \"$cache_pm not found\";\n+}\n+\n+# it is currently not a proper Perl module, so we use 'do FILE'\n+# instead of: BEGIN { use_ok('GitwebCache::SimpleFileCache') }\n+my $return = do \"$cache_pm\";\n+ok(!$@, \"parse gitweb/cache.pm\")\n+\tor diag(\"parse error:\\n\", $@);\n+ok(defined $return, \"do gitweb/cache.pm\");\n+ok($return, \"run gitweb/cache.pm\");\n+\n+\n+# Test creating a cache\n+#\n+my $cache = new_ok('GitwebCache::SimpleFileCache',\n+\t[ { 'cache_root' => 'cache', 'cache_depth' => 2 } ]);\n+\n+# Test that default values are defined\n+#\n+ok(defined $GitwebCache::SimpleFileCache::DEFAULT_CACHE_ROOT,\n+\t'$DEFAULT_CACHE_ROOT defined');\n+ok(defined $GitwebCache::SimpleFileCache::DEFAULT_CACHE_DEPTH,\n+\t'$DEFAULT_CACHE_DEPTH defined');\n+\n+# Test accessors and default values for cache\n+#\n+SKIP: {\n+\tskip 'default values not defined', 3\n+\t\tunless ($GitwebCache::SimpleFileCache::DEFAULT_CACHE_ROOT &&\n+\t\t $GitwebCache::SimpleFileCache::DEFAULT_CACHE_DEPTH);\n+\n+\tis($cache->get_namespace(), '', \"default namespace is ''\");\n+\tis($cache->get_root(), $GitwebCache::SimpleFileCache::DEFAULT_CACHE_ROOT,\n+\t\t\"default cache root is '$GitwebCache::SimpleFileCache::DEFAULT_CACHE_ROOT'\");\n+\tcmp_ok($cache->get_depth(), '==', $GitwebCache::SimpleFileCache::DEFAULT_CACHE_DEPTH,\n+\t\t\"default cache depth is $GitwebCache::SimpleFileCache::DEFAULT_CACHE_DEPTH\");\n+}\n+\n+# Test the getting, setting, and removal of a cached value\n+# (Cache::Cache interface)\n+#\n+my $key = 'Test Key';\n+my $value = 'Test Value';\n+can_ok($cache, qw(get set remove));\n+#ok(!defined($cache->get($key)), 'get before set')\n+#\tor diag(\"get returned '\", $cache->get($key), \"' for $key\");\n+$cache->set($key, $value);\n+is($cache->get($key), $value, 'get after set, returns cached value');\n+cmp_ok($cache->get_size($key), '>', 0, 'get_size after set, is greater than 0');\n+$cache->remove($key);\n+ok(!defined($cache->get($key)), 'get after remove, is undefined');\n+eval { $cache->remove('Not-Existent Key'); };\n+ok(!$@, 'remove on non-existent key doesn\\'t die');\n+\n+# Test the getting and setting of a cached value\n+# (CHI interface)\n+#\n+my $call_count = 0;\n+sub get_value {\n+\t$call_count++;\n+\treturn $value;\n+}\n+can_ok($cache, qw(compute));\n+is($cache->compute($key, \\&get_value), $value, 'compute 1st time (set)');\n+is($cache->compute($key, \\&get_value), $value, 'compute 2nd time (get)');\n+is($cache->compute($key, \\&get_value), $value, 'compute 3rd time (get)');\n+cmp_ok($call_count, '==', 1, 'get_value() is called once from compute');\n+\n+done_testing();\n+\n+print Dumper($cache);\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex afd3053..a13bdd5 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -453,6 +453,9 @@ test_external () {\n \t\t# Announce the script to reduce confusion about the\n \t\t# test output that follows.\n \t\tsay_color \"\" \" run $test_count: $descr ($*)\"\n+\t\t# Export TEST_DIRECTORY and TRASH_DIRECTORY\n+\t\t# to be able to use them in script\n+\t\texport TEST_DIRECTORY TRASH_DIRECTORY\n \t\t# Run command; redirect its stderr to &4 as in\n \t\t# test_run_, but keep its stdout on our stdout even in\n \t\t# non-verbose mode.\n-- \n1.6.6.1\n"},{"id":"134016","messageId":"1265711427-15193-4-git-send-email-jnareb@gmail.com","threadId":"22584","inReplyTo":"1265711427-15193-1-git-send-email-jnareb@gmail.com","subject":"[RFC PATCHv2 03/10] gitweb/cache.pm - Stat-based cache expiration","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-09T10:30:20Z","receivedAt":"2010-02-09T10:30:20Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Add stat-based cache expiration to file-based GitwebCache::SimpleFileCache.\nContrary to the way other caching interfaces such as Cache::Cache and CHI\ndo it, the time cache element expires in is _global_ value associated with\ncache instance, and is not local property of cache entry. (Currently cache\nentry does not store any metadata associated with entry... which means that\nthere is no need for serialization / marshaling / freezing and thawing.)\nDefault expire time is -1, which means never expire.\n\nTo check if cache entry is expired, GitwebCache::SimpleFileCache compares\ndifference between mtime (last modify time) of a cache file and current time\nwith (global) time to expire. It is done using CHI-compatible is_valid()\nmethod.\n\nAdd some tests checking that expiring works correctly (on the level of API).\n\nTo be implemented (from original patch by J.H.):\n* actually using this cache in gitweb, except error pages\n* adaptive cache expiration, based on average system load\n* optional locking interface, where only one process can update cache\n (using flock)\n* server-side progress indicator when waiting for filling cache,\n which in turn requires separating situations (like snapshots and\n other non-HTML responses) where we should not show 'please wait'\n message\n\nInspired-by-code-by: John 'Warthog9' Hawley \nSigned-off-by: Jakub Narebski \n---\nNow that caching engine supports cache expiration, we can add caching\nsupport to gitweb.\n\nDifferences from v1:\n* Showing diagnostic if there were parse errors in gitweb/cache.pm\n was moved to previous commit\n* ->get_expires_in() and ->set_expires_in($duration) accessors are generated\n rather than written by hand (reducing repetition of very similar code).\n* Test that value is not expired with expiration time of 1 day, and not \n forever ('expires_in' == -1).\n\nDifferences from relevant parts of J.H. patch:\n* It simply uses stat on last accessed file (checked for existence),\n instead of opening file for reading (without error detection!), running\n stat on it, and then closing it.\n* One can use expire time of -1 (or to be more exact less than 0) to set\n expire time to never (cache is considered fresh forever).\n* There are some tests in t9503 of cache expiration (one of those assume\n that expire time of one day would be not expired in get after set).\n\n gitweb/cache.pm | 34 ++++++++++++++++++++++++++++++++--\n t/t9503/test_cache_interface.pl | 10 ++++++++++\n 2 files changed, 42 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/cache.pm b/gitweb/cache.pm\nindex aebab01..b59509f 100644\n--- a/gitweb/cache.pm\n+++ b/gitweb/cache.pm\n@@ -52,6 +52,10 @@ our $DEFAULT_CACHE_ROOT = \"cache\";\n # The number of subdirectories deep to cache object item. This should be\n # large enough that no cache directory has more than a few hundred objects.\n # Defaults to 1 unless explicitly set.\n+# * 'default_expires_in' (Cache::Cache compatibile),\n+# 'expires_in' (CHI compatibile) [seconds]\n+# The expiration time for objects place in the cache.\n+# Defaults to -1 (never expire) if not explicitly set.\n sub new {\n \tmy ($proto, $p_options_hash_ref) = @_;\n \n@@ -59,7 +63,7 @@ sub new {\n \tmy $self = {};\n \t$self = bless($self, $class);\n \n-\tmy ($root, $depth, $ns);\n+\tmy ($root, $depth, $ns, $expires_in);\n \tif (defined $p_options_hash_ref) {\n \t\t$root =\n \t\t\t$p_options_hash_ref->{'cache_root'} ||\n@@ -68,14 +72,19 @@ sub new {\n \t\t\t$p_options_hash_ref->{'cache_depth'} ||\n \t\t\t$p_options_hash_ref->{'depth'};\n \t\t$ns = $p_options_hash_ref->{'namespace'};\n+\t\t$expires_in =\n+\t\t\t$p_options_hash_ref->{'default_expires_in'} ||\n+\t\t\t$p_options_hash_ref->{'expires_in'};\n \t}\n \t$root = $DEFAULT_CACHE_ROOT unless defined($root);\n \t$depth = $DEFAULT_CACHE_DEPTH unless defined($depth);\n \t$ns = '' unless defined($ns);\n+\t$expires_in = -1 unless defined($expires_in); # <0 means never\n \n \t$self->set_root($root);\n \t$self->set_depth($depth);\n \t$self->set_namespace($ns);\n+\t$self->set_expires_in($expires_in);\n \n \treturn $self;\n }\n@@ -85,7 +94,7 @@ sub new {\n \n # http://perldesignpatterns.com/perldesignpatterns.html#AccessorPattern\n \n-foreach my $i (qw(depth root namespace)) {\n+foreach my $i (qw(depth root namespace expires_in)) {\n \tmy $field = $i;\n \tno strict 'refs';\n \t*{\"get_$field\"} = sub {\n@@ -232,6 +241,26 @@ sub remove {\n \tunlink($file);\n }\n \n+# exists in cache and is not expired\n+sub is_valid {\n+\tmy ($self, $key) = @_;\n+\n+\tmy $path = $self->path_to_key($key);\n+\n+\t# does file exists in cache?\n+\treturn 0 unless -f $path;\n+\n+\t# expire time can be set to never\n+\tmy $expires_in = $self->get_expires_in();\n+\treturn 1 unless (defined $expires_in && $expires_in >= 0);\n+\n+\t# is file expired?\n+\tmy $mtime = (stat(_))[9];\n+\tmy $now = time();\n+\n+\treturn (($now - $mtime) < $expires_in);\n+}\n+\n # Getting and setting\n \n sub set {\n@@ -245,6 +274,7 @@ sub set {\n sub get {\n \tmy ($self, $key) = @_;\n \n+\treturn undef unless $self->is_valid($key);\n \tmy $data = $self->fetch($key)\n \t\tor return undef;\n \ndiff --git a/t/t9503/test_cache_interface.pl b/t/t9503/test_cache_interface.pl\nindex 9714f72..8700b71 100755\n--- a/t/t9503/test_cache_interface.pl\n+++ b/t/t9503/test_cache_interface.pl\n@@ -79,6 +79,16 @@ is($cache->compute($key, \\&get_value), $value, 'compute 2nd time (get)');\n is($cache->compute($key, \\&get_value), $value, 'compute 3rd time (get)');\n cmp_ok($call_count, '==', 1, 'get_value() is called once from compute');\n \n+# Test cache expiration for 'expire now'\n+#\n+$cache->set_expires_in(60*60*24); # set expire time to 1 day\n+cmp_ok($cache->get_expires_in(), '>', 0, '\"expires in\" is greater than 0');\n+is($cache->get($key), $value, 'get returns cached value (not expired)');\n+$cache->set_expires_in(0);\n+is($cache->get_expires_in(), 0, '\"expires in\" is set to now (0)');\n+$cache->set($key, $value);\n+ok(!defined($cache->get($key)), 'cache is expired');\n+\n done_testing();\n \n print Dumper($cache);\n-- \n1.6.6.1\n"},{"id":"134017","messageId":"1265711427-15193-5-git-send-email-jnareb@gmail.com","threadId":"22584","inReplyTo":"1265711427-15193-1-git-send-email-jnareb@gmail.com","subject":"[RFC PATCHv2 04/10] gitweb: Use Cache::Cache compatibile (get, set) output caching","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-09T10:30:21Z","receivedAt":"2010-02-09T10:30:21Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"This commit actually adds output caching to gitweb, as we have now\nminimal features required for it in GitwebCache::SimpleFileCache\n(a 'dumb' but fast file-based cache engine). To enable cache you need\nat least set $caching_enabled to true in gitweb config, and copy cache.pm\nfrom gitweb/ alongside gitweb.cgi - this is described in more detail\nin the new \"Gitweb caching\" section in gitweb/README\n\nCurrently cache support related subroutines in cache.pm (which are\noutside GitwebCache::SimpleFileCache package) are not well separated\nfrom gitweb script itself; cache.pm lacks encapsulation. cache.pm\nassumes that there are href() subroutine and %actions variable, and\nthat there exist $actions{$action} (where $action is parameter passed\nto cache_fetch), and it is a code reference (see also comments in\nt/t9503/test_cache_interface.pl). This is remaining artifact from the\noriginal patch by J.H. (which also had cache_fetch() subroutine).\n\nGitweb itself uses directly only cache_fetch, to get page from cache\nor to generate page and save it to cache, and cache_stop, to be used\nin die_error subroutine, as currently error pages are not cached.\n\nThe cache_fetch subroutine captures output (from STDOUT only, as\nSTDERR is usually logged) using either ->push_layer()/->pop_layer()\nfrom PerlIO::Util submodule (if it is available), or by setting and\nrestoring *STDOUT. Note that only the former could be tested reliably\nto be reliable in t9503 test!\n\nEnabling caching causes the following additional changes to gitweb\noutput:\n* Disables content-type negotiation (choosing between 'text/html'\n mimetype and 'application/xhtml+xml') when caching, as there is no\n content-type negotiation done when retrieving page from cache.\n Use 'text/html' mimetype that can be used by all browsers.\n* Disable timing info (how much time it took to generate original\n page, and how many git commands it took), and in its place show when\n page was originally generated (in GMT / UTC timezone).\n\nAdd basic tests of caching support to t9500-gitweb-standalone-no-errors\ntest: set $caching_enabled to true and check for errors for first time\nrun (generating cache) and second time run (retrieving from cache) for a\nsingle view - summary view for a project.\n\nIf PerlIO::Util is available (see comments), test that cache_fetch\nbehaves correctly, namely that it saves and restores action output in\ncache, and that it prints generated output or cached output.\n\n\nTo be implemented (from original patch by J.H.):\n* adaptive cache expiration, based on average system load\n* optional locking interface, where only one process can update cache\n (using flock)\n* server-side progress indicator when waiting for filling cache,\n which in turn requires separating situations (like snapshots and\n other non-HTML responses) where we should not show 'please wait'\n message\n\nInspired-by-code-by: John 'Warthog9' Hawley \nSigned-off-by: Jakub Narebski \n---\nDifferences from v1:\n* Commit message is present\n\n* When caching is enabled gitweb outputs the time when page was generated in\n place of timing info (time it took to generate page). In previous version\n 'timed' feature wasn't supported (it was disabled) when caching was turned\n on.\n\n The 'timed' feature is turned off when caching is enabled for 'blame_data'\n (used by 'blame_incremental' view).\n\n* Capturing gitweb output is done without need to modify gitweb to always\n print explicitly to $out filehandle. Turning caching off in die_error()\n is now done using cache_stop() subroutine from cache.pm. See also \n description of capturing output below.\n\n* Description of $caching_enabled variable in \"Gitweb config file variables\"\n section in gitweb/README and adding \"Gitweb caching\" section.\n\n* No need for changing $/, _input_ record separator, when printing (output).\n\n* $cache can be now set not only to caching engine object (initialized\n instance), but also to class name. \n\n Pass Cache::Cache ('default_expires_in') and CHI ('root_dir', 'depth')\n compatible aliases for cache options to the $cache->new(), which is\n invoked when $cache is undefined (GitwebCache::SimpleFileCache is used)\n or when it is class name.\n\n* You can override cache options in $GITWEB_CONFIG by modifying\n %cache_options hash. All cache options are described in detail in\n comments.\n\n* Gitweb dies with appropriate error message when caching is enabled, but\n $cache_pm cannot be found. (Note that if the change that automatically\n escapes error message in die_error goes in, this would have to be changed\n to not escape $cache_pm in error message in caller.)\n\n* t/t9500-gitweb-standalone-no-errors.sh tests also the case when data\n is retrieved from cache, not only when it is saved to cache (for\n that expire time is set to never expire).\n\n* t9503 tests also cache_fetch() subroutine, using the same capturing\n mechanism as cache.pm / cache_fetch() itself to check if it produces\n correct output. As noted in comments, I could make tests only for the\n situation where both tests and cache.pm used PerlIO::Util module (not\n among Perl core modules).\n\n The need for mockup of gitweb's href() and %actions shows that\n cache_fetch() is not properly encapsulated / separated from inner workings\n of gitweb code.\n\n\nDifferences from relevant parts of J.H. patch:\n* cache_fetch() subroutine is much, much simpler. Well, it lacks most of\n the features of original cache_fetch() by J.H.: it doesn't have adaptive\n cache lifetime, nor locking to prevent 'stampeding herd' problem, nor\n serving stale data when waiting for cache regeneration, nor background\n data generation, nor activity indicator... but the cache_fetch() itself\n doesn't change much, as those features are added mainly via methods\n and subroutines cache_fetch() calls.\n\n* Capturing gitweb output is done without need to modify gitweb to either\n save generated output into $output variable and print it or save in cache\n after it is generated in full (original J.H. patch in \"Gitweb caching v2\"),\n or changing all print statements to print to explicit filehandle which\n points to STDOUT if caching is disabled and to in-memory file if caching\n is enabled (modified J.H. patch in \"Gitweb caching v5\"). See also below.\n\n* It introduces $cache_pm variable, to be able to test caching in\n t/t9500-gitweb-standalone-no-errors.sh, but also to be able to install\n cache.pm in some other place than along gitweb.cgi. There would be no\n such problems if we used 'require GitwebCache' or somesuch, in place of\n 'do \"cache.pm\"' like in original patch by J.H.\n\n Currently however cache.pm is not independent of gitweb.perl; it requires\n href() subroutine and appropriate %actions hash for dispatch. Therefore\n we must use 'do $cache_pm;' instead of e.g. 'use cache;'.\n\n* Instead of using \"binary\" (sic!) valued $cache_enable (which means 0 or 1\n valued $cache_enable), a set of two variables is used. The $cache\n variable can be used to select alternate caching engine / caching class.\n The $caching_enabled variable is used to actually enable/disable cache.\n\n* The information about the time when page was generated is shown only if\n 'timed' feature is enabled in gitweb config, and it is shown in place of\n usual time it took to generate page (shown when caching is not enabled).\n This means tha change to gitweb/gitweb.css was not needed.\n\n* cache_fetch() is run only when $caching_enabled. Some of cache\n initializations are in gitweb.perl, and not at beginning of cache_fetch()\n\n* Cache options are contained in %cache_options hash, instead of individual\n global variables (which were using non-Perlish camelCase notation).\n\n* There is information about caching in gitweb in gitweb/README\n\n* There are tests of gitweb caching in t9500, and caching API in t9503\n\n\nCapturing gitweb output\n=======================\nWhen output (response) caching is enabled, the caching mechanism has to\ncapture gitweb output (perhaps while printing it to standard output) to\nsave it to cache, unless the data is available in cache and not expired.\n\nBecause die_error uses 'exit', and because it uses git_header_html and\nother printing subroutines (which output has to be captured in other\nsituations), it needs to disable caching, unless we are \"tee\"-ing.\nOtherwise we would get no output from die_error (because it is captured),\nbut also we would not get data to be saved in cache and printed, because\n'exit' in die_error would exit capture, too. This restricts methods and\nmodules that can be used to capture output.\n\nBelow there are presented various considered and used ways of capturing the\noutput, or \"tee\"-ing it (capturing while printing), with their advantages\nand disadvantages.\n\n\nCapturing output (capture)\n~~~~~~~~~~~~~~~~~~~~~~~~~~\n\n1. Modify gitweb to save its output to variable before printing it, which\n means changing 'print ' to '$output .= ', and 'printf '\n to '$output .= sprintf '.\n\n This was the approach taken in last patch in \"Gitweb caching v2\" series\n by J.H. It has the disadvantage of requiring large patch to gitweb;\n it also makes gitweb output data only after it is generated in full,\n even if caching is disabled (data is streamed now). It also requires\n taking care of printing in die_error and after running action, and\n using correct mode (':utf8' or ':raw' aka binary mode) both when printing\n to cache file, and when printing to STDOUT in die_error.\n\n This approach got superceded by the next method.\n\n\n2. Modify gitweb to print to explicit filehandle (instead of implicitly to\n STDOUT), which means changing 'print ' to 'print $out ' (or to\n 'print {$out} ', 'printf ' to 'printf $out ', and last\n 'binmode STDOUT ' to 'binmode $out '.\n\n This was the approach taken in previous version of this series, and also\n in last patch in \"Gitewb caching v5\" by J.H. It has the disadvantage of\n requiring large patch to gitweb, but on the other hand the base change\n itself is just simple mechanical change (plus optionally whitespace-only\n reindent change, and optionally re-wrapping too long lines). It is very\n easy to capture output: just set $out to in-memory file opened for\n reading (or even point $out directly to cache file / temporary file). It\n is also easy to stop capturing: just set $out to \\*STDOUT (or *STDOUT).\n\n Should we chose this method (now that it is shown that you can do without\n changes to gitweb, and what needs then to be done to capture output)?\n\n\n3. The recommended module for capturing output in Perl is Capture::Tiny. It\n has nice and simple API: to capture output you use 'capture \\&code',\n which is prototyped to allow to be called in block form:\n\n ($stdout, $stderr) = capture {\n # your code here ...\n };\n\n It is a minor annoyance that it captures all output also in scalar\n context, i.e. when called as '$stdout = capture \\&code;', so one would\n need to 'print STDERR $stderr;' after capture. But what is more\n important is the fact that it doesn't offer any mechanism to turn off\n capturing inside called code... which is required for die_error, at least\n the way it works now (see above).\n\n Nevertheless the API is clean and I think worth emulating, which I did\n with cache_capture(&) subroutine in cache.pm. Also the problems vanish\n in \"tee\" mode, i.e. for '$stdout = tee \\&code;', see the next subsection.\n \n Capture::Tiny distribution is only 28K packed. There exist\n perl-Capture-Tiny RPM for Fedora:\n http://spreadsheets.google.com/pub?key=tVoSlaYU1SGovwwjV0fqt9Q&output=html\n\n\n4. I have found PerlIO::Util as a way of capturing output when searching the\n way to tee output: PerlIO::Util distribution contains PerlIO::tee module.\n Actually 'scalar' layer to write to in-memory file is implemented in\n PerlIO, which is a core Perl module. Unfortunately PerlIO lacks API to\n add layer which needs parameters. You can open in-memory file using\n \n open my $data_fh, '>', \\$data\n\n you can request using given output layer on output with e.g. '>:utf8'\n as mode, you can add a layer with binmode, e.g. 'binmode $out, \":utf8\";'\n but you can't add layer which requires extra parameter which is not\n a string (like in the case of ':encoding()' layer). So you\n can't just add scalar layer to STDOUT to redirect it to in-memory file\n (to a scalar variable). For that you need PerlIO::Util. \n\n The PerlIO manpage talks about ':pop' pseudo layer, and how \"a more\n elegant (and safer) interface is needed.\" - PerlIO::Util is such\n interface.\n\n To start capturing output to a scalar variable (in-memory file) with\n PerlIO::Util, you just need to\n\n *STDOUT->push_layer(scalar => \\$data);\n\n To stop capturing you do\n\n *STDOUT->pop_layer();\n\n Pushing yet another layer works as it should, therefore my tests of\n cache_fetch() subroutine in t9503 require PerlIO::Util (and do not run if\n it is not present) - I couldn't get to do the test with capture within\n capture using he method described below. (There is Test::Output module\n that could have been used in tests, or I could have used Capture::Tiny.)\n\n I haven't found RPM module for PerlIO::Util (but nowadays I install\n modules from CPAN using local::lib and cpan client).\n\n\n5. Without 'print ' to 'print $out ' change to gitweb, one can try\n manipulating *STDOUT directly, first saving *STDOUT or \\*STDOUT to some\n variable, then setting *STDOUT = $data_fh, where $data_fh is filehandle\n opened to in-memory file.\n\n This seems to work, does not require large patch to gitweb, and does not\n require extra (non-core) Perl module. Nevertheless it seems fragile with\n respect to restoring STDOUT, and even though basic tests (not included)\n of this mechanism producted expected result, I wasn't able to write\n working tests when using this method.\n\n We could probably examine how Capture::Tiny does it, and reuse (copy)\n relevant parts of code, perhaps duplicating STDOUT, closing it and then\n reopening as in-memory filehandle.\n\n YMMV (Your Mileage May Vary).\n\n\nSaving output while printing (tee)\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\nInstead of capturing gitweb output (except die_error, which displays error\nand exists), and then saving it to cache and displaying it, we can capture\n(save) output while it is being displayed, somewhat similar to what tee(1)\nUNIX (coreutils) command does. \n\nThis has the advantage that for the client that generates data (see further\ncommit in series about using locking to avoid cache miss stampede) that it\nserves as acrivity indicator by itself. On the other hand achieving this\neffect in other processes that wait for the data to be generated would be, \nI think, quite complicated (a `tail -f' like behavior).\n\nBecause the original patch by J.H. (both versions) did capturing then\ndisplaying, \"tee\"-ing data was not added to this series. Nevetheless this\nis something to consider for the future.\n\n1. While using 'capture' from Capture::Tiny would have difficulties (if it\n was at all possible) for gitweb as it is now, using 'tee' from the same\n module would be quite simple:\n\n $data = tee {\n # generate gitweb output\n };\n # save $data to cache\n\n die_error would not be a problem in this situation: data would be\n displayed, even if not saved to cache.\n\n2. One of the easiest way of adding output (response) caching to a CGI\n script in Perl is in my opinion using CGI::Cache module. Modifying\n CGI.pm script (like gitweb) to make use of CGI::Cache requires adding\n only a few lines to script source.\n\n While relying on non-core module (which I have not found RPM for) makes\n this solution less feasible, its use of slow Cache::SizeAwareFileCache\n (from Cache::Cache) makes its not good for gitweb. Also CGI::Cache\n requires Tie::Restore to be able to restore ties to filehandles it\n watches... something that gitweb doesn't need.\n\n Nevetheless we can borrow some code from CGI::Cache, replacing cache\n engine with something more efficient. CGI::Cache is GPL licensed.\n\n Internally, the CGI::Cache module ties the output file descriptor\n (usually STDOUT) to an internal variable to which all output is saved.\n And that leads us to the next method.\n\n3. tie filehandle (*STDOUT or $out) so that when we are printing to\n filehandle, it would also save data in a scalar variable (or duplicate it\n to other filehandle). See perltie(1) and Tie::Handle.\n\n4. Multiplexing output stream like tee(1) with PerlIO::tee from PerlIO::Util\n is very simple: to start saving output while it is being printed, just do\n\n *STDOUT->push_layer(tee => \\$data);\n\n The source, beside scalar reference as above, may be a file name, or a\n filehandle. To stop multiplexing, just do\n\n *STDOUT->pop_layer();\n\n I haven't found RPM module for PerlIO::Util (which includes PerlIO::tee).\n\n\n gitweb/README | 70 +++++++++++++++++++++\n gitweb/cache.pm | 78 +++++++++++++++++++++++\n gitweb/gitweb.perl | 106 ++++++++++++++++++++++++++++----\n t/gitweb-lib.sh | 2 +\n t/t9500-gitweb-standalone-no-errors.sh | 19 ++++++\n t/t9503/test_cache_interface.pl | 93 ++++++++++++++++++++++++++++\n 6 files changed, 355 insertions(+), 13 deletions(-)\n\ndiff --git a/gitweb/README b/gitweb/README\nindex 6c2c8e1..53759fc 100644\n--- a/gitweb/README\n+++ b/gitweb/README\n@@ -233,6 +233,11 @@ not include variables usually directly set during build):\n If server load exceed this value then return \"503 Service Unavaliable\" error.\n Server load is taken to be 0 if gitweb cannot determine its value. Set it to\n undefined value to turn it off. The default is 300.\n+ * $caching_enabled\n+ If true, gitweb would use caching to speed up generating response.\n+ Currently supported is only output (response) caching. See \"Gitweb caching\"\n+ section below for details on how to configure and customize caching.\n+ The default is false (caching is disabled).\n \n \n Projects list file format\n@@ -305,6 +310,71 @@ You can use the following files in repository:\n descriptions.\n \n \n+Gitweb caching\n+~~~~~~~~~~~~~~\n+\n+Currently gitweb supports only output (HTTP response) caching, similar\n+to the one used on git.kernel.org. To turn it on, set $caching_enabled\n+variable to true value in gitweb config file, i.e.:\n+\n+ our $caching_enabled = 1;\n+\n+You can choose what caching engine should gitweb use by setting $cache\n+variable either to _initialized_ instance of cache interface, e.g.:\n+\n+ use CHI;\n+ our $cache = CHI->new( driver => 'Memcached',\n+ \tservers => [ \"10.0.0.15:11211\", \"10.0.0.15:11212\" ],\n+ \tl1_cache => { driver => 'FastMmap', root_dir => '/var/cache/gitweb' }\n+ );\n+\n+Alternatively you can set $cache variable to the name of cache class, in\n+which case caching engine should support Cache::Cache or CHI names for cache\n+config (see below), and ignore unrecognized options, e.g.:\n+\n+ use Cache::FileCache;\n+ our $cache = 'Cache::FileCache';\n+\n+Such caching engine should implement (at least) ->get($key) and\n+->set($key, $data) methods (Cache::Cache and CHI compatible interface).\n+\n+If $cache is left unset (if it is left undefined), then gitweb would use\n+GitwebCache::SimpleFileCache from cache.pm as caching engine. This engine\n+is 'dumb' (but fast) file based caching layer, currently without any support\n+for cache size limiting, or even removing expired / grossly expired entries.\n+It has therefore the downside of requiring a huge amount of disk space if\n+there are a number of repositories involved. It is not uncommon for\n+git.kernel.org to have on the order of 80G - 120G accumulate over the course\n+of a few months. It is therefore recommended that the cache directory be\n+periodically completely deleted; this operation is safe to perform.\n+Suggested mechanism (substitute $cachedir for actual path to gitweb cache):\n+\n+ # mv $cachedir $cachedir.flush && mkdir $cachedir && rm -rf $cachedir.flush\n+\n+For gitweb to use caching it must find 'cache.pm' file, which contains\n+GitwebCache::SimpleFileCache and cache-related subroutines, from which\n+cache_fetch and cache_stop are used in gitweb itself. Location of\n+'cache.pm' file is provided in $cache_pm variable; if it is relative path,\n+it is relative to the directory gitweb is run from. Default value of\n+$cache_pm assumes that 'cache.pm' is copied to the same directory as\n+'gitweb.cgi'.\n+\n+Currently 'cache.pm' is not a proper Perl module, because it is not\n+encapsulated / it is not separated from details of gitweb. That is why it\n+is sourced using 'do \"$cache_pm\"', rather than with \"use\" or \"require\"\n+operators.\n+\n+Site-wide cache options are defined in %cache_options hash. Those options\n+apply only when $cache is unset (GitwebCache::SimpleFileCache is used), or\n+if $cache is name of cache class (e.g. $cache = 'Cache::FileCache'). You\n+can override cache options in gitweb config, e.g.:\n+\n+ $cache_options{'expires_in'} = 60; # 60 seconds = 1 minute\n+\n+Please read comments for %cache_options entries in gitweb/gitweb.perl for\n+description of available cache options.\n+\n+\n Webserver configuration\n -----------------------\n \ndiff --git a/gitweb/cache.pm b/gitweb/cache.pm\nindex b59509f..64c333b 100644\n--- a/gitweb/cache.pm\n+++ b/gitweb/cache.pm\n@@ -296,4 +296,82 @@ sub compute {\n 1;\n } # end of package GitwebCache::SimpleFileCache;\n \n+# human readable key identifying gitweb output\n+sub gitweb_output_key {\n+\treturn href(-replay => 1, -full => 1, -path_info => 0);\n+}\n+\n+\n+our $perlio_util = eval { require PerlIO::Util; 1 };\n+our $STDOUT = *STDOUT; #our $STDOUTref = \\*STDOUT;\n+our $data_fh;\n+\n+# Start caching data printed to STDOUT\n+sub cache_start {\n+\tmy $data_ref = shift;\n+\n+\tif ($perlio_util) {\n+\t\t*STDOUT->push_layer(scalar => $data_ref);\n+\n+\t} else {\n+\t\topen $data_fh, '>', $data_ref\n+\t\t\tor die \"Can't open memory file: $!\";\n+\t\t# matches \"binmode STDOUT, ':uft8'\" at beginning\n+\t\tbinmode $data_fh, ':utf8';\n+\t\t*STDOUT = $data_fh;\n+\n+\t}\n+}\n+\n+# Stop caching data (required for die_error)\n+sub cache_stop {\n+\n+\tif ($perlio_util) {\n+\t\t*STDOUT->pop_layer()\n+\t\t\tif ((*STDOUT->get_layers())[-1] eq 'scalar');\n+\n+\t} else {\n+\t\tclose $data_fh\n+\t\t\tor die \"Error closing memory file: $!\";\n+\t\t*STDOUT = $STDOUT;\n+\n+\t}\n+}\n+\n+# Wrap caching data; capture only STDOUT\n+sub cache_capture (&) {\n+\tmy $code = shift;\n+\tmy $data;\n+\n+\tcache_start(\\$data);\n+\t$code->();\n+\tcache_stop();\n+\n+\treturn $data;\n+}\n+\n+sub cache_fetch {\n+\tmy ($cache, $action) = @_;\n+\n+\tmy $key = gitweb_output_key();\n+\tmy $data = $cache->get($key);\n+\n+\tif (defined $data) {\n+\t\t# print cached data\n+\t\tbinmode STDOUT, ':raw';\n+\t\tprint STDOUT $data;\n+\n+\t} else {\n+\t\t$data = cache_capture {\n+\t\t\t$actions{$action}->();\n+\t\t};\n+\n+\t\tif (defined $data) {\n+\t\t\t$cache->set($key, $data);\n+\t\t\tbinmode STDOUT, ':raw';\n+\t\t\tprint STDOUT $data;\n+\t\t}\n+\t}\n+}\n+\n 1;\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 97ea3ec..f02ead9 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -227,6 +227,44 @@ our %avatar_size = (\n # Leave it undefined (or set to 'undef') to turn off load checking.\n our $maxload = 300;\n \n+# This enables/disables the caching layer in gitweb. Currently supported\n+# is only output (response) caching, similar to the one used on git.kernel.org.\n+our $caching_enabled = 0;\n+# Set to _initialized_ instance of cache interface implementing (at least)\n+# get($key) and set($key, $data) methods (Cache::Cache and CHI interfaces).\n+# If unset, GitwebCache::SimpleFileCache would be used, which is 'dumb'\n+# (but fast) file based caching layer, currently without any support for\n+# cache size limiting. It is therefore recommended that the cache directory\n+# be periodically completely deleted; this operation is safe to perform.\n+# Suggested mechanism:\n+# mv $cachedir $cachedir.flush && mkdir $cachedir && rm -rf $cachedir.flush\n+our $cache;\n+# Locations of 'cache.pm' file; if it is relative path, it is relative to\n+# the directory gitweb is run from\n+our $cache_pm = 'cache.pm';\n+# You define site-wide cache options defaults here; override them with\n+# $GITWEB_CONFIG as necessary.\n+our %cache_options = (\n+\t# The location in the filesystem that will hold the root of the cache.\n+\t# This directory will be created as needed (if possible) on the first\n+\t# cache set. Note that either this directory must exists and web server\n+\t# has to have write permissions to it, or web server must be able to\n+\t# create this directory.\n+\t# Possible values:\n+\t# * 'cache' (relative to gitweb),\n+\t# * File::Spec->catdir(File::Spec->tmpdir(), 'gitweb-cache'),\n+\t# * '/var/cache/gitweb' (FHS compliant, requires being set up),\n+\t'cache_root' => 'cache',\n+\t# The number of subdirectories deep to cache object item. This should be\n+\t# large enough that no cache directory has more than a few hundred\n+\t# objects. Each non-leaf directory contains up to 256 subdirectories\n+\t# (00-ff). Must be larger than 0.\n+\t'cache_depth' => 1,\n+\t# The (global) expiration time for objects placed in the cache, in seconds.\n+\t'expires_in' => 20,\n+);\n+\n+\n # You define site-wide feature defaults here; override them with\n # $GITWEB_CONFIG as necessary.\n our %feature = (\n@@ -964,7 +1002,34 @@ if ($action !~ m/^(?:opml|project_list|project_index)$/ &&\n !$project) {\n \tdie_error(400, \"Project needed\");\n }\n-$actions{$action}->();\n+\n+if ($caching_enabled) {\n+\tdie_error(500, 'Caching enabled and \"'.esc_path($cache_pm).'\" not found')\n+\t\tunless -f $cache_pm;\n+\tdo $cache_pm;\n+\tdie $@ if $@;\n+\n+\t# $cache might be initialized (instantiated) cache, i.e. cache object,\n+\t# or it might be name of class, or it might be undefined\n+\tunless (defined $cache && ref($cache)) {\n+\t\t$cache ||= 'GitwebCache::SimpleFileCache';\n+\t\t$cache = $cache->new({\n+\t\t\t%cache_options,\n+\t\t\t#'cache_root' => '/tmp/cache',\n+\t\t\t#'cache_depth' => 2,\n+\t\t\t#'expires_in' => 20, # in seconds (CHI compatibile)\n+\t\t\t# (Cache::Cache compatibile initialization)\n+\t\t\t'default_expires_in' => $cache_options{'expires_in'},\n+\t\t\t# (CHI compatibile initialization)\n+\t\t\t'root_dir' => $cache_options{'cache_root'},\n+\t\t\t'depth' => $cache_options{'cache_depth'},\n+\t\t});\n+\t}\n+\tcache_fetch($cache, $action);\n+} else {\n+\t$actions{$action}->();\n+}\n+\n exit;\n \n ## ======================================================================\n@@ -3169,7 +3234,9 @@ sub git_header_html {\n \t# 'application/xhtml+xml', otherwise send it as plain old 'text/html'.\n \t# we have to do this because MSIE sometimes globs '*/*', pretending to\n \t# support xhtml+xml but choking when it gets what it asked for.\n-\tif (defined $cgi->http('HTTP_ACCEPT') &&\n+\t# Disable content-type negotiation when caching (use mimetype good for all).\n+\tif (!$caching_enabled &&\n+\t defined $cgi->http('HTTP_ACCEPT') &&\n \t $cgi->http('HTTP_ACCEPT') =~ m/(,|;|\\s|^)application\\/xhtml\\+xml(,|;|\\s|$)/ &&\n \t $cgi->Accept('application/xhtml+xml') != 0) {\n \t\t$content_type = 'application/xhtml+xml';\n@@ -3342,17 +3409,25 @@ sub git_footer_html {\n \t}\n \tprint \"\\n\"; # class=\"page_footer\"\n \n-\tif (defined $t0 && gitweb_check_feature('timed')) {\n+\t# timing info doesn't make much sense with output (response) caching,\n+\t# so when caching is enabled gitweb prints the time of page generation\n+\tif ((defined $t0 || $caching_enabled) &&\n+\t gitweb_check_feature('timed')) {\n \t\tprint \"
\\n\";\n-\t\tprint 'This page took '.\n-\t\t ''.\n-\t\t Time::HiRes::tv_interval($t0, [Time::HiRes::gettimeofday()]).\n-\t\t ' seconds '.\n-\t\t ' and '.\n-\t\t ''.\n-\t\t $number_of_git_cmds.\n-\t\t ' git commands '.\n-\t\t \" to generate.\\n\";\n+\t\tif ($caching_enabled) {\n+\t\t\tprint 'This page was generated at '.\n+\t\t\t gmtime( time() ).\" GMT\\n\";\n+\t\t} else {\n+\t\t\tprint 'This page took '.\n+\t\t\t ''.\n+\t\t\t Time::HiRes::tv_interval($t0, [Time::HiRes::gettimeofday()]).\n+\t\t\t ' seconds '.\n+\t\t\t ' and '.\n+\t\t\t ''.\n+\t\t\t $number_of_git_cmds.\n+\t\t\t ' git commands '.\n+\t\t\t \" to generate.\\n\";\n+\t\t}\n \t\tprint \"
\\n\"; # class=\"page_footer\"\n \t}\n \n@@ -3402,6 +3477,10 @@ sub die_error {\n \t\t500 => '500 Internal Server Error',\n \t\t503 => '503 Service Unavailable',\n \t);\n+\n+\t# Do not cache error pages (die_error() uses 'exit')\n+\tcache_stop() if ($caching_enabled);\n+\n \tgit_header_html($http_responses{$status});\n \tprint <\n@@ -5050,7 +5129,8 @@ sub git_blame_common {\n \t\t\tor print \"ERROR $!\\n\";\n \n \t\tprint 'END';\n-\t\tif (defined $t0 && gitweb_check_feature('timed')) {\n+\t\tif (!$caching_enabled &&\n+\t\t defined $t0 && gitweb_check_feature('timed')) {\n \t\t\tprint ' '.\n \t\t\t Time::HiRes::tv_interval($t0, [Time::HiRes::gettimeofday()]).\n \t\t\t ' '.$number_of_git_cmds;\ndiff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh\nindex 5a734b1..b7c2937 100644\n--- a/t/gitweb-lib.sh\n+++ b/t/gitweb-lib.sh\n@@ -27,6 +27,8 @@ our \\$export_ok = '';\n our \\$strict_export = '';\n our \\$maxload = undef;\n \n+our \\$cache_pm = '$TEST_DIRECTORY/../gitweb/cache.pm';\n+\n EOF\n \n \tcat >.git/description <>gitweb_config.perl <<\\EOF\n+$caching_enabled = 1;\n+$cache_options{'expires_in'} = -1; # never expire cache for tests\n+EOF\n+rm -rf cache\n+\n+test_expect_success \\\n+\t'caching enabled (project summary, first run)' \\\n+\t'gitweb_run \"p=.git;a=summary\"'\n+test_debug 'cat gitweb.log'\n+\n+test_expect_success \\\n+\t'caching enabled (project summary, second run)' \\\n+\t'gitweb_run \"p=.git;a=summary\"'\n+test_debug 'cat gitweb.log'\n+\n test_done\ndiff --git a/t/t9503/test_cache_interface.pl b/t/t9503/test_cache_interface.pl\nindex 8700b71..42c49e9 100755\n--- a/t/t9503/test_cache_interface.pl\n+++ b/t/t9503/test_cache_interface.pl\n@@ -7,9 +7,36 @@ use strict;\n use Test::More;\n use Data::Dumper;\n \n+# Modules that could have been used to capture output for testing cache_fetch\n+#use Capture::Tiny;\n+#use Test::Output qw(:stdout);\n+# Modules that could have been used in $cache_pm for cache_fetch\n+#use IO::Capture\n+\n # test source version; there is no installation target for gitweb\n my $cache_pm = \"$ENV{TEST_DIRECTORY}/../gitweb/cache.pm\";\n \n+# ......................................................................\n+\n+# Setup mockup of gitweb's subroutines and variables used in $cache_pm;\n+# must be done before loading $cache_pm. (This probably means that\n+# gitweb.perl and cache.pm are too tightly coupled.)\n+sub href {\n+\treturn 'href';\n+}\n+my $action = 'action';\n+my $fake_action_output = <<'EOF';\n+# This is data to be cached and shown\n+EOF\n+sub fake_action {\n+\tprint $fake_action_output;\n+}\n+our %actions = (\n+\t$action => \\&fake_action,\n+);\n+\n+# ......................................................................\n+\n unless (-f \"$cache_pm\") {\n \tplan skip_all => \"$cache_pm not found\";\n }\n@@ -22,6 +49,7 @@ ok(!$@, \"parse gitweb/cache.pm\")\n ok(defined $return, \"do gitweb/cache.pm\");\n ok($return, \"run gitweb/cache.pm\");\n \n+# ......................................................................\n \n # Test creating a cache\n #\n@@ -88,6 +116,71 @@ $cache->set_expires_in(0);\n is($cache->get_expires_in(), 0, '\"expires in\" is set to now (0)');\n $cache->set($key, $value);\n ok(!defined($cache->get($key)), 'cache is expired');\n+$cache->set_expires_in(-1);\n+\n+# ......................................................................\n+\n+# Prepare for testing cache_fetch from $cache_pm\n+my $test_perlio_util = eval { require PerlIO::Util; 1 };\n+my $cached_output = <<\"EOF\";\n+$fake_action_output# (version recovered from cache)\n+EOF\n+$key = gitweb_output_key();\n+\n+# Catch output printed by cache_fetch\n+# Test all ways of capturing output in cache_fetch\n+our ($perlio_util, $STDOUT);\n+my ($test_data, $test_data_fh, $test_STDOUT);\n+sub capture_cache_fetch_output {\n+\t$test_data = '' if defined $test_data;\n+\n+\tif ($perlio_util) { # or $test_perlio_util\n+\t\t*STDOUT->push_layer(scalar => \\$test_data);\n+\n+\t\tcache_fetch($cache, $action);\n+\n+\t\t*STDOUT->pop_layer();\n+\n+\t} else {\n+\t\tdiag(\"PerlIO::Util not available, not all tests run\");\n+\t\t$test_STDOUT = *STDOUT;\n+\t\topen $test_data_fh, '>', \\$test_data;\n+\t\t$STDOUT = *STDOUT = $test_data_fh; # $STDOUT is from $cache_pm\n+\n+\t\tcache_fetch($cache, $action);\n+\n+\t\t*STDOUT = $test_STDOUT;\n+\n+\t}\n+}\n+\n+\n+# Due to some bad interaction between double capturing, both if second\n+# capture (for this test) is done using PerlIO layers (via PerlIO::Util),\n+# and if it is done using *STDOUT manipulation, tests below do not work if\n+# $perlio_util is false, i.e. if cache_fetch() uses *STDOUT manipulation.\n+# Earlier manual test shown that cache_fetch() *STDOUT manipulation seems\n+# to work all right... but this test would fail when 'if' is replaced by\n+# (currently commented out) 'for'.\n+\n+#for my $use_perlio_util (0..$test_perlio_util) {\n+if ((my $use_perlio_util = $test_perlio_util)) {\n+\t$perlio_util = $use_perlio_util;\n+\tdiag(($perlio_util ? \"Use\" : \"Don't use\").\" PerlIO::Util\");\n+\n+\t# clean state\n+\t$cache->remove($key);\n+\n+\t# first time (if there is no cache) generates cache entry\n+\tcapture_cache_fetch_output();\n+\tis($test_data, $fake_action_output, 'action output is printed (generated)');\n+\tis($cache->get($key), $fake_action_output, 'action output is in cache (generated)');\n+\n+\t# second time (if cache is set/valid) reads from cache\n+\t$cache->set($key, $cached_output);\n+\tcapture_cache_fetch_output();\n+\tis($test_data, $cached_output, 'action output is printed (from cache)');\n+}\n \n done_testing();\n \n-- \n1.6.6.1\n"},{"id":"134020","messageId":"1265711427-15193-6-git-send-email-jnareb@gmail.com","threadId":"22584","inReplyTo":"1265711427-15193-1-git-send-email-jnareb@gmail.com","subject":"[RFC PATCHv2 05/10] gitweb/cache.pm - Adaptive cache expiration time","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-09T10:30:22Z","receivedAt":"2010-02-09T10:30:22Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Add to GitwebCache::SimpleFileCache support for adaptive lifetime\n(cache expiration) control. Cache lifetime can be increased or\ndecreased by any factor, e.g. load average, through the definition\nof the 'check_load' callback.\n\nNote that using ->set_expires_in, or unsetting 'check_load' via\n->set_check_load(undef) turns off adaptive caching.\n\nMake gitweb automatically adjust cache lifetime by load, using\nget_loadavg() function. Define and describe default parameters for\ndynamic (adaptive) cache expiration time control.\n\nThere are some very basic tests of dynamic expiration time in t9503,\nnamely checking if dynamic expire time is within given upper and lower\nbounds.\n\nTo be implemented (from original patch by J.H.):\n* optional locking interface, where only one process can update cache\n (using flock)\n* server-side progress indicator when waiting for filling cache,\n which in turn requires separating situations (like snapshots and\n other non-HTML responses) where we should not show 'please wait'\n message\n\nInspired-by-code-by: John 'Warthog9' Hawley \nSigned-off-by: Jakub Narebski \n---\nDifferences from v1:\n* Proper commit message\n* Adaptive cache expiration can be configured in gitweb config file by\n changing 'expires_min', 'expires_max', 'increase_factor' in\n %cache_options. Those parameters are described in detail.\n* You can turn adaptive cache expiration by setting 'check_load' to undef.\n* By default adaptive cache expiration is turned off; there are extra check\n when to run 'check_load' subroutine.\n* Instead of removing ->set_expires_in(), it just sets 'expires_min' and\n 'expires_max' to the same value, which turns off adaptive expiration.\n* Test assertions for adaptive cache expiration:\n 'expires_min' <= $cache->get_expires_in() <= 'expires_max'\n\nDifferences from relevant parts of J.H. patch:\n* 'increase_factor' is configurable rather than hardcoded\n* 'check_load' is passed via conctructor parameter; gitweb by default sets\n it to \\&get_loadavg. This means that cache.pm is not entangled with\n gitweb, at least for this feature.\n* options are stored in %cache_options, e.g. as 'expires_max' (compatible\n with Cache::Adaptive), and not as global variables with un-Perlish\n camelCase names like $maxCacheTime.\n* API tests\n\nPossible improvements:\n* Turn off adaptive cache lifetime by unsetting check_load, and not only by\n using ->set_expires_in()\n\n gitweb/cache.pm | 55 +++++++++++++++++++++++++++++++++++---\n gitweb/gitweb.perl | 27 +++++++++++++++++-\n t/t9503/test_cache_interface.pl | 22 +++++++++++++++\n 3 files changed, 97 insertions(+), 7 deletions(-)\n\ndiff --git a/gitweb/cache.pm b/gitweb/cache.pm\nindex 64c333b..d81f6e8 100644\n--- a/gitweb/cache.pm\n+++ b/gitweb/cache.pm\n@@ -63,7 +63,8 @@ sub new {\n \tmy $self = {};\n \t$self = bless($self, $class);\n \n-\tmy ($root, $depth, $ns, $expires_in);\n+\tmy ($root, $depth, $ns);\n+\tmy ($expires_min, $expires_max, $increase_factor, $check_load);\n \tif (defined $p_options_hash_ref) {\n \t\t$root =\n \t\t\t$p_options_hash_ref->{'cache_root'} ||\n@@ -72,19 +73,31 @@ sub new {\n \t\t\t$p_options_hash_ref->{'cache_depth'} ||\n \t\t\t$p_options_hash_ref->{'depth'};\n \t\t$ns = $p_options_hash_ref->{'namespace'};\n-\t\t$expires_in =\n+\t\t$expires_min =\n+\t\t\t$p_options_hash_ref->{'expires_min'} ||\n \t\t\t$p_options_hash_ref->{'default_expires_in'} ||\n \t\t\t$p_options_hash_ref->{'expires_in'};\n+\t\t$expires_max =\n+\t\t\t$p_options_hash_ref->{'expires_max'};\n+\t\t$increase_factor = $p_options_hash_ref->{'expires_factor'};\n+\t\t$check_load = $p_options_hash_ref->{'check_load'};\n \t}\n \t$root = $DEFAULT_CACHE_ROOT unless defined($root);\n \t$depth = $DEFAULT_CACHE_DEPTH unless defined($depth);\n \t$ns = '' unless defined($ns);\n-\t$expires_in = -1 unless defined($expires_in); # <0 means never\n+\t$expires_min = 20 unless defined($expires_min);\n+\t$expires_max = $expires_min\n+\t\tif (!defined($expires_max) && exists $p_options_hash_ref->{'expires_in'});\n+\t$expires_max = 1200 unless (defined($expires_max));\n+\t$increase_factor = 60 unless defined($increase_factor);\n \n \t$self->set_root($root);\n \t$self->set_depth($depth);\n \t$self->set_namespace($ns);\n-\t$self->set_expires_in($expires_in);\n+\t$self->set_expires_min($expires_min);\n+\t$self->set_expires_max($expires_max);\n+\t$self->set_increase_factor($increase_factor);\n+\t$self->set_check_load($check_load);\n \n \treturn $self;\n }\n@@ -94,7 +107,7 @@ sub new {\n \n # http://perldesignpatterns.com/perldesignpatterns.html#AccessorPattern\n \n-foreach my $i (qw(depth root namespace expires_in)) {\n+foreach my $i (qw(depth root namespace expires_min expires_max increase_factor check_load)) {\n \tmy $field = $i;\n \tno strict 'refs';\n \t*{\"get_$field\"} = sub {\n@@ -107,6 +120,38 @@ foreach my $i (qw(depth root namespace expires_in)) {\n \t};\n }\n \n+# ......................................................................\n+# pseudo-accessors\n+\n+sub get_expires_in {\n+\tmy ($self) = @_;\n+\n+\t# short-circuit\n+\tif (!defined $self->get_check_load() ||\n+\t $self->get_expires_max() <= $self->get_expires_min()) {\n+\t\treturn $self->get_expires_min();\n+\t}\n+\n+\tmy $expires_in =\n+\t\t#$self->get_expires_min() +\n+\t\t$self->get_increase_factor() * $self->get_check_load()->();\n+\n+\tif ($expires_in < $self->get_expires_min()) {\n+\t\treturn $self->get_expires_min();\n+\t} elsif ($expires_in > $self->get_expires_max()) {\n+\t\treturn $self->get_expires_max();\n+\t}\n+\n+\treturn $expires_in;\n+}\n+\n+sub set_expires_in {\n+\tmy ($self, $duration) = @_;\n+\n+\t$self->set_expires_min($duration);\n+\t$self->set_expires_max($duration);\n+}\n+\n # ----------------------------------------------------------------------\n # utility functions and methods\n \ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex f02ead9..aabed72 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -255,13 +255,36 @@ our %cache_options = (\n \t# * File::Spec->catdir(File::Spec->tmpdir(), 'gitweb-cache'),\n \t# * '/var/cache/gitweb' (FHS compliant, requires being set up),\n \t'cache_root' => 'cache',\n+\n \t# The number of subdirectories deep to cache object item. This should be\n \t# large enough that no cache directory has more than a few hundred\n \t# objects. Each non-leaf directory contains up to 256 subdirectories\n \t# (00-ff). Must be larger than 0.\n \t'cache_depth' => 1,\n-\t# The (global) expiration time for objects placed in the cache, in seconds.\n-\t'expires_in' => 20,\n+\n+\t# The (global) minimum expiration time for objects placed in the cache,\n+\t# in seconds. If the dynamic adaptive cache exporation time is lower\n+\t# than this number, we set cache timeout to this minimum.\n+\t'expires_min' => 20, # 20 seconds\n+\n+\t# The (global) maximum expiration time for dynamic (adaptive) caching\n+\t# algorithm, in seconds. If the adaptive cache lifetime exceeds this\n+\t# number, we set cache timeout to this maximum.\n+\t# (If 'expires_min' >= 'expires_max', there is no adaptive cache timeout,\n+\t# and 'expires_min' is used as expiration time for objects in cache.)\n+\t'expires_max' => 1200, # 20 minutes\n+\n+\t# Cache lifetime will be increased by applying this factor to the result\n+\t# from 'check_load' callback (see below).\n+\t'expires_factor' => 60, # expire time in seconds for 1.0 (100% CPU) load\n+\n+\t# User supplied callback for deciding the cache policy, usually system\n+\t# load. Multiplied by 'expires_factor' gives adaptive expiration time,\n+\t# in seconds, subject to the limits imposed by 'expires_min' and\n+\t# 'expires_max' bounds. Set to undef (or delete) to turn off dynamic\n+\t# lifetime control.\n+\t# (Compatibile with Cache::Adaptive.)\n+\t'check_load' => \\&get_loadavg,\n );\n \n \ndiff --git a/t/t9503/test_cache_interface.pl b/t/t9503/test_cache_interface.pl\nindex 42c49e9..3945adc 100755\n--- a/t/t9503/test_cache_interface.pl\n+++ b/t/t9503/test_cache_interface.pl\n@@ -118,6 +118,28 @@ $cache->set($key, $value);\n ok(!defined($cache->get($key)), 'cache is expired');\n $cache->set_expires_in(-1);\n \n+# Test assertions for adaptive cache expiration\n+#\n+my $load = 0.0;\n+sub load { return $load; }\n+my $expires_min = 10;\n+my $expires_max = 30;\n+$cache->set_expires_min($expires_min);\n+$cache->set_expires_max($expires_max);\n+$cache->set_check_load(\\&load);\n+cmp_ok($cache->get_expires_min(), '==', $expires_min, '\"expires min\" set correctly');\n+cmp_ok($cache->get_expires_max(), '==', $expires_max, '\"expires max\" set correctly');\n+cmp_ok($cache->get_expires_in(), '>=', $expires_min,\n+ '\"expires in\" bound from down for load=0');\n+cmp_ok($cache->get_expires_in(), '<=', $expires_max,\n+ '\"expires in\" bound from up for load=0');\n+$load = 1_000;\n+cmp_ok($cache->get_expires_in(), '>=', $expires_min,\n+ '\"expires in\" bound from down for heavy load');\n+cmp_ok($cache->get_expires_in(), '<=', $expires_max,\n+ '\"expires in\" bound from up for heavy load');\n+$cache->set_expires_in(-1);\n+\n # ......................................................................\n \n # Prepare for testing cache_fetch from $cache_pm\n-- \n1.6.6.1\n"},{"id":"134024","messageId":"1265711427-15193-7-git-send-email-jnareb@gmail.com","threadId":"22584","inReplyTo":"1265711427-15193-1-git-send-email-jnareb@gmail.com","subject":"[RFC PATCHv2 06/10] gitweb: Use CHI compatibile (compute method) caching","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-09T10:30:23Z","receivedAt":"2010-02-09T10:30:23Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"If $cache provides CHI compatible ->compute($key, $code) method, use it\ninstead of Cache::Cache compatible ->get($key) and ->set($key, $data).\nWhile at it, refactor regenerating cache into cache_calculate subroutine.\n\nGitwebCache::SimpleFileCache provides 'compute' method, which currently\nsimply use 'get' and 'set' methods in proscribed manner. Nevertheless\n'compute' method can be more flexible in choosing when to refresh cache,\nand which process is to refresh/(re)generate cache entry. This method\nwould use (advisory) locking to prevent 'cache miss stampede' (aka\n'stampeding herd') problem in the next commit.\n\nThe support for $cache which do not provide '->compute($key, $code)'\nmethod is left just in case we would want to use such (external)\ncaching engine.\n\nSigned-off-by: Jakub Narebski \n---\nThis commit could be moved one commit earlier in this patch series for\nshortlog to look nicer (although commit message would have to be adjusted to\nthat fact) ;-)\n\nDifferences from v1:\n* Proper commit message\n* cache_calculate uses cache_capture and is therefore shorter\n* Don't set $/ to undef, which applies to input, for printing (output).\n\nThis patch doesn't strictly have an equivalent in J.H. patch adding caching\nto gitweb.\n\n gitweb/cache.pm | 39 ++++++++++++++++++++++++++++++++++++---\n 1 files changed, 36 insertions(+), 3 deletions(-)\n\ndiff --git a/gitweb/cache.pm b/gitweb/cache.pm\nindex d81f6e8..899a4b4 100644\n--- a/gitweb/cache.pm\n+++ b/gitweb/cache.pm\n@@ -399,6 +399,41 @@ sub cache_fetch {\n \tmy ($cache, $action) = @_;\n \n \tmy $key = gitweb_output_key();\n+\tif ($cache->can('compute')) {\n+\t\tcache_fetch_compute($cache, $action, $key);\n+\t} else {\n+\t\tcache_fetch_get_set($cache, $action, $key);\n+\t}\n+}\n+\n+# calculate data to regenerate cache\n+sub cache_calculate {\n+\tmy ($action) = @_;\n+\n+\tmy $data = cache_capture {\n+\t\t$actions{$action}->();\n+\t};\n+\n+\treturn $data;\n+}\n+\n+# for $cache which can ->compute($key, $code)\n+sub cache_fetch_compute {\n+\tmy ($cache, $action, $key) = @_;\n+\n+\tmy $data = $cache->compute($key, sub { cache_calculate($action) });\n+\n+\tif (defined $data) {\n+\t\t# print cached data\n+\t\tbinmode STDOUT, ':raw';\n+\t\tprint STDOUT $data;\n+\t}\n+}\n+\n+# for $cache which can ->get($key) and ->set($key, $data)\n+sub cache_fetch_get_set {\n+\tmy ($cache, $action, $key) = @_;\n+\n \tmy $data = $cache->get($key);\n \n \tif (defined $data) {\n@@ -407,9 +442,7 @@ sub cache_fetch {\n \t\tprint STDOUT $data;\n \n \t} else {\n-\t\t$data = cache_capture {\n-\t\t\t$actions{$action}->();\n-\t\t};\n+\t\t$data = cache_calculate($action);\n \n \t\tif (defined $data) {\n \t\t\t$cache->set($key, $data);\n-- \n1.6.6.1\n"},{"id":"134025","messageId":"1265711427-15193-8-git-send-email-jnareb@gmail.com","threadId":"22584","inReplyTo":"1265711427-15193-1-git-send-email-jnareb@gmail.com","subject":"[RFC PATCHv2 07/10] gitweb/cache.pm - Use locking to avoid 'cache miss stampede' problem","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-09T10:30:24Z","receivedAt":"2010-02-09T10:30:24Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"In the ->compute($key, $code) method from GitwebCache::SimpleFileCache,\nuse locking (via flock) to ensure that only one process would generate\ndata to update/fill-in cache; the rest would wait for the cache to\nbe (re)generated and would read data from cache.\n\nCurrently this feature can not be disabled (via %cache_options).\n\n\nA test in t9503 shows that in the case where there are two clients\ntrying to simultaneously access non-existent or stale cache entry,\n(and generating data takes (artifically) a bit of time), if they are\nusing ->compute method the data is (re)generated once, as opposed to\nif those clients are just using ->get/->set methods.\n\nTo be implemented (from original patch by J.H.):\n* background building, and showing stale cache\n* server-side progress indicator when waiting for filling cache,\n which in turn requires separating situations (like snapshots and\n other non-HTML responses) where we should not show 'please wait'\n message\n\nInspired-by-code-by: John 'Warthog9' Hawley \nSigned-off-by: Jakub Narebski \n---\nDifferences from v1:\n* Proper commit message\n* ->get_lockname($key) replaced ->_lockfile_to_key($namespace, $key)\n* mkpath to ensure that path leading to lockfile exists instead of private\n _Make_Path subroutine (this is a bit of code repetition in new code).\n* Testing of removing non-existent key moved to earlier commit\n\nDifferences from relevant parts of J.H. patch:\n* Locking on separate *.lock file, for simpler work and robustness;\n acquiring exclusive lock might require having file opened for possible\n write, following\n \"File Locking Tricks and Traps\" (http://perl.plover.com/yak/flock/)\n J.H. patch used separate *.bg file for lock only for background caching \n (to be implemented in one of next commits).\n* Single variable $lock_state, in place of $lockingStatus, $lockStatBG,\n $lockStat, $lockStatus in J.H. patch.\n* Use indirect filehandles for lockfiles, instead of barewords, i.e. global\n filehandles: \n open my $lock_fh, '+>', $lockfile;\n rather than equivalent of\n open LOCK, '>:utf8', $lockfile\n* Do not use LOCK_UN: closing lockfile would unlock\n* In my opinion much cleaner flow control\n\nPossible improvements:\n* When using locking, only one process would be able to write to temporary\n file. Therefore temporary file can now have non-random name; no need for\n File::Temp nor unique_id (performance). It would probably be best done\n via ->get_tempfile() method, or something like that.\n* Run tests which contain 'sleep 1' in them (which is required to test how\n they handle concurrent access, e.g. testing cache miss stampede situation)\n only when test is run with --long. This would require update to\n t/test-lib.sh to export GIT_TEST_LONG to external program in test_external\n etc.\n* ->get_lockname should perhaps be ->get_lockfile which would return opened\n filehandle, or filehandle + filename like File::Temp::tempfile, or objects\n that functions like filehandle but stringifies to filename like\n File::Temp->new().\n* Equivalent of old _Make_Path?\n* Would ability to turn this feature off make sense?\n\n gitweb/cache.pm | 29 ++++++++++++++++-\n t/t9503/test_cache_interface.pl | 65 +++++++++++++++++++++++++++++++++++++++\n 2 files changed, 92 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/cache.pm b/gitweb/cache.pm\nindex 899a4b4..5c2de7e 100644\n--- a/gitweb/cache.pm\n+++ b/gitweb/cache.pm\n@@ -24,6 +24,7 @@ use File::Path qw(mkpath);\n use File::Spec;\n use File::Temp;\n use Digest::MD5 qw(md5_hex);\n+use Fcntl qw(:flock);\n \n # by default, the cache nests all entries on the filesystem two\n # directories deep\n@@ -195,6 +196,12 @@ sub path_to_key {\n \treturn $filepath;\n }\n \n+sub get_lockname {\n+\tmy $self = shift;\n+\n+\treturn $self->path_to_key(@_) . '.lock';\n+}\n+\n # ----------------------------------------------------------------------\n # worker methods\n \n@@ -330,17 +337,35 @@ sub compute {\n \tmy ($self, $p_key, $p_coderef) = @_;\n \n \tmy $data = $self->get($p_key);\n-\tif (!defined $data) {\n+\treturn $data if defined $data;\n+\n+\tmy $dir;\n+\tmy $lockfile = $self->get_lockname($p_key, \\$dir);\n+\n+\t# ensure that directory leading to lockfile exists\n+\tmkpath($dir, 0, 0777) if !-d $dir;\n+\n+\topen my $lock_fh, '+>', $lockfile;\n+\t#\tor die \"Can't open lockfile '$lockfile': $!\";\n+\tif (my $lock_state = flock($lock_fh, LOCK_EX | LOCK_NB)) {\n+\t\t# acquired writers lock\n \t\t$data = $p_coderef->($self, $p_key);\n \t\t$self->set($p_key, $data);\n+\t} else {\n+\t\t# get readers lock\n+\t\tflock($lock_fh, LOCK_SH);\n+\t\t$data = $self->fetch($p_key);\n \t}\n-\n+\t# closing lockfile releases lock\n+\tclose $lock_fh;\n \treturn $data;\n }\n \n 1;\n } # end of package GitwebCache::SimpleFileCache;\n \n+# ======================================================================\n+\n # human readable key identifying gitweb output\n sub gitweb_output_key {\n \treturn href(-replay => 1, -full => 1, -path_info => 0);\ndiff --git a/t/t9503/test_cache_interface.pl b/t/t9503/test_cache_interface.pl\nindex 3945adc..42fe359 100755\n--- a/t/t9503/test_cache_interface.pl\n+++ b/t/t9503/test_cache_interface.pl\n@@ -107,6 +107,71 @@ is($cache->compute($key, \\&get_value), $value, 'compute 2nd time (get)');\n is($cache->compute($key, \\&get_value), $value, 'compute 3rd time (get)');\n cmp_ok($call_count, '==', 1, 'get_value() is called once from compute');\n \n+# Test 'stampeding herd' / 'cache miss stampede' problem\n+# (probably should be run only if GIT_TEST_LONG)\n+#\n+sub get_value_slow {\n+\t$call_count++;\n+\tsleep 1;\n+\treturn $value;\n+}\n+my ($pid, $kid_fh);\n+\n+$call_count = 0;\n+$cache->remove($key);\n+$pid = open $kid_fh, '-|';\n+SKIP: {\n+\tskip \"cannot fork: $!\", 1\n+\t\tunless defined $pid;\n+\n+\tmy $data = $cache->get($key);\n+\tif (!defined $data) {\n+\t\t$data = get_value_slow();\n+\t\t$cache->set($key, $data);\n+\t}\n+\n+\tif ($pid) {\n+\t\tmy $child_count = <$kid_fh>;\n+\t\tchomp $child_count;\n+\n+\t\twaitpid $pid, 0;\n+\t\tclose $kid_fh;\n+\n+\t\t$call_count += $child_count;\n+\t} else {\n+\t\tprint \"$call_count\\n\";\n+\t\texit 0;\n+\t}\n+\n+\tcmp_ok($call_count, '==', 2, 'parallel get/set: get_value_slow() called twice');\n+}\n+\n+$call_count = 0;\n+$cache->remove($key);\n+$pid = open $kid_fh, '-|';\n+SKIP: {\n+\tskip \"cannot fork: $!\", 1\n+\t\tunless defined $pid;\n+\n+\tmy $data = $cache->compute($key, \\&get_value_slow);\n+\n+\tif ($pid) {\n+\t\tmy $child_count = <$kid_fh>;\n+\t\tchomp $child_count;\n+\n+\t\twaitpid $pid, 0;\n+\t\tclose $kid_fh;\n+\n+\t\t$call_count += $child_count;\n+\t} else {\n+\t\tprint \"$call_count\\n\";\n+\t\texit 0;\n+\t}\n+\n+\tcmp_ok($call_count, '==', 1, 'parallel compute: get_value_slow() called once');\n+}\n+\n+\n # Test cache expiration for 'expire now'\n #\n $cache->set_expires_in(60*60*24); # set expire time to 1 day\n-- \n1.6.6.1\n"},{"id":"134022","messageId":"1265711427-15193-9-git-send-email-jnareb@gmail.com","threadId":"22584","inReplyTo":"1265711427-15193-1-git-send-email-jnareb@gmail.com","subject":"[RFC PATCHv2 08/10] gitweb/cache.pm - Serve stale data when waiting for filling cache","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-09T10:30:25Z","receivedAt":"2010-02-09T10:30:25Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"When process fails to acquire exclusive (writers) lock, then instead\nof waiting for the other process to (re)generate and fill cache, serve\nstale (expired) data from cache. This is of course possible only if\nthere is some stale data in cache for given key.\n\nThis feature of GitwebCache::SimpleFileCache is used only for an\n->update($key, $code) method. It is controlled by 'max_lifetime'\ncache parameter; you can set it to -1 to always serve stale data\nif it exists, and you can set it to 0 (or any value smaller than\n'expires_min') to turn this feature off.\n\nThis feature, as it is implemented currently, makes ->update() method a\nbit assymetric with respect to process that acquired writers lock and\nthose processes that didn't, which can be seen in the new test in t9503.\nThe process that is to regenerate (refresh) data in cache must wait for\nthe data to be generated in full before showing anything to client, while\nthe other processes show stale (expired) data immediately. In order to\nremove or reduce this assymetry gitweb would need to employ one of the two\nalternate solutions. Either data should be (re)generated in background,\nso that process that acquired writers lock would generate data in\nbackground while serving stale data, or alternatively the process that\ngenerates data should pass output to original STDOUT while capturing it\n(\"tee\" otput).\n\nWhen developing this feature, ->is_valid() method acquired additional\nextra optional parameter, where one cap pass expire time instead of using\ncache-wode global expire time.\n\nTo be implemented (from original patch by J.H.):\n* background building,\n* server-side progress indicator when waiting for filling cache,\n which in turn requires separating situations (like snapshots and\n other non-HTML responses) where we should not show 'please wait'\n message\n\nInspired-by-code-by: John 'Warthog9' Hawley \nSigned-off-by: Jakub Narebski \n---\nDifferences from v1:\n* Proper commit messages\n* Both serving stale data (this patch) and background generation (next\n patch) were together in a single patch in previous version of this\n series.\n* There is limit (like in J.H. patch) how stale can cache entry be to be\n eligible to be served while waiting for cache to be refreshed. You can\n set 'max_lifetime' to -1 to always serve stale data if it exists, no\n matter how long ago it was generated (this is the default, like in v1).\n* The above led to modifying ->is_valid() to accept optional parameter\n specifying max age. Note that this max age is not influenced by\n 'check_load' (is not adaptive).\n* Because background cache generation was split in separate patch, for now\n serving stale data is used only for readers (processes which would have to\n wait for refreshing cache).\n* This feature is turned off in tests, and turned on only for testng this\n feature. Extra test that this feature can be turned off, which caused\n refactoring common code.\n\nDifferences from relevant parts of J.H. patch:\n* Instead of using $maxCacheLife, use 'max_lifetime' option (with\n 'max_cache_lifetime' as optional spelling in GitwebCache::SimpleFileCache\n constructor).\n* Use 5*60*60 seconds for 5 hours, rather than 18000 (or 18_000) seconds.\n* Serving stale data was enabled only when background cache regeneration was\n enabled; here it is enabled for readers regardless whether background\n generation (introduced in next commit) is turned on or not. \n\nPossible improvements:\n* Run long tests only with --long (well, they are not _that_ long).\n\n gitweb/cache.pm | 23 ++++++++++----\n gitweb/gitweb.perl | 8 +++++\n t/t9503/test_cache_interface.pl | 63 +++++++++++++++++++++++++++++++++++++-\n 3 files changed, 86 insertions(+), 8 deletions(-)\n\ndiff --git a/gitweb/cache.pm b/gitweb/cache.pm\nindex 5c2de7e..e3bbe57 100644\n--- a/gitweb/cache.pm\n+++ b/gitweb/cache.pm\n@@ -66,6 +66,7 @@ sub new {\n \n \tmy ($root, $depth, $ns);\n \tmy ($expires_min, $expires_max, $increase_factor, $check_load);\n+\tmy ($max_lifetime);\n \tif (defined $p_options_hash_ref) {\n \t\t$root =\n \t\t\t$p_options_hash_ref->{'cache_root'} ||\n@@ -82,6 +83,9 @@ sub new {\n \t\t\t$p_options_hash_ref->{'expires_max'};\n \t\t$increase_factor = $p_options_hash_ref->{'expires_factor'};\n \t\t$check_load = $p_options_hash_ref->{'check_load'};\n+\t\t$max_lifetime =\n+\t\t\t$p_options_hash_ref->{'max_lifetime'} ||\n+\t\t\t$p_options_hash_ref->{'max_cache_lifetime'};\n \t}\n \t$root = $DEFAULT_CACHE_ROOT unless defined($root);\n \t$depth = $DEFAULT_CACHE_DEPTH unless defined($depth);\n@@ -91,12 +95,14 @@ sub new {\n \t\tif (!defined($expires_max) && exists $p_options_hash_ref->{'expires_in'});\n \t$expires_max = 1200 unless (defined($expires_max));\n \t$increase_factor = 60 unless defined($increase_factor);\n+\t$max_lifetime = -1 unless defined($max_lifetime);\n \n \t$self->set_root($root);\n \t$self->set_depth($depth);\n \t$self->set_namespace($ns);\n \t$self->set_expires_min($expires_min);\n \t$self->set_expires_max($expires_max);\n+\t$self->set_max_lifetime($max_lifetime);\n \t$self->set_increase_factor($increase_factor);\n \t$self->set_check_load($check_load);\n \n@@ -108,7 +114,7 @@ sub new {\n \n # http://perldesignpatterns.com/perldesignpatterns.html#AccessorPattern\n \n-foreach my $i (qw(depth root namespace expires_min expires_max increase_factor check_load)) {\n+foreach my $i (qw(depth root namespace expires_min expires_max increase_factor check_load max_lifetime)) {\n \tmy $field = $i;\n \tno strict 'refs';\n \t*{\"get_$field\"} = sub {\n@@ -295,7 +301,7 @@ sub remove {\n \n # exists in cache and is not expired\n sub is_valid {\n-\tmy ($self, $key) = @_;\n+\tmy ($self, $key, $expires_in) = @_;\n \n \tmy $path = $self->path_to_key($key);\n \n@@ -303,7 +309,7 @@ sub is_valid {\n \treturn 0 unless -f $path;\n \n \t# expire time can be set to never\n-\tmy $expires_in = $self->get_expires_in();\n+\t$expires_in = defined $expires_in ? $expires_in : $self->get_expires_in();\n \treturn 1 unless (defined $expires_in && $expires_in >= 0);\n \n \t# is file expired?\n@@ -352,9 +358,14 @@ sub compute {\n \t\t$data = $p_coderef->($self, $p_key);\n \t\t$self->set($p_key, $data);\n \t} else {\n-\t\t# get readers lock\n-\t\tflock($lock_fh, LOCK_SH);\n-\t\t$data = $self->fetch($p_key);\n+\t\t# try to retrieve stale data\n+\t\t$data = $self->fetch($p_key)\n+\t\t\tif $self->is_valid($p_key, $self->get_max_lifetime());\n+\t\tif (!defined $data) {\n+\t\t\t# get readers lock if there is no stale data to serve\n+\t\t\tflock($lock_fh, LOCK_SH);\n+\t\t\t$data = $self->fetch($p_key);\n+\t\t}\n \t}\n \t# closing lockfile releases lock\n \tclose $lock_fh;\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex aabed72..ea0ad25 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -285,6 +285,14 @@ our %cache_options = (\n \t# lifetime control.\n \t# (Compatibile with Cache::Adaptive.)\n \t'check_load' => \\&get_loadavg,\n+\n+\t# Maximum cache file life, in seconds. If cache entry lifetime exceeds\n+\t# this value, it wouldn't be served as being too stale when waiting for\n+\t# cache to be regenerated/refreshed, instead of trying to display\n+\t# existing cache date.\n+\t# Set it to -1 to always serve existing data if it exists,\n+\t# set it to 0 to turn off serving stale data - always wait.\n+\t'max_lifetime' => 5*60*60, # 5 hours\n );\n \n \ndiff --git a/t/t9503/test_cache_interface.pl b/t/t9503/test_cache_interface.pl\nindex 42fe359..922c7cd 100755\n--- a/t/t9503/test_cache_interface.pl\n+++ b/t/t9503/test_cache_interface.pl\n@@ -53,8 +53,11 @@ ok($return, \"run gitweb/cache.pm\");\n \n # Test creating a cache\n #\n-my $cache = new_ok('GitwebCache::SimpleFileCache',\n-\t[ { 'cache_root' => 'cache', 'cache_depth' => 2 } ]);\n+my $cache = new_ok('GitwebCache::SimpleFileCache', [ {\n+\t'cache_root' => 'cache',\n+\t'cache_depth' => 2,\n+\t'max_lifetime' => 0, # turn it off\n+} ]);\n \n # Test that default values are defined\n #\n@@ -269,6 +272,62 @@ if ((my $use_perlio_util = $test_perlio_util)) {\n \tis($test_data, $cached_output, 'action output is printed (from cache)');\n }\n \n+# Test that cache returns stale data in existing but expired cache situation\n+# (probably should be run only if GIT_TEST_LONG)\n+#\n+my $stale_value = 'Stale Value';\n+my $child_data = '';\n+$cache->set($key, $stale_value);\n+$call_count = 0;\n+sub run_compute_forked {\n+\tmy $pid = shift;\n+\n+\tmy $data = $cache->compute($key, \\&get_value_slow);\n+\n+\tif ($pid) {\n+\t\t$child_data = <$kid_fh>;\n+\t\tchomp $child_data;\n+\n+\t\twaitpid $pid, 0;\n+\t\tclose $kid_fh;\n+\t} else {\n+\t\tprint \"$data\\n\";\n+\t\texit 0;\n+\t}\n+\n+\treturn $data;\n+}\n+$cache->set_expires_in(0); # expire now\n+$cache->set_max_lifetime(-1); # forever\n+$pid = open $kid_fh, '-|';\n+SKIP: {\n+\tskip \"cannot fork: $!\", 2\n+\t\tunless defined $pid;\n+\n+\tmy $data = run_compute_forked($pid);\n+\n+\t# returning stale data works\n+\tok($data eq $stale_value || $child_data eq $stale_value,\n+\t 'stale data in at least one process when expired');\n+\n+\t$cache->set_expires_in(-1); # never expire for next ->get\n+\tis($cache->get($key), $value, 'value got set correctly, even if stale data returned');\n+}\n+$cache->set_expires_in(0); # expire now\n+$cache->set_max_lifetime(0); # don't serve stale data\n+$pid = open $kid_fh, '-|';\n+SKIP: {\n+\tskip \"cannot fork: $!\", 1\n+\t\tunless defined $pid;\n+\n+\tmy $data = run_compute_forked($pid);\n+\n+\t# no returning stale data\n+\tok($data ne $stale_value && $child_data ne $stale_value,\n+\t 'configured to never return stale data');\n+}\n+$cache->set_expires_in(-1);\n+\n done_testing();\n \n print Dumper($cache);\n-- \n1.6.6.1\n"},{"id":"134023","messageId":"1265711427-15193-10-git-send-email-jnareb@gmail.com","threadId":"22584","inReplyTo":"1265711427-15193-1-git-send-email-jnareb@gmail.com","subject":"[RFC PATCHv2 09/10] gitweb/cache.pm - Regenerate (refresh) cache in background","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-09T10:30:26Z","receivedAt":"2010-02-09T10:30:26Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"This commit removes assymetry in serving stale data (if it exists)\nwhen regenerating cache in GitwebCache::SimpleFileCache. The process\nthat acquired exclusive (writers) lock, and is therefore selected to\nbe the one that (re)generates data to fill the cache, can now generate\ndata in background, while serving stale data.\n\nThis feature can be enabled or disabled on demand via 'background_cache'\ncache parameter. It is turned on by default.\n\nTo be implemented (from original patch by J.H.):\n* server-side progress indicator when waiting for filling cache,\n which in turn requires separating situations (like snapshots and\n other non-HTML responses) where we should not show 'please wait'\n message\n\nInspired-by-code-by: John 'Warthog9' Hawley \nSigned-off-by: Jakub Narebski \n---\nThis is second part of what was single commit in previous version of\nthis series.\n\nDifferences from v1:\n* Proper commit message\n* You can now configure whether to use background cache (re)generation with\n 'background_cache' parameter (field), which is turned on by default.\n* Simplified code for forking writer.\n* Close lockfile, releasing lock, before waitpid for child.\n* Child (serving stale data) doesn't close lockfile explicitly, although\n that doesn't mean much as lockfile is closed automatically on leaving\n dynamic scope for indirect filehandle $lock_fh (on its desctruction).\n This change perhaps should be reverted.\n\nDifferences from relevant parts of J.H. patch:\n* Fork (run generating process in background) only if there is stale data to\n serve (and if background cache is turned on).\n* In J.H. patch forking was done uncoditionally, only generation or exit\n depended on $backgroundCache.\n* Lock before forking (which I am not sure if is a good idea, but on the\n other hand tests passes, so...)\n* Tests (currently only of API)\n* In my opinion cleaner control flow.\n\n gitweb/cache.pm | 36 +++++++++++++++++++++++++++++-------\n gitweb/gitweb.perl | 9 +++++++++\n t/t9503/test_cache_interface.pl | 14 ++++++++------\n 3 files changed, 46 insertions(+), 13 deletions(-)\n\ndiff --git a/gitweb/cache.pm b/gitweb/cache.pm\nindex e3bbe57..c3a6c26 100644\n--- a/gitweb/cache.pm\n+++ b/gitweb/cache.pm\n@@ -66,7 +66,7 @@ sub new {\n \n \tmy ($root, $depth, $ns);\n \tmy ($expires_min, $expires_max, $increase_factor, $check_load);\n-\tmy ($max_lifetime);\n+\tmy ($max_lifetime, $background_cache);\n \tif (defined $p_options_hash_ref) {\n \t\t$root =\n \t\t\t$p_options_hash_ref->{'cache_root'} ||\n@@ -86,6 +86,7 @@ sub new {\n \t\t$max_lifetime =\n \t\t\t$p_options_hash_ref->{'max_lifetime'} ||\n \t\t\t$p_options_hash_ref->{'max_cache_lifetime'};\n+\t\t$background_cache = $p_options_hash_ref->{'background_cache'};\n \t}\n \t$root = $DEFAULT_CACHE_ROOT unless defined($root);\n \t$depth = $DEFAULT_CACHE_DEPTH unless defined($depth);\n@@ -96,6 +97,7 @@ sub new {\n \t$expires_max = 1200 unless (defined($expires_max));\n \t$increase_factor = 60 unless defined($increase_factor);\n \t$max_lifetime = -1 unless defined($max_lifetime);\n+\t$background_cache = 1 unless defined($background_cache);\n \n \t$self->set_root($root);\n \t$self->set_depth($depth);\n@@ -105,6 +107,7 @@ sub new {\n \t$self->set_max_lifetime($max_lifetime);\n \t$self->set_increase_factor($increase_factor);\n \t$self->set_check_load($check_load);\n+\t$self->set_background_cache($background_cache);\n \n \treturn $self;\n }\n@@ -114,7 +117,9 @@ sub new {\n \n # http://perldesignpatterns.com/perldesignpatterns.html#AccessorPattern\n \n-foreach my $i (qw(depth root namespace expires_min expires_max increase_factor check_load max_lifetime)) {\n+foreach my $i (qw(depth root namespace\n+ expires_min expires_max increase_factor check_load\n+ max_lifetime background_cache)) {\n \tmy $field = $i;\n \tno strict 'refs';\n \t*{\"get_$field\"} = sub {\n@@ -353,14 +358,31 @@ sub compute {\n \n \topen my $lock_fh, '+>', $lockfile;\n \t#\tor die \"Can't open lockfile '$lockfile': $!\";\n+\n+\t# try to retrieve stale data\n+\t$data = $self->fetch($p_key)\n+\t\tif $self->is_valid($p_key, $self->get_max_lifetime());\n+\n \tif (my $lock_state = flock($lock_fh, LOCK_EX | LOCK_NB)) {\n \t\t# acquired writers lock\n-\t\t$data = $p_coderef->($self, $p_key);\n-\t\t$self->set($p_key, $data);\n+\t\tmy $pid = fork() if ($data && $self->get_background_cache());\n+\t\tif (!defined $pid || $pid) {\n+\t\t\t# parent, or didn't fork\n+\t\t\t$data = $p_coderef->($self, $p_key);\n+\t\t\t$self->set($p_key, $data);\n+\n+\t\t\tif ($pid) {\n+\t\t\t\t# releases lock before waiting and exiting\n+\t\t\t\tclose $lock_fh;\n+\t\t\t\t# wait for child (which would print) and exit\n+\t\t\t\twaitpid $pid, 0;\n+\t\t\t\texit 0;\n+\t\t\t}\n+\t\t} else {\n+\t\t\t# child is to serve stale data\n+\t\t\treturn $data;\n+\t\t}\n \t} else {\n-\t\t# try to retrieve stale data\n-\t\t$data = $self->fetch($p_key)\n-\t\t\tif $self->is_valid($p_key, $self->get_max_lifetime());\n \t\tif (!defined $data) {\n \t\t\t# get readers lock if there is no stale data to serve\n \t\t\tflock($lock_fh, LOCK_SH);\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex ea0ad25..8a97e50 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -293,6 +293,15 @@ our %cache_options = (\n \t# Set it to -1 to always serve existing data if it exists,\n \t# set it to 0 to turn off serving stale data - always wait.\n \t'max_lifetime' => 5*60*60, # 5 hours\n+\n+\t# This enables/disables background caching. If it is set to true value,\n+\t# caching engine would return stale data (if it is not older than\n+\t# 'max_lifetime' seconds) if it exists, and launch process if regenerating\n+\t# (refreshing) cache into the background. If it is set to false value,\n+\t# the process that fills cache must always wait for data to be generated.\n+\t# In theory this will make gitweb seem more responsive at the price of\n+\t# serving possibly stale data.\n+\t'background_cache' => 1,\n );\n \n \ndiff --git a/t/t9503/test_cache_interface.pl b/t/t9503/test_cache_interface.pl\nindex 922c7cd..41c7bc3 100755\n--- a/t/t9503/test_cache_interface.pl\n+++ b/t/t9503/test_cache_interface.pl\n@@ -299,32 +299,34 @@ sub run_compute_forked {\n }\n $cache->set_expires_in(0); # expire now\n $cache->set_max_lifetime(-1); # forever\n+ok($cache->get_background_cache(), 'generate cache in background by default');\n $pid = open $kid_fh, '-|';\n SKIP: {\n-\tskip \"cannot fork: $!\", 2\n+\tskip \"cannot fork: $!\", 3\n \t\tunless defined $pid;\n \n \tmy $data = run_compute_forked($pid);\n \n \t# returning stale data works\n-\tok($data eq $stale_value || $child_data eq $stale_value,\n-\t 'stale data in at least one process when expired');\n+\tis($data, $stale_value, 'stale data in parent when expired');\n+\tis($child_data, $stale_value, 'stale data in child when expired');\n \n \t$cache->set_expires_in(-1); # never expire for next ->get\n+\tsleep 1; # wait for background process to have chance to set data\n \tis($cache->get($key), $value, 'value got set correctly, even if stale data returned');\n }\n $cache->set_expires_in(0); # expire now\n $cache->set_max_lifetime(0); # don't serve stale data\n $pid = open $kid_fh, '-|';\n SKIP: {\n-\tskip \"cannot fork: $!\", 1\n+\tskip \"cannot fork: $!\", 2\n \t\tunless defined $pid;\n \n \tmy $data = run_compute_forked($pid);\n \n \t# no returning stale data\n-\tok($data ne $stale_value && $child_data ne $stale_value,\n-\t 'configured to never return stale data');\n+\tisnt($data, $stale_value, 'no stale data in parent if configured');\n+\tisnt($child_data, $stale_value, 'no stale data in child if configured');\n }\n $cache->set_expires_in(-1);\n \n-- \n1.6.6.1\n"},{"id":"134019","messageId":"1265711427-15193-11-git-send-email-jnareb@gmail.com","threadId":"22584","inReplyTo":"1265711427-15193-1-git-send-email-jnareb@gmail.com","subject":"[RFC PATCHv2 10/10] gitweb: Show appropriate \"Generating...\" page when regenerating cache","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-09T10:30:27Z","receivedAt":"2010-02-09T10:30:27Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"When there exist stale/expired (but not too stale) version of\n(re)generated page in cache, gitweb returns stale version (and updates\ncache in background, assuming 'background_cache' is set to true value).\nWhen there is no stale version suitable to serve the client, currently\nwe have to wait for the data to be generated in full before showing it.\nAdd to GitwebCache::SimpleFileCache, via 'generating_info' callback,\nthe ability to show user some activity indicator / progress bar, to\nshow that we are working on generating data.\n\nGitweb itself uses \"Generating...\" page as activity indicator, which\nredirects (via ) to refreshed version\nof the page after the cache is filled (via trick of not closing page\nand therefore not closing connection till data is available in cache,\nchecked by getting shared/readers lock on lockfile for cache entry).\nThe git_generating_data_html() subroutine, which is used by gitweb\nto implement this feature, is highly configurable: you can choose\ninitial delay, frequency of writing some data so that connection\nwon't get closed, and maximum time to wait for data in \"Generating...\"\npage (see %generating_options hash).\n\nCurrently git_generating_data_html() contains hardcoded \"whitelist\" of\nactions for which such HTML \"Generating...\" page makes sense.\n\n\nThis implements final feature from the original gitweb output caching\npatch by J.H.\n\nInspired-by-code-by: John 'Warthog9' Hawley \nSigned-off-by: Jakub Narebski \n---\nDifferences from v1:\n* Proper commit message.\n* get/set_generating_info accessors are now generated.\n* Fixed bug in ->generating_info() method, caught by new tests.\n* You can now turn off \"Generating...\" feature, by unsetting 9setting to\n undef or deleting) of the 'generating_info' key in %cache_options\n* git_generating_data_html() uses whitelist of actions where \"Generating...\"\n page is allowed, instead of blacklist of actions where it should not be\n used.\n* Action check is done using extended regexp rather than comparing with\n strings with 'eq'.\n* git_generating_data_html() must now stop capturing response; with \n 'print $out ' it was easy to ensure printing activity indicator\n to client (to standard output).\n\nDifferences from v1, which are also differences from J.H. patch:\n* \"Generating...\" info behavior can be configured (at least the timings) via\n %generating_options hash, instead of having those options hardcoded.\n* There is initial 'startup_delay' (by default 1 second); if the data is\n generated by that time, the \"Generating...\" page is not shown.\n* Waiting is done using blocking flock + alarm, rather than \"busy wait\"\n loop with non-blocking flock + sleep.\n* Basic test for \"Generating...\" feature (but which didn't caught error\n mentioned below), but only when PerlIO::Util is available.\n\nDifferences from relevant parts of J.H. patch:\n* The subroutine that is responsible for doing \"Generating...\" progress\n info / activity indicator (cacheWaitForUpdate() subroutine in J.H. patch,\n git_generating_data_html() in this patch) is in gitweb.perl, and not in\n cache.pm. It is passed as callback (as code reference) to $cache\n constructor: this method should be used also for other subroutines from\n gitweb which are needed in cache, namely key generation and data\n generation subroutines.\n* gitweb prints generating info in more restricted set of situations; the\n set of actions where gitweb does not generate activity indicator is\n larger. We could probably provide activity indicator also for (possibly)\n non-HTML output, like 'blob_plain' or 'patches', provided that\n 'User-Agent' denotes that we are using web browser.\n* Removed (by accident) copyright assignment from \"Genrating...\" page.\n Needs to be updated and brought back.\n* Fixed typo in DTD URL.\n\nRequired improvements:\n* I have just realized that tests of this feature do test the case when\n ->generating_info does redirect. We need to fork (if enabled) if there\n is stale data to serve OR (what is currently missing) if there is\n ->get_generating_info (and if it is marked as redirecting).\n* Remove accidental \"#'\" needed to fix font-locking (syntax highlighting)\n that got confused in esc_param.\n\nPossible improvements:\n* Ajax-y JavaScript-based activity indicator, without need for http-equiv\n refresh trick. Proof of concept patch of mentioned method was present in\n previous series.\n\n gitweb/cache.pm | 23 +++++-\n gitweb/gitweb.perl | 154 ++++++++++++++++++++++++++++++++++++++-\n t/t9503/test_cache_interface.pl | 45 +++++++++++\n 3 files changed, 216 insertions(+), 6 deletions(-)\n\ndiff --git a/gitweb/cache.pm b/gitweb/cache.pm\nindex c3a6c26..df74ea1 100644\n--- a/gitweb/cache.pm\n+++ b/gitweb/cache.pm\n@@ -66,7 +66,7 @@ sub new {\n \n \tmy ($root, $depth, $ns);\n \tmy ($expires_min, $expires_max, $increase_factor, $check_load);\n-\tmy ($max_lifetime, $background_cache);\n+\tmy ($max_lifetime, $background_cache, $generating_info);\n \tif (defined $p_options_hash_ref) {\n \t\t$root =\n \t\t\t$p_options_hash_ref->{'cache_root'} ||\n@@ -87,6 +87,7 @@ sub new {\n \t\t\t$p_options_hash_ref->{'max_lifetime'} ||\n \t\t\t$p_options_hash_ref->{'max_cache_lifetime'};\n \t\t$background_cache = $p_options_hash_ref->{'background_cache'};\n+\t\t$generating_info = $p_options_hash_ref->{'generating_info'};\n \t}\n \t$root = $DEFAULT_CACHE_ROOT unless defined($root);\n \t$depth = $DEFAULT_CACHE_DEPTH unless defined($depth);\n@@ -108,6 +109,7 @@ sub new {\n \t$self->set_increase_factor($increase_factor);\n \t$self->set_check_load($check_load);\n \t$self->set_background_cache($background_cache);\n+\t$self->set_generating_info($generating_info);\n \n \treturn $self;\n }\n@@ -119,7 +121,7 @@ sub new {\n \n foreach my $i (qw(depth root namespace\n expires_min expires_max increase_factor check_load\n- max_lifetime background_cache)) {\n+ max_lifetime background_cache generating_info)) {\n \tmy $field = $i;\n \tno strict 'refs';\n \t*{\"get_$field\"} = sub {\n@@ -164,6 +166,14 @@ sub set_expires_in {\n \t$self->set_expires_max($duration);\n }\n \n+sub generating_info {\n+\tmy $self = shift;\n+\n+\tif (defined $self->get_generating_info()) {\n+\t\t$self->get_generating_info()->($self, @_);\n+\t}\n+}\n+\n # ----------------------------------------------------------------------\n # utility functions and methods\n \n@@ -367,6 +377,9 @@ sub compute {\n \t\t# acquired writers lock\n \t\tmy $pid = fork() if ($data && $self->get_background_cache());\n \t\tif (!defined $pid || $pid) {\n+\t\t\t# provide \"generating page...\" info if there is no stale data to serve\n+\t\t\t$self->generating_info($p_key, $lock_fh)\n+\t\t\t\tunless ($data);\n \t\t\t# parent, or didn't fork\n \t\t\t$data = $p_coderef->($self, $p_key);\n \t\t\t$self->set($p_key, $data);\n@@ -383,8 +396,12 @@ sub compute {\n \t\t\treturn $data;\n \t\t}\n \t} else {\n+\t\t# some else process is (re)generating cache\n \t\tif (!defined $data) {\n-\t\t\t# get readers lock if there is no stale data to serve\n+\t\t\t# there is no stale data to serve\n+\t\t\t# provide \"generating page...\" info\n+\t\t\t$self->generating_info($p_key, $lock_fh);\n+\t\t\t# get readers lock\n \t\t\tflock($lock_fh, LOCK_SH);\n \t\t\t$data = $self->fetch($p_key);\n \t\t}\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 8a97e50..27c1adf 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -13,7 +13,7 @@ use CGI qw(:standard :escapeHTML -nosticky);\n use CGI::Util qw(unescape);\n use CGI::Carp qw(fatalsToBrowser);\n use Encode;\n-use Fcntl ':mode';\n+use Fcntl qw(:mode :flock);\n use File::Find qw();\n use File::Basename qw(basename);\n binmode STDOUT, ':utf8';\n@@ -302,8 +302,33 @@ our %cache_options = (\n \t# In theory this will make gitweb seem more responsive at the price of\n \t# serving possibly stale data.\n \t'background_cache' => 1,\n-);\n \n+\t# Subroutine which would be called when gitweb has to wait for data to\n+\t# be generated (it can't serve stale data because there isn't any,\n+\t# or if it exists it is older than 'max_lifetime'). The default\n+\t# is to use git_generating_data_html(), which creates \"Generating...\"\n+\t# page, which would then redirect or redraw/rewrite the page when\n+\t# data is ready.\n+\t# Set it to `undef' to disable this feature.\n+\t#\n+\t# Such subroutine (if invoked from GitwebCache::SimpleFileCache)\n+\t# is passed the following parameters: $cache instance, human-readable\n+\t# $key to current page, and filehandle $lock_fh to lockfile.\n+\t'generating_info' => \\&git_generating_data_html,\n+);\n+# You define site-wide options for \"Generating...\" page (if enabled) here;\n+# override them with $GITWEB_CONFIG as necessary.\n+our %generating_options = (\n+\t# The delay before displaying \"Generating...\" page, in seconds. It is\n+\t# intended for \"Generating...\" page to be shown only when really needed.\n+\t'startup_delay' => 1,\n+\t# The time before generating new piece of output, to prevent from\n+\t# redirection before data is ready, in seconds.\n+\t'print_interval' => 2,\n+\t# Maximum time \"Generating...\" page would be present, waiting for data,\n+\t# before unconditional redirect, in seconds.\n+\t'timeout' => $cache_options{'expires_min'},\n+);\n \n # You define site-wide feature defaults here; override them with\n # $GITWEB_CONFIG as necessary.\n@@ -1265,7 +1290,7 @@ sub to_utf8 {\n # correct, but quoted slashes look too horrible in bookmarks\n sub esc_param {\n \tmy $str = shift;\n-\t$str =~ s/([^A-Za-z0-9\\-_.~()\\/:@ ]+)/CGI::escape($1)/eg;\n+\t$str =~ s/([^A-Za-z0-9\\-_.~()\\/:@ ]+)/CGI::escape($1)/eg;#'\n \t$str =~ s/ /\\+/g;\n \treturn $str;\n }\n@@ -3252,6 +3277,129 @@ sub blob_contenttype {\n ## ======================================================================\n ## functions printing HTML: header, footer, error page\n \n+# creates \"Generating...\" page when caching enabled and not in cache\n+sub git_generating_data_html {\n+\tmy ($cache, $key, $lock_fh) = @_;\n+\n+\t# whitelist of actions that should get \"Generating...\" page\n+\tunless ($action =~ /(?:blame(?:|_incremental) | blobdiff | blob |\n+\t commitdiff | commit | forks | heads | tags |\n+\t log | shortlog | history | search |\n+\t tag | tree | summary | project_list)/x) {\n+\t\treturn;\n+\t}\n+\t# blacklist of actions that should not have \"Generating...\" page\n+\t#if ($action =~ /(?:atom | rss | opml |\n+\t# blob_plain | blobdiff_plain | commitdiff_plain |\n+\t# patch | patches |\n+\t# blame_data | search_help | object | project_index |\n+\t# snapshot/x) { # binary\n+\t#\treturn;\n+\t#}\n+\n+\t# Stop capturing response\n+\t#\n+\t# NOTE: this would not be needed if gitweb would do 'print $out',\n+\t# and one could rely on printing to STDOUT to be not captured\n+\tcache_stop(); # or gitweb could use 'print $STDOUT' in place of 'print STDOUT'\n+\n+\t# Initial delay\n+\tif ($generating_options{'startup_delay'} > 0) {\n+\t\teval {\n+\t\t\tlocal $SIG{ALRM} = sub { die \"alarm clock restart\\n\" }; # NB: \\n required\n+\t\t\talarm $generating_options{'startup_delay'};\n+\t\t\tflock($lock_fh, LOCK_SH); # blocking readers lock\n+\t\t\talarm 0;\n+\t\t};\n+\t\tif ($@) {\n+\t\t\t# propagate unexpected errors\n+\t\t\tdie unless $@ !~ /alarm clock restart/;\n+\t\t} else {\n+\t\t\t# we got response within 'startup_delay' timeout\n+\t\t\treturn;\n+\t\t}\n+\t}\n+\n+\tmy $title = \"[Generating...] $site_name\";\n+\t# TODO: the following fragment of code duplicates the one\n+\t# in git_header_html, and it should be refactored.\n+\tif (defined $project) {\n+\t\t$title .= \" - \" . to_utf8($project);\n+\t\tif (defined $action) {\n+\t\t\t$title .= \"/$action\";\n+\t\t\tif (defined $file_name) {\n+\t\t\t\t$title .= \" - \" . esc_path($file_name);\n+\t\t\t\tif ($action eq \"tree\" && $file_name !~ m|/$|) {\n+\t\t\t\t\t$title .= \"/\";\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t}\n+\tmy $mod_perl_version = $ENV{'MOD_PERL'} ? \" $ENV{'MOD_PERL'}\" : '';\n+\n+\t# Use the trick that 'refresh' HTTP header equivalent (set via http-equiv)\n+\t# with timeout of 0 seconds would redirect as soon as page is finished.\n+\t# This \"Generating...\" redirect page should not be cached (externally).\n+\tprint STDOUT $cgi->header(-type => 'text/html', -charset => 'utf-8',\n+\t -status=> '200 OK', -expires => 'now');\n+\tprint STDOUT <<\"EOF\";\n+\n+\n+\n+\n+\n+\n+\n+\n+\n+\n+$title\n+\n+\n+EOF\n+\tprint STDOUT 'Generating...';\n+\n+\tlocal $| = 1; # autoflush\n+\tmy $total_time = 0;\n+\tmy $interval = $generating_options{'print_interval'} || 1;\n+\tmy $timeout = $generating_options{'timeout'};\n+\tmy $alarm_handler = sub {\n+\t\tprint STDOUT '.';\n+\t\t$total_time += $interval;\n+\t\tif ($total_time > $timeout) {\n+\t\t\tdie \"timeout\\n\";\n+\t\t}\n+\t};\n+\teval {\n+\t\t# check if we can use functions from Time::HiRes\n+\t\tif (defined $t0) {\n+\t\t\tlocal $SIG{ALRM} = $alarm_handler;\n+\t\t\tTime::HiRes::alarm($interval, $interval);\n+\t\t} else {\n+\t\t\tlocal $SIG{ALRM} = sub {\n+\t\t\t\t$alarm_handler->();\n+\t\t\t\talarm($interval);\n+\t\t\t};\n+\t\t\talarm($interval);\n+\t\t}\n+\t\tflock($lock_fh, LOCK_SH); # blocking readers lock\n+\t\talarm 0;\n+\t};\n+\t# It doesn't really matter if we got lock, or timed-out\n+\t# but we should re-throw unknown (unexpected) errors\n+\tdie if ($@ and $@ !~ /timeout/);\n+\n+\tprint STDOUT <<\"EOF\";\n+\n+\n+\n+EOF\n+\n+\t#exit 0;\n+\treturn;\n+}\n+\n sub git_header_html {\n \tmy $status = shift || \"200 OK\";\n \tmy $expires = shift;\ndiff --git a/t/t9503/test_cache_interface.pl b/t/t9503/test_cache_interface.pl\nindex 41c7bc3..ac760a2 100755\n--- a/t/t9503/test_cache_interface.pl\n+++ b/t/t9503/test_cache_interface.pl\n@@ -330,6 +330,51 @@ SKIP: {\n }\n $cache->set_expires_in(-1);\n \n+# Test 'generating_info' feature\n+#\n+$cache->remove($key);\n+my $progress_info = \"Generating...\";\n+sub test_generating_info {\n+\tlocal $| = 1;\n+\tprint \"$progress_info\";\n+}\n+$cache->set_generating_info(\\&test_generating_info);\n+# NOTE: I coulnd't be bothered to rewrite those tests to work without PerlIO::Util\n+$pid = open $kid_fh, '-|' if $perlio_util;\n+my ($data, $output, $child_output);\n+SKIP: {\n+\tskip \"No PerlIO::Util\", 2\n+\t\tunless $perlio_util;\n+\tskip \"cannot fork: $!\", 2\n+\t\tunless defined $pid;\n+\n+\t*STDOUT->push_layer(scalar => \\$output);\n+\t$data = $cache->compute($key, \\&get_value_slow);\n+\t*STDOUT->pop_layer();\n+\n+\tif ($pid) {\n+\t\tlocal $/ = \"\\0\";\n+\t\tchomp($child_output = <$kid_fh>);\n+\t\tchomp($child_data = <$kid_fh>);\n+\n+\t\twaitpid $pid, 0;\n+\t\tclose $kid_fh;\n+\t} else {\n+\t\tlocal $| = 1;\n+\t\tprint \"$output\\0$data\\0\";\n+\t\texit 0;\n+\t}\n+\n+\tis(\"$output$data\", \"$progress_info$value\",\n+\t 'progress info and data from parent');\n+\tis(\"$child_output$child_data\", \"$progress_info$value\",\n+\t 'progress info and data from child');\n+}\n+diag(\"parent output is $output\");\n+diag(\"parent data is $data\");\n+diag(\"child output is $child_output\");\n+diag(\"child data is $child_data\");\n+\n done_testing();\n \n print Dumper($cache);\n-- \n1.6.6.1\n"},{"id":"134082","messageId":"201002092323.24832.jnareb@gmail.com","threadId":"22584","inReplyTo":"1265711427-15193-10-git-send-email-jnareb@gmail.com","subject":"Re: [RFC PATCHv2 09/10] gitweb/cache.pm - Regenerate (refresh) cache in background","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-09T22:23:17Z","receivedAt":"2010-02-09T22:23:17Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Tue, 9 Feb 2010, Jakub Narebski wrote:\n\n[...]\n> +               my $pid = fork() if ($data && $self->get_background_cache());\n> +               if (!defined $pid || $pid) {\n> +                       # parent, or didn't fork\n> +                       $data = $p_coderef->($self, $p_key);\n> +                       $self->set($p_key, $data);\n> +\n> +                       if ($pid) {\n> +                               # releases lock before waiting and exiting\n> +                               close $lock_fh;\n> +                               # wait for child (which would print) and exit\n> +                               waitpid $pid, 0;\n> +                               exit 0;\n> +                       }\n> +               } else {\n> +                       # child is to serve stale data\n> +                       return $data;\n> +               }\n[...]\n\nI have decide to rely on $SIG{CHLD} = 'IGNORE' to not generate zombies,\nand consistently exit in child (in forked process) and return in parent.\nThis change would be in next version of this series.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"134100","messageId":"201002100212.26157.jnareb@gmail.com","threadId":"22584","inReplyTo":"1265711427-15193-5-git-send-email-jnareb@gmail.com","subject":"Re: [RFC PATCHv2 04/10] gitweb: Use Cache::Cache compatibile (get, set) output caching","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-10T01:12:24Z","receivedAt":"2010-02-10T01:12:24Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Tue, 9 Feb 2010 at 11:30 +0100, Jakub Narebski wrote:\n\n> The cache_fetch subroutine captures output (from STDOUT only, as\n> STDERR is usually logged) using either ->push_layer()/->pop_layer()\n> from PerlIO::Util submodule (if it is available), or by setting and\n> restoring *STDOUT. Note that only the former could be tested reliably\n> to be reliable in t9503 test!\n\nScratch that, I have just checked that (at least for Apache + mod_cgi,\nbut I don't think that it matters) the latter solution, with setting\nand restoring *STDOUT doesn't work: I would get data in cache (so it\ncan be restored later), but instead of output I would get Internal Server\nError (\"The server encountered an internal error or misconfiguration and\nwas unable to complete your request.\") without even a hint what the\nproblem was. Sprinkling \"die ...: $!\" didn't help to catch this error:\nI suspect that the problem is with capturing.\n\nSo we either would have to live with non-core PerlIO::Util or (pure Perl)\nCapture::Tiny, or do the 'print -> print $out' patch...\n\n[....]\n> Capturing gitweb output\n> =======================\n> When output (response) caching is enabled, the caching mechanism has to\n> capture gitweb output (perhaps while printing it to standard output) to\n> save it to cache, unless the data is available in cache and not expired.\n> \n> Because die_error uses 'exit', and because it uses git_header_html and\n> other printing subroutines (which output has to be captured in other\n> situations), it needs to disable caching, unless we are \"tee\"-ing.\n> Otherwise we would get no output from die_error (because it is captured),\n> but also we would not get data to be saved in cache and printed, because\n> 'exit' in die_error would exit capture, too. This restricts methods and\n> modules that can be used to capture output.\n> \n> Below there are presented various considered and used ways of capturing the\n> output, or \"tee\"-ing it (capturing while printing), with their advantages\n> and disadvantages.\n> \n> \n> Capturing output (capture)\n> ~~~~~~~~~~~~~~~~~~~~~~~~~~\n[...]\n\n> 5. Without 'print ' to 'print $out ' change to gitweb, one can try\n> manipulating *STDOUT directly, first saving *STDOUT or \\*STDOUT to some\n> variable, then setting *STDOUT = $data_fh, where $data_fh is filehandle\n> opened to in-memory file.\n> \n> This seems to work, does not require large patch to gitweb, and does not\n> require extra (non-core) Perl module. Nevertheless it seems fragile with\n> respect to restoring STDOUT, and even though basic tests (not included)\n> of this mechanism producted expected result, I wasn't able to write\n> working tests when using this method.\n> \n> We could probably examine how Capture::Tiny does it, and reuse (copy)\n> relevant parts of code, perhaps duplicating STDOUT, closing it and then\n> reopening as in-memory filehandle.\n> \n> YMMV (Your Mileage May Vary).\n\n-- \nJakub Narebski\nPoland\n"},{"id":"134101","messageId":"20100210012343.GS9553@machine.or.cz","threadId":"22584","inReplyTo":"201002100212.26157.jnareb@gmail.com","subject":"Re: [RFC PATCHv2 04/10] gitweb: Use Cache::Cache compatibile (get, set) output caching","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2010-02-10T01:23:43Z","receivedAt":"2010-02-10T01:23:43Z","isPatch":false,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Wed, Feb 10, 2010 at 02:12:24AM +0100, Jakub Narebski wrote:\n> On Tue, 9 Feb 2010 at 11:30 +0100, Jakub Narebski wrote:\n> \n> > The cache_fetch subroutine captures output (from STDOUT only, as\n> > STDERR is usually logged) using either ->push_layer()/->pop_layer()\n> > from PerlIO::Util submodule (if it is available), or by setting and\n> > restoring *STDOUT. Note that only the former could be tested reliably\n> > to be reliable in t9503 test!\n> \n> Scratch that, I have just checked that (at least for Apache + mod_cgi,\n> but I don't think that it matters) the latter solution, with setting\n> and restoring *STDOUT doesn't work: I would get data in cache (so it\n> can be restored later), but instead of output I would get Internal Server\n> Error (\"The server encountered an internal error or misconfiguration and\n> was unable to complete your request.\") without even a hint what the\n> problem was. Sprinkling \"die ...: $!\" didn't help to catch this error:\n> I suspect that the problem is with capturing.\n> \n> So we either would have to live with non-core PerlIO::Util or (pure Perl)\n> Capture::Tiny, or do the 'print -> print $out' patch...\n\nAll the magic methods seem to be troublesome, but in that case I'd\nreally prefer a level of indirection instead of filehandle - as is,\n'print (...) -> output (...)' ins. of 'print (...) -> print $out (...)'\n(or whatever). That should be really flexible and completely\nfutureproof, and I don't think the level of indirection would incur any\nmeasurable overhead, would it?\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nIf you can't see the value in jet powered ants you should turn in\nyour nerd card. -- Dunbal (464142)\n"},{"id":"134139","messageId":"201002101228.15732.jnareb@gmail.com","threadId":"22584","inReplyTo":"20100210012343.GS9553@machine.or.cz","subject":"Re: [RFC PATCHv2 04/10] gitweb: Use Cache::Cache compatibile (get, set) output caching","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-10T11:28:14Z","receivedAt":"2010-02-10T11:28:14Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Wed, 10 Feb 2010, Petr Baudis wrote:\n> On Wed, Feb 10, 2010 at 02:12:24AM +0100, Jakub Narebski wrote:\n> > On Tue, 9 Feb 2010 at 11:30 +0100, Jakub Narebski wrote:\n> > \n> > > The cache_fetch subroutine captures output (from STDOUT only, as\n> > > STDERR is usually logged) using either ->push_layer()/->pop_layer()\n> > > from PerlIO::Util submodule (if it is available), or by setting and\n> > > restoring *STDOUT. Note that only the former could be tested reliably\n> > > to be reliable in t9503 test!\n> > \n> > Scratch that, I have just checked that (at least for Apache + mod_cgi,\n> > but I don't think that it matters) the latter solution, with setting\n> > and restoring *STDOUT doesn't work: I would get data in cache (so it\n> > can be restored later), but instead of output I would get Internal Server\n> > Error (\"The server encountered an internal error or misconfiguration and\n> > was unable to complete your request.\") without even a hint what the\n> > problem was. Sprinkling \"die ...: $!\" didn't help to catch this error:\n> > I suspect that the problem is with capturing.\n> > \n> > So we either would have to live with non-core PerlIO::Util or (pure Perl)\n> > Capture::Tiny, or do the 'print -> print $out' patch...\n> \n> All the magic methods seem to be troublesome, but in that case I'd\n> really prefer a level of indirection instead of filehandle - as is,\n> 'print (...) -> output (...)' ins. of 'print (...) -> print $out (...)'\n> (or whatever). That should be really flexible and completely\n> futureproof, and I don't think the level of indirection would incur any\n> measurable overhead, would it?\n\nFirst, it is not only 'print (...) -> print $out (...)'; you need to\ndo all those:\n\n print -> print $out \n printf -> printf $out \n binmode STDOUT, -> binmode $out, \n\nSecond, using \"tie\" on filehandle (on *STDOUT) can be used also for \njust capturing output, not only for \"tee\"-ing; what's more to print\nwhile capturing one has to do extra work. It is quite similar to\nreplacing 'print (...)' with 'output (...)' etc., but using\ntie/untie doesn't require large patch to gitweb.\n\nThird, as you can see below tie-ing is about 1% slower than using\n'output (...)', which in turn is less than 10% slower than explicit\nfilehandle solution i.e. 'print $out (...)'... and is almost twice\nslower than solution using PerlIO::Util\n\nBenchmark: timing 50000 iterations of output, perlio, print \\$out, tie *STDOUT...\n output: 1.81462 wallclock secs ( 1.77 usr + 0.00 sys = 1.77 CPU) @ 28248.59/s (n=50000)\n perlio: 1.05585 wallclock secs ( 1.03 usr + 0.00 sys = 1.03 CPU) @ 48543.69/s (n=50000)\nprint \\$out: 1.70027 wallclock secs ( 1.66 usr + 0.00 sys = 1.66 CPU) @ 30120.48/s (n=50000)\ntie *STDOUT: 1.82248 wallclock secs ( 1.79 usr + 0.00 sys = 1.79 CPU) @ 27932.96/s (n=50000)\n Rate tie *STDOUT output print \\$out perlio\ntie *STDOUT 27933/s -- -1% -7% -42%\noutput 28249/s 1% -- -6% -42%\nprint \\$out 30120/s 8% 7% -- -38%\nperlio 48544/s 74% 72% 61% --\n\nBenchmark: running output, perlio, print \\$out, tie *STDOUT for at least 10 CPU seconds...\n output: 10.7199 wallclock secs (10.53 usr + 0.00 sys = 10.53 CPU) @ 28029.63/s (n=295152)\n perlio: 11.2884 wallclock secs (10.46 usr + 0.00 sys = 10.46 CPU) @ 49967.11/s (n=522656)\nprint \\$out: 10.5978 wallclock secs (10.43 usr + 0.00 sys = 10.43 CPU) @ 30318.79/s (n=316225)\ntie *STDOUT: 11.3525 wallclock secs (10.68 usr + 0.00 sys = 10.68 CPU) @ 27635.96/s (n=295152)\n Rate tie *STDOUT output print \\$out perlio\ntie *STDOUT 27636/s -- -1% -9% -45%\noutput 28030/s 1% -- -8% -44%\nprint \\$out 30319/s 10% 8% -- -39%\nperlio 49967/s 81% 78% 65% --\nneed\n\nAttached there is script that was used to produce those results.\n-- \nJakub Narebski\nPoland\n\n\n#!/usr/bin/perl\n\nuse strict;\nuse warnings;\n\nuse Test::More;\nuse Time::HiRes;\nuse Benchmark qw(:all :hireswallclock);\nuse Data::Dumper;\n\nuse PerlIO::Util;\n\nmy $chunk = \"test\\n\";\n\nmy $lorem = <<'EOF';\nLorem ipsum dolor sit amet, consectetur adipisicing elit, sed do\neiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad\nminim veniam, quis nostrud exercitation ullamco laboris nisi ut\naliquip ex ea commodo consequat. Duis aute irure dolor in\nreprehenderit in voluptate velit esse cillum dolore eu fugiat nulla\npariatur. Excepteur sint occaecat cupidatat non proident, sunt in\nculpa qui officia deserunt mollit anim id est laborum.\n\nSed ut perspiciatis unde omnis iste natus error sit voluptatem\naccusantium doloremque laudantium, totam rem aperiam, eaque ipsa quae\nab illo inventore veritatis et quasi architecto beatae vitae dicta\nsunt explicabo. Nemo enim ipsam voluptatem quia voluptas sit\naspernatur aut odit aut fugit, sed quia consequuntur magni dolores eos\nqui ratione voluptatem sequi nesciunt. Neque porro quisquam est, qui\ndolorem ipsum quia dolor sit amet, consectetur, adipisci velit, sed\nquia non numquam eius modi tempora incidunt ut labore et dolore magnam\naliquam quaerat voluptatem. Ut enim ad minima veniam, quis nostrum\nexercitationem ullam corporis suscipit laboriosam, nisi ut aliquid ex\nea commodi consequatur? Quis autem vel eum iure reprehenderit qui in\nea voluptate velit esse quam nihil molestiae consequatur, vel illum\nqui dolorem eum fugiat quo voluptas nulla pariatur?\nEOF\n\nsub capture_perlio {\n\tmy $data = '';\n\n\t*STDOUT->push_layer(scalar => \\$data);\n\n\tprint $chunk;\n\n\t*STDOUT->pop_layer();\n\n\treturn $data;\n}\n\nmy $out = \\*STDOUT;\n\nsub capture_filehandle {\n\tmy $data = '';\n\n\topen my $data_fh, '>', \\$data;\n\t$out = $data_fh;\n\n\tprint $out $chunk;\n\n\t$out = \\*STDOUT;\n\tclose $data_fh;\n\n\treturn $data;\n}\n\nsub output {\n\tprint $out @_;\n}\n\nsub capture_indirection {\n\tmy $data = '';\n\n\topen my $data_fh, '>', \\$data;\n\t$out = $data_fh;\n\n\toutput $chunk;\n\n\t$out = \\*STDOUT;\n\tclose $data_fh;\n\n\treturn $data;\n}\n\nsub capture_tied {\n\tmy $data = '';\n\n\ttie *STDOUT, 'CatchSTDOUT', \\$data;\n\n\tprint $chunk;\n\n\tuntie *STDOUT;\n\n\treturn $data;\n}\n\n{\n\tpackage CatchSTDOUT;\n\n\tuse strict;\n\tuse warnings;\n\tuse Data::Dumper;\n\n\tsub TIEHANDLE {\n\t\tmy ($proto, $dataref) = @_;\n\t\tmy $class = ref($proto) || $proto;\n\n\t\tmy $self = {};\n\t\t$self = bless($self, $class);\n\t\t$self->{'scalar'} = $dataref;\n\t\treturn $self;\n\t}\n\n\tsub PRINT {\n\t\tmy $self = shift;\n\t\t${$self->{'scalar'}} .= join('',@_);\n\t}\n}\n\n# ----------------------------------------------------------------------\n\nprint 'capture_perlio = '.capture_perlio();\nprint 'capture_filehandle = '.capture_filehandle();\nprint 'capture_indirection = '.capture_indirection();\nprint 'capture_tied = '.capture_tied();\n\n$chunk = \"another test\\n\";\nprint 'capture_perlio = '.capture_perlio();\nprint 'capture_filehandle = '.capture_filehandle();\nprint 'capture_indirection = '.capture_indirection();\nprint 'capture_tied = '.capture_tied();\n\nprint \"\\n\";\n\n\n$chunk = $lorem;\n\nmy $result = timethese(50_000, {\n\t'perlio' => \\&capture_perlio,\n\t'print \\$out' => \\&capture_filehandle,\n\t'output' => \\&capture_indirection,\n\t'tie *STDOUT' => \\&capture_indirection,\n});\ncmpthese($result);\nprint \"\\n\";\n\n$result = timethese(-10, {\n\t'perlio' => \\&capture_perlio,\n\t'print \\$out' => \\&capture_filehandle,\n\t'output' => \\&capture_indirection,\n\t'tie *STDOUT' => \\&capture_indirection,\n});\ncmpthese($result);\nprint \"\\n\";\n"},{"id":"134140","messageId":"20100210120257.GP4159@machine.or.cz","threadId":"22584","inReplyTo":"201002101228.15732.jnareb@gmail.com","subject":"Re: [RFC PATCHv2 04/10] gitweb: Use Cache::Cache compatibile (get, set) output caching","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2010-02-10T12:02:57Z","receivedAt":"2010-02-10T12:02:57Z","isPatch":false,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Wed, Feb 10, 2010 at 12:28:14PM +0100, Jakub Narebski wrote:\n> On Wed, 10 Feb 2010, Petr Baudis wrote:\n> > On Wed, Feb 10, 2010 at 02:12:24AM +0100, Jakub Narebski wrote:\n> > > On Tue, 9 Feb 2010 at 11:30 +0100, Jakub Narebski wrote:\n> > > \n> > > > The cache_fetch subroutine captures output (from STDOUT only, as\n> > > > STDERR is usually logged) using either ->push_layer()/->pop_layer()\n> > > > from PerlIO::Util submodule (if it is available), or by setting and\n> > > > restoring *STDOUT. Note that only the former could be tested reliably\n> > > > to be reliable in t9503 test!\n> > > \n> > > Scratch that, I have just checked that (at least for Apache + mod_cgi,\n> > > but I don't think that it matters) the latter solution, with setting\n> > > and restoring *STDOUT doesn't work: I would get data in cache (so it\n> > > can be restored later), but instead of output I would get Internal Server\n> > > Error (\"The server encountered an internal error or misconfiguration and\n> > > was unable to complete your request.\") without even a hint what the\n> > > problem was. Sprinkling \"die ...: $!\" didn't help to catch this error:\n> > > I suspect that the problem is with capturing.\n> > > \n> > > So we either would have to live with non-core PerlIO::Util or (pure Perl)\n> > > Capture::Tiny, or do the 'print -> print $out' patch...\n> > \n> > All the magic methods seem to be troublesome, but in that case I'd\n> > really prefer a level of indirection instead of filehandle - as is,\n> > 'print (...) -> output (...)' ins. of 'print (...) -> print $out (...)'\n> > (or whatever). That should be really flexible and completely\n> > futureproof, and I don't think the level of indirection would incur any\n> > measurable overhead, would it?\n> \n> First, it is not only 'print (...) -> print $out (...)'; you need to\n> do all those:\n> \n> print -> print $out \n> printf -> printf $out \n> binmode STDOUT, -> binmode $out, \n> \n> Second, using \"tie\" on filehandle (on *STDOUT) can be used also for \n> just capturing output, not only for \"tee\"-ing; what's more to print\n> while capturing one has to do extra work. It is quite similar to\n> replacing 'print (...)' with 'output (...)' etc., but using\n> tie/untie doesn't require large patch to gitweb.\n> \n> Third, as you can see below tie-ing is about 1% slower than using\n> 'output (...)', which in turn is less than 10% slower than explicit\n> filehandle solution i.e. 'print $out (...)'... and is almost twice\n> slower than solution using PerlIO::Util\n> \n> Benchmark: timing 50000 iterations of output, perlio, print \\$out, tie *STDOUT...\n> output: 1.81462 wallclock secs ( 1.77 usr + 0.00 sys = 1.77 CPU) @ 28248.59/s (n=50000)\n> perlio: 1.05585 wallclock secs ( 1.03 usr + 0.00 sys = 1.03 CPU) @ 48543.69/s (n=50000)\n> print \\$out: 1.70027 wallclock secs ( 1.66 usr + 0.00 sys = 1.66 CPU) @ 30120.48/s (n=50000)\n> tie *STDOUT: 1.82248 wallclock secs ( 1.79 usr + 0.00 sys = 1.79 CPU) @ 27932.96/s (n=50000)\n> Rate tie *STDOUT output print \\$out perlio\n> tie *STDOUT 27933/s -- -1% -7% -42%\n> output 28249/s 1% -- -6% -42%\n> print \\$out 30120/s 8% 7% -- -38%\n> perlio 48544/s 74% 72% 61% --\n> \n> Benchmark: running output, perlio, print \\$out, tie *STDOUT for at least 10 CPU seconds...\n> output: 10.7199 wallclock secs (10.53 usr + 0.00 sys = 10.53 CPU) @ 28029.63/s (n=295152)\n> perlio: 11.2884 wallclock secs (10.46 usr + 0.00 sys = 10.46 CPU) @ 49967.11/s (n=522656)\n> print \\$out: 10.5978 wallclock secs (10.43 usr + 0.00 sys = 10.43 CPU) @ 30318.79/s (n=316225)\n> tie *STDOUT: 11.3525 wallclock secs (10.68 usr + 0.00 sys = 10.68 CPU) @ 27635.96/s (n=295152)\n> Rate tie *STDOUT output print \\$out perlio\n> tie *STDOUT 27636/s -- -1% -9% -45%\n> output 28030/s 1% -- -8% -44%\n> print \\$out 30319/s 10% 8% -- -39%\n> perlio 49967/s 81% 78% 65% --\n> need\n> \n> Attached there is script that was used to produce those results.\n\nOk, on my machine it's similar:\n\n Rate output tie *STDOUT print \\$out\noutput 150962/s -- -1% -7%\ntie *STDOUT 152769/s 1% -- -6%\nprint \\$out 162604/s 8% 6% --\n\nis roughly consistent image coming out of it.\n\nI guess the time spent here is generally negligible in gitweb anyway...\nI suggested using output() because I think hacking it would be _very_\n_slightly_ easier than tied filehandle, but you are right that doing\nthat is also really easy; having the possibility to use PerlIO::Util if\navailable would be non-essentially nice, but requiring it by stock\ngitweb is not reasonable, especially seeing that it's not packaged even\nfor Debian. ;-)\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nIf you can't see the value in jet powered ants you should turn in\nyour nerd card. -- Dunbal (464142)\n"},{"id":"134163","messageId":"201002101922.50010.jnareb@gmail.com","threadId":"22584","inReplyTo":"20100210120257.GP4159@machine.or.cz","subject":"Re: [RFC PATCHv2 04/10] gitweb: Use Cache::Cache compatibile (get, set) output caching","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-10T18:22:48Z","receivedAt":"2010-02-10T18:22:48Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Dnia środa 10. lutego 2010 13:02, Petr Baudis napisał:\n> On Wed, Feb 10, 2010 at 12:28:14PM +0100, Jakub Narebski wrote:\n>> On Wed, 10 Feb 2010, Petr Baudis wrote:\n>>> On Wed, Feb 10, 2010 at 02:12:24AM +0100, Jakub Narebski wrote:\n\n[...]\n>>>> So we either would have to live with non-core PerlIO::Util or (pure Perl)\n>>>> Capture::Tiny, or do the 'print -> print $out' patch...\n>>> \n>>> All the magic methods seem to be troublesome, but in that case I'd\n>>> really prefer a level of indirection instead of filehandle - as is,\n>>> 'print (...) -> output (...)' ins. of 'print (...) -> print $out (...)'\n>>> (or whatever). That should be really flexible and completely\n>>> futureproof, and I don't think the level of indirection would incur any\n>>> measurable overhead, would it?\n>> \n>> First, it is not only 'print (...) -> print $out (...)'; [...]\n>> \n>> Second, using \"tie\" on filehandle (on *STDOUT) can be used also for \n>> just capturing output, not only for \"tee\"-ing; [...]\n>> \n>> Third, as you can see below tie-ing is about 1% slower than using\n>> 'output (...)', which in turn is less than 10% slower than explicit\n>> filehandle solution i.e. 'print $out (...)'... and is almost twice\n>> slower than solution using PerlIO::Util\n[...]\n>> Rate tie *STDOUT output print \\$out perlio\n>> tie *STDOUT 27636/s -- -1% -9% -45%\n>> output 28030/s 1% -- -8% -44%\n>> print \\$out 30319/s 10% 8% -- -39%\n>> perlio 49967/s 81% 78% 65% --\n>> need\n \n> Ok, on my machine it's similar:\n> \n> Rate output tie *STDOUT print \\$out\n> output 150962/s -- -1% -7%\n> tie *STDOUT 152769/s 1% -- -6%\n> print \\$out 162604/s 8% 6% --\n\nI wonder why in my case the 'output (...)' solution was faster than tie,\nand you have tie faster than 'output'... but I guess 1% is the noise\nlevel.\n \n> is roughly consistent image coming out of it.\n> \n> I guess the time spent here is generally negligible in gitweb anyway...\n> I suggested using output() because I think hacking it would be _very_\n> _slightly_ easier than tied filehandle, but you are right that doing\n> that is also really easy; having the possibility to use PerlIO::Util if\n> available would be non-essentially nice, but requiring it by stock\n> gitweb is not reasonable, especially seeing that it's not packaged even\n> for Debian. ;-)\n\nWell, the idea was to use PerlIO::Util if possible, checking it via\n\n out $use_perlio_layers = eval { require PerlIO::Util; 1 };\n\nand fallback to generic mechanism if it is not present. Only the\ngeneric mechanism would have to be changed from manipulating *STDOUT\n(*STDOUT = $data_fh etc.) to tied filehandle.\n\nWhat we need to be careful about is ':utf8' vs ':raw' mode (IO layer).\nIn the PerlIO layers solution, and in 'print -> print $out '\nsolution where $out = $data_fh, and $data_fh was opened to in-memory\nfile, the data saved in variable is already converted, already passed\nvia 'utf8' layer, and is saved as bytes. And if we use binary mode,\nit is passed unchanged, and is also saved as bytes. Therefore we can\nsave to cache file in ':raw' more, and read from cache file in ':raw'\nmode, and that is why we don't need separate files for text and for\nbinary output.\n\nPRINT method in class tied to filehandle gets _untransformed_ argument,\nso we have to use utf8::encode($str) if in ':utf8' mode, and either\nuse PerlIO::get_layers on *STDOUT, or provide BINMODE method in tied\nclass to watch for mode changes. (Note that e.g. for snapshots we \nprint HTTP headers in ':utf8' mode, and the snapshot itself in ':raw'\ni.e. binary mode.).\n\nBut all that is doable, and not that much work. Well, perhaps more\nthan in the case of 'print -> print $out' etc., and opening in-memory\nfile via\n\n open $data_fh, '>', \\$data;\n\nbut not that more, and we don't need extra global variable $out. But\nno large gitweb patch, and no worry about somebody accidentally using\n'print ;' or 'printf , ;' instead of respectively\n'print $out ;' and 'printf $out , ;'.\n\n\nAs to how I installed PerlIO::Util for myself (this might be interesting\nto other people): in short, I use local::lib bootstrapping and cpan \nclient. I could from start install some Perl modules from CPAN locally\nusing 'cpan' client (included in perl RPM). I have asked on #perl \nchannel on FreeNode what to do, and they recommended local::lib. After\nfollowing the bootstapping technique described in local::lib manpage\n(see e.g. http://p3rl.org/local::lib) installing PerlIO::Util in\n~/perl5 is as simple as 'cpan -i PerlIO::Util' (or using 'cpan' client\ninteractively).\n\nYou can always put\n use lib '/path/to/perl5/lib';\nin your $GITWEB_CONFIG file.\n\nPerhaps adding something like \"use lib __DIR__.'/lib';\" somewhere near\nbeginning of file (where __DIR__ is appropriate expression that expands\nto directory the gitweb.cgi/gitweb.perl is in) to gitweb would be a good\nidea? Then you would be able to make __DIR__/lib symlink to local Perl\nmodules, or put extra modules by hand under __DIR__/lib.\n-- \nJakub Narebski\nPoland\n"},{"id":"134179","messageId":"201002102132.56406.jnareb@gmail.com","threadId":"22584","inReplyTo":"201002101922.50010.jnareb@gmail.com","subject":"Re: [RFC PATCHv2 04/10] gitweb: Use Cache::Cache compatibile (get, set) output caching","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-10T20:32:54Z","receivedAt":"2010-02-10T20:32:54Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Dnia środa 10. lutego 2010 19:22, Jakub Narebski napisał:\n\n> Well, the idea was to use PerlIO::Util if possible, checking it via\n> \n>   out $use_perlio_layers = eval { require PerlIO::Util; 1 };\n> \n> and fallback to generic mechanism if it is not present.  Only the\n> generic mechanism would have to be changed from manipulating *STDOUT\n> (*STDOUT = $data_fh etc.) to tied filehandle.\n\nWell, damn, this is not needed[1]. Instead of manipulating *STDOUT,\nit is enough to use 'select FILEHANDLE'. It means that the capture\nwould look like this:\n\n open my $data_fh, '>', \\$data;\n my $oldfh = select($data_fh);\n\n # ... code that uses 'print '\n\n select($oldfh);\n close $data_fh;\n\nOne thing that needs to be done is changing 'binmode STDOUT ...' into\n'binmode select() ...'\n\n[1] Thanks to mst on #perl channel for pointing this to me.\n-- \nJakub Narebski\nPoland\n"}]}