{"thread":{"id":"12725","subject":"[PATCH 1/3] gitweb: Separate @projects population into git_get_projects_details()","startedAt":"2008-03-17T15:09:27Z","lastAt":"2008-03-18T09:52:52Z","messageCount":12,"participants":["Jakub Narebski","Frank Lichtenheld","Petr Baudis"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"72292","messageId":"1205766570-13550-1-git-send-email-jnareb@gmail.com","threadId":"12725","inReplyTo":null,"subject":"[PATCH 0/3 v2] gitweb: Support caching projects list","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-03-17T15:09:27Z","receivedAt":"2008-03-17T15:09:27Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"This series of patches is resend of patch by Petr 'Pasky' Baudis with\nthe same subject, which can be found in,\n Message-ID: <20080313231413.27966.3383.stgit@rover>\n http://permalink.gmane.org/gmane.comp.version-control.git/77151\nsplit into two patches (so the exact details of serializing and\ncaching can be separated from independent code improvement), and with\nadded lazy filling of details for a project.\n\nAt the bottom there is interdiff between Pasky's result and result\nafter first two patches here. Besides a bit of style changes the main\ndifference is that in this version dump of @projects array is done in\n'terse' form, so it can be eval'ed directly into @projects.\n\nTable of contents:\n==================\n [PATCH 1/3] gitweb: Separate filling projects info\n into git_get_projects_details()\n [PATCH 2/3] gitweb: Support caching projects list\n [PATCH 3/3] gitweb: Fill project details only if project path\n mtime changed\n\nShortlog:\n=========\nJakub Narebski (1):\n gitweb: Fill project details only if project path mtime changed\n\nPetr Baudis (2):\n gitweb: Separate filling projects info into git_get_projects_details()\n gitweb: Support caching projects list\n\nDiffstat:\n=========\n gitweb/gitweb.css | 6 ++++\n gitweb/gitweb.perl | 73 ++++++++++++++++++++++++++++++++++++++++++++++++---\n 2 files changed, 74 insertions(+), 5 deletions(-)\n\nInterdiff:\n==========\n gitweb/gitweb.perl | 35 ++++++++++++++++++++---------------\n 1 files changed, 20 insertions(+), 15 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex bee5ec8..5527378 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -123,7 +123,7 @@ our @diff_opts = ('-M'); # taken from git_commit\n # index lifetime in minutes\n # the cached list version is stored in /tmp and can be tweaked\n # by other scripts running with the same uid as gitweb - use this\n-# only at secure installations; only single gitweb project root per\n+# ONLY at secure installations; only single gitweb project root per\n # system is supported!\n our $projlist_cache_lifetime = 0;\n \n@@ -3482,6 +3482,8 @@ sub git_patchset_body {\n \n # . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .\n \n+# fill age, description, owner, forks (last one only if $check_forks)\n+# for all projects in $projlist reference; fill projects info\n sub git_get_projects_details {\n \tmy ($projlist, $check_forks) = @_;\n \n@@ -3521,34 +3523,37 @@ sub git_project_list_body {\n \n \tmy ($check_forks) = gitweb_check_feature('forks');\n \n-\tmy $cache_file = '/tmp/gitweb.index.cache';\n \tuse File::stat;\n+\tuse POSIX qw(:fcntl_h);\n+\n+\tmy $cache_file = '/tmp/gitweb.index.cache';\n \n \tmy @projects;\n \tmy $stale = 0;\n-\tif ($cache_lifetime and -f $cache_file\n-\t and stat($cache_file)->mtime + $cache_lifetime * 60 > time()\n-\t and open (my $fd, $cache_file)) {\n-\t\t$stale = time() - stat($cache_file)->mtime;\n-\t\tmy @dump = <$fd>;\n+\tmy $now = time();\n+\tif ($cache_lifetime && -f $cache_file &&\n+\t stat($cache_file)->mtime + $cache_lifetime * 60 > $now &&\n+\t open(my $fd, '<', $cache_file)) {\n+\t\t$stale = $now - stat($cache_file)->mtime;\n+\t\tlocal $/ = undef;\n+\t\tmy $dump = <$fd>;\n \t\tclose $fd;\n-\t\t# Hack zone start\n-\t\tmy $VAR1;\n-\t\teval join(\"\\n\", @dump);\n-\t\t@projects = @$VAR1;\n-\t\t# Hack zone end\n+\t\t@projects = @{ eval $dump };\n \t} else {\n-\t\tif ($cache_lifetime and -f $cache_file) {\n+\t\tif ($cache_lifetime && -f $cache_file) {\n \t\t\t# Postpone timeout by two minutes so that we get\n \t\t\t# enough time to do our job.\n \t\t\tmy $time = time() - $cache_lifetime + 120;\n \t\t\tutime $time, $time, $cache_file;\n \t\t}\n \t\t@projects = git_get_projects_details($projlist, $check_forks);\n-\t\tif ($cache_lifetime and open (my $fd, '>'.$cache_file)) {\n+\t\tif ($cache_lifetime &&\n+\t\t sysopen(my $fd, \"$cache_file.lock\", O_WRONLY|O_CREAT|O_EXCL, 0600)) {\n \t\t\tuse Data::Dumper;\n+\t\t\t$Data::Dumper::Terse = 1;\n \t\t\tprint $fd Dumper(\\@projects);\n \t\t\tclose $fd;\n+\t\t\trename \"$cache_file.lock\", $cache_file;\n \t\t}\n \t}\n \n@@ -3556,7 +3561,7 @@ sub git_project_list_body {\n \t$from = 0 unless defined $from;\n \t$to = $#projects if (!defined $to || $#projects < $to);\n \n-\tif ($cache_lifetime and $stale) {\n+\tif ($cache_lifetime && $stale) {\n \t\tprint \"
Cached version (${stale}s old)
\\n\";\n \t}\n \n"},{"id":"72290","messageId":"1205766570-13550-2-git-send-email-jnareb@gmail.com","threadId":"12725","inReplyTo":"1205766570-13550-1-git-send-email-jnareb@gmail.com","subject":"[PATCH 1/3] gitweb: Separate @projects population into git_get_projects_details()","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-03-17T15:09:28Z","receivedAt":"2008-03-17T15:09:28Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"From: Petr Baudis \n\nFor clarity projects scanning and @projects population is separated to\ngit_get_projects_details().\n\nThis would be required if/when implementing in-gitweb caching of\nprojects list generation.\n\nSigned-off-by: Petr Baudis \nSigned-off-by: Jakub Narebski \n---\nThis is first part of patch sent by Petr Baudis; one that could be\napplied to have better, more clear code, even as we are rehashing on\n_how_ to do caching in gitweb in general, and projects list caching in\nparticular.\n\nNote: git_get_projects_details() does not do\n return wantarray ? @projects : \\@projects\ndance.\n\nBy the way; it could modify %$projlist directly, and return simply\n$projlist.\n\n gitweb/gitweb.perl | 17 +++++++++++++----\n 1 files changed, 13 insertions(+), 4 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex ec73cb1..90ab894 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3473,10 +3473,10 @@ sub git_patchset_body {\n \n # . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .\n \n-sub git_project_list_body {\n-\tmy ($projlist, $order, $from, $to, $extra, $no_header) = @_;\n-\n-\tmy ($check_forks) = gitweb_check_feature('forks');\n+# fill age, description, owner, forks (last one only if $check_forks)\n+# for all projects in $projlist reference; fill projects info\n+sub git_get_projects_details {\n+\tmy ($projlist, $check_forks) = @_;\n \n \tmy @projects;\n \tforeach my $pr (@$projlist) {\n@@ -3506,6 +3506,15 @@ sub git_project_list_body {\n \t\t}\n \t\tpush @projects, $pr;\n \t}\n+\treturn @projects;\n+}\n+\n+sub git_project_list_body {\n+\tmy ($projlist, $order, $from, $to, $extra, $no_header) = @_;\n+\n+\tmy ($check_forks) = gitweb_check_feature('forks');\n+\n+\tmy @projects = git_get_projects_details($projlist, $check_forks);\n \n \t$order ||= $default_projects_order;\n \t$from = 0 unless defined $from;\n-- \n1.5.4.3.453.gc1ad83\n"},{"id":"72293","messageId":"1205766570-13550-3-git-send-email-jnareb@gmail.com","threadId":"12725","inReplyTo":"1205766570-13550-1-git-send-email-jnareb@gmail.com","subject":"[RFC/PATCH 2/3] gitweb: Support caching projects list","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-03-17T15:09:29Z","receivedAt":"2008-03-17T15:09:29Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"From: Petr Baudis \n\nOn repo.or.cz (permanently I/O overloaded and hosting 1050 project +\nforks), the projects list (the default gitweb page) can take more than\na minute to generate. This naive patch adds simple support for caching\nthe projects list data structure so that all the projects do not need\nto get rescanned at every page access.\n\n$projlist_cache_lifetime gitweb configuration variable is introduced,\nby default set to zero. If set to non-zero, it describes the number of\nminutes for which the cache remains valid. Only single project root\nper system can use the cache. Any script running with the same uid as\ngitweb can change the cache trivially - this is for secure\ninstallations only.\n\nThe cache itself is stored in /tmp/gitweb.index.cache as a\nData::Dumper dump of the perl data structure with the list of project\ndetails. When reusing the cache, the file is simply eval'd back into\n@projects.\n\nTo prevent contention when multiple accesses coincide with cache\nexpiration, the timeout is postponed to time()+120 when we start\nrefreshing. When showing cached version, a disclaimer is shown\nat the top of the projects list.\n\nSigned-off-by: Petr Baudis \nSigned-off-by: Jakub Narebski \n---\nThis is (slightly changed) second part of Petr Baudis patch; the\ndifference (intediff) between this version and the original can be\nfound in cover letter for this series.\n\nThe differences, besides a bit of style changes like using '&&'\ninstead of 'and', are:\n * Current version reads cache file in full, in 'slurp' mode, instead\n of reading it line by line and then concatenating lines.\n * Current version dumps @projects in the 'terse' mode, so it can be\n eval'ed directly into @projects, without need of extra variable.\n * Current version does atomic writing to cache file by writing first\n to temporary file (there in exclusive mode to *.lock file, but\n File::Temp::tempfile() temporary file could be used instead), and\n then renaming file. This way we avoid possibility of reading\n partially created file. Opening file in O_EXCL mode should prevent\n writers trampling one over another, and make only one instance of\n gitweb fill cache; on the other hand if somehow *.lock file is not\n deleted it would prevent regenerating cache.\n\nNote: instead of using Data::Dumper to serialize data we could use\nStorable module (distributed with Perl like Data::Dumper). From what\nI've checked it has larger initial cost, but might be better for\nlarger number of projects, exactly the situation when projects list\ncaching is needed.\n\nI can send version using Storable; could you compare then Data::Dumper\non repo.or.cz set of repositories then, Pasky?\n\n gitweb/gitweb.css | 6 ++++++\n gitweb/gitweb.perl | 51 ++++++++++++++++++++++++++++++++++++++++++++++++---\n 2 files changed, 54 insertions(+), 3 deletions(-)\n\ndiff --git a/gitweb/gitweb.css b/gitweb/gitweb.css\nindex 446a1c3..1e83896 100644\n--- a/gitweb/gitweb.css\n+++ b/gitweb/gitweb.css\n@@ -85,6 +85,12 @@ div.title, a.title {\n \tcolor: #000000;\n }\n \n+div.stale_info {\n+\tdisplay: block;\n+\ttext-align: right;\n+\tfont-style: italic;\n+}\n+\n div.readme {\n \tpadding: 8px;\n }\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 90ab894..5527378 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -118,6 +118,15 @@ our $fallback_encoding = 'latin1';\n # - one might want to include '-B' option, e.g. '-B', '-M'\n our @diff_opts = ('-M'); # taken from git_commit\n \n+# projects list cache for busy sites with many projects;\n+# if you set this to non-zero, it will be used as the cached\n+# index lifetime in minutes\n+# the cached list version is stored in /tmp and can be tweaked\n+# by other scripts running with the same uid as gitweb - use this\n+# ONLY at secure installations; only single gitweb project root per\n+# system is supported!\n+our $projlist_cache_lifetime = 0;\n+\n # information about snapshot formats that gitweb is capable of serving\n our %known_snapshot_formats = (\n \t# name => {\n@@ -3510,16 +3519,52 @@ sub git_get_projects_details {\n }\n \n sub git_project_list_body {\n-\tmy ($projlist, $order, $from, $to, $extra, $no_header) = @_;\n+\tmy ($projlist, $order, $from, $to, $extra, $no_header, $cache_lifetime) = @_;\n \n \tmy ($check_forks) = gitweb_check_feature('forks');\n \n-\tmy @projects = git_get_projects_details($projlist, $check_forks);\n+\tuse File::stat;\n+\tuse POSIX qw(:fcntl_h);\n+\n+\tmy $cache_file = '/tmp/gitweb.index.cache';\n+\n+\tmy @projects;\n+\tmy $stale = 0;\n+\tmy $now = time();\n+\tif ($cache_lifetime && -f $cache_file &&\n+\t stat($cache_file)->mtime + $cache_lifetime * 60 > $now &&\n+\t open(my $fd, '<', $cache_file)) {\n+\t\t$stale = $now - stat($cache_file)->mtime;\n+\t\tlocal $/ = undef;\n+\t\tmy $dump = <$fd>;\n+\t\tclose $fd;\n+\t\t@projects = @{ eval $dump };\n+\t} else {\n+\t\tif ($cache_lifetime && -f $cache_file) {\n+\t\t\t# Postpone timeout by two minutes so that we get\n+\t\t\t# enough time to do our job.\n+\t\t\tmy $time = time() - $cache_lifetime + 120;\n+\t\t\tutime $time, $time, $cache_file;\n+\t\t}\n+\t\t@projects = git_get_projects_details($projlist, $check_forks);\n+\t\tif ($cache_lifetime &&\n+\t\t sysopen(my $fd, \"$cache_file.lock\", O_WRONLY|O_CREAT|O_EXCL, 0600)) {\n+\t\t\tuse Data::Dumper;\n+\t\t\t$Data::Dumper::Terse = 1;\n+\t\t\tprint $fd Dumper(\\@projects);\n+\t\t\tclose $fd;\n+\t\t\trename \"$cache_file.lock\", $cache_file;\n+\t\t}\n+\t}\n \n \t$order ||= $default_projects_order;\n \t$from = 0 unless defined $from;\n \t$to = $#projects if (!defined $to || $#projects < $to);\n \n+\tif ($cache_lifetime && $stale) {\n+\t\tprint \"
Cached version (${stale}s old)
\\n\";\n+\t}\n+\n \tprint \"\\n\";\n \tunless ($no_header) {\n \t\tprint \"\\n\";\n@@ -3902,7 +3947,7 @@ sub git_project_list {\n \t\tclose $fd;\n \t\tprint \"\\n\";\n \t}\n-\tgit_project_list_body(\\@list, $order);\n+\tgit_project_list_body(\\@list, $order, undef, undef, undef, undef, $projlist_cache_lifetime);\n \tgit_footer_html();\n }\n \n-- \n1.5.4.3.453.gc1ad83\n"},{"id":"72291","messageId":"1205766570-13550-4-git-send-email-jnareb@gmail.com","threadId":"12725","inReplyTo":"1205766570-13550-1-git-send-email-jnareb@gmail.com","subject":"[RFC/PATCH 3/3] gitweb: Fill project details lazily when caching","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-03-17T15:09:30Z","receivedAt":"2008-03-17T15:09:30Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"If caching is turned on project details can be filled in already from\nthe cache. When refreshing project info details for all project (when\ncache is stale and has to be refreshed) generate projects info only if\nmodification time (as returned by lstat()) of projects repository\ngitdir changed.\n\nThis way we can avoid hitting repository refs, object database and\nrepository config at the cost of additional lstat.\n\nSigned-off-by: Jakub Narebski \n---\nThis is an idea for further improvement of 'projects list caching'.\nCould you please: \n\n 1.) comment if it is a good idea, or why it works, or why it\n couldn't work :), \n\n 2.) check if this change gives any improvements in performance on\n real data; note that testing would require updating repositories\n if test on generated data was done, or gathering statistics over\n larger time period if it was tested on \"live\" set.\n\nThanks in advance.\n\n gitweb/gitweb.perl | 9 +++++++++\n 1 files changed, 9 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 5527378..1741628 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3487,8 +3487,14 @@ sub git_patchset_body {\n sub git_get_projects_details {\n \tmy ($projlist, $check_forks) = @_;\n \n+\tuse File::stat;\n \tmy @projects;\n \tforeach my $pr (@$projlist) {\n+\t\tmy $mtime;\n+\t\tif ($cached && $pr->{'mtime'}) {\n+\t\t\t$mtime = lstat(\"$projectroot/$pr->{'path'}\")->mtime;\n+\t\t\tnext if ($mtime <= $pr->{'mtime'});\n+\t\t}\n \t\tmy (@aa) = git_get_last_activity($pr->{'path'});\n \t\tunless (@aa) {\n \t\t\tnext;\n@@ -3513,6 +3519,9 @@ sub git_get_projects_details {\n \t\t\t\t$pr->{'forks'} = 0;\n \t\t\t}\n \t\t}\n+\t\tif ($cached) {\n+\t\t\t$pr->{'mtime'} = $mtime;\n+\t\t}\n \t\tpush @projects, $pr;\n \t}\n \treturn @projects;\n-- \n1.5.4.3.453.gc1ad83\n"},{"id":"72301","messageId":"20080317165405.GD18624@mail-vs.djpig.de","threadId":"12725","inReplyTo":"1205766570-13550-3-git-send-email-jnareb@gmail.com","subject":"Re: [RFC/PATCH 2/3] gitweb: Support caching projects list","fromName":"Frank Lichtenheld","fromEmail":"frank@lichtenheld.de","sentAt":"2008-03-17T16:54:05Z","receivedAt":"2008-03-17T16:54:05Z","isPatch":true,"sender":{"key":"frank@lichtenheld.de","avatar":"https://gravatar.com/avatar/b9f1d4b120e138f157c9e480d0818197c474628923786adb98f30017cdb99c3c?d=mp&s=160"},"body":"On Mon, Mar 17, 2008 at 04:09:29PM +0100, Jakub Narebski wrote:\n> From: Petr Baudis \n> $projlist_cache_lifetime gitweb configuration variable is introduced,\n> by default set to zero. If set to non-zero, it describes the number of\n> minutes for which the cache remains valid. Only single project root\n> per system can use the cache. Any script running with the same uid as\n> gitweb can change the cache trivially - this is for secure\n> installations only.\n\nThe more subtle threat is the fact that anyone with writing\nrights to /tmp can give gitweb any data he wants if the file doesn't\nexist yet.\n\nAt the very least you should:\n\n - Allow to override /tmp (via ENV{TMPDIR} or via a configuration\n variable)\n - Advise people to change that to something that is not world-writable\n - Check if the file is owned by the uid gitweb is running under and\n not word-writable.\n\n[...]\n> +\tmy @projects;\n> +\tmy $stale = 0;\n> +\tmy $now = time();\n> +\tif ($cache_lifetime && -f $cache_file &&\n> +\t stat($cache_file)->mtime + $cache_lifetime * 60 > $now &&\n> +\t open(my $fd, '<', $cache_file)) {\n> +\t\t$stale = $now - stat($cache_file)->mtime;\n\nOne stat() call instead of three would be better for performance.\n\nGruesse,\n-- \nFrank Lichtenheld \nwww: http://www.djpig.de/\n"},{"id":"72308","messageId":"200803171952.15186.jnareb@gmail.com","threadId":"12725","inReplyTo":"20080317165405.GD18624@mail-vs.djpig.de","subject":"Re: [RFC/PATCH 2/3] gitweb: Support caching projects list","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-03-17T18:52:13Z","receivedAt":"2008-03-17T18:52:13Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Dnia poniedziałek 17. marca 2008 17:54, Frank Lichtenheld napisał:\n\n>At the very least you should:\n[...]\n>  - Check if the file is owned by the uid gitweb is running under and\n>    not word-writable.\n\nUID ($>) or PID ($$) should be equal to cache owner: stat($file)->uid?\n\n-- \nJakub Narebski\nPoland\n"},{"id":"72310","messageId":"20080317191029.GE18624@mail-vs.djpig.de","threadId":"12725","inReplyTo":"200803171952.15186.jnareb@gmail.com","subject":"Re: [RFC/PATCH 2/3] gitweb: Support caching projects list","fromName":"Frank Lichtenheld","fromEmail":"frank@lichtenheld.de","sentAt":"2008-03-17T19:10:29Z","receivedAt":"2008-03-17T19:10:29Z","isPatch":true,"sender":{"key":"frank@lichtenheld.de","avatar":"https://gravatar.com/avatar/b9f1d4b120e138f157c9e480d0818197c474628923786adb98f30017cdb99c3c?d=mp&s=160"},"body":"On Mon, Mar 17, 2008 at 07:52:13PM +0100, Jakub Narebski wrote:\n> Dnia poniedziałek 17. marca 2008 17:54, Frank Lichtenheld napisał:\n> \n> >At the very least you should:\n> [...]\n> >  - Check if the file is owned by the uid gitweb is running under and\n> >    not word-writable.\n> \n> UID ($>) or PID ($$) should be equal to cache owner: stat($file)->uid?\n\nI'm not sure what the PID has to do with anything here?\nBut yeah, $> was what I meant.\n(Although I actually prefer to use POSIX::geteuid instead, since I can\nunderstand that faster).\n\nGruesse,\n-- \nFrank Lichtenheld \nwww: http://www.djpig.de/\n"},{"id":"72318","messageId":"200803172125.39150.jnareb@gmail.com","threadId":"12725","inReplyTo":"20080317191029.GE18624@mail-vs.djpig.de","subject":"Re: [RFC/PATCH 2/3] gitweb: Support caching projects list","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-03-17T20:25:38Z","receivedAt":"2008-03-17T20:25:38Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Dnia poniedziałek 17. marca 2008 20:10, Frank Lichtenheld napisał:\n> On Mon, Mar 17, 2008 at 07:52:13PM +0100, Jakub Narebski wrote:\n>> Dnia poniedziałek 17. marca 2008 17:54, Frank Lichtenheld napisał:\n>> \n>>>At the very least you should:\n>> [...]\n>>>  - Check if the file is owned by the uid gitweb is running under and\n>>>    not word-writable.\n>> \n>> UID ($>) or PID ($$) should be equal to cache owner: stat($file)->uid?\n> \n> I'm not sure what the PID has to do with anything here?\n> But yeah, $> was what I meant.\n> (Although I actually prefer to use POSIX::geteuid instead, since I can\n> understand that faster).\n\nActually what I wanted to ask was UID ($<) vs EUID ($>), or appropriate\nPOSIX::get*uid functions.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"72324","messageId":"1205795590-18420-1-git-send-email-jnareb@gmail.com","threadId":"12725","inReplyTo":"1205766570-13550-3-git-send-email-jnareb@gmail.com","subject":"[RFC/PATCH 2/3 v2] gitweb: Support caching projects list","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-03-17T23:13:10Z","receivedAt":"2008-03-17T23:13:10Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"This version uses Storable (which gives binary output), instead of a\nbit unsafe, because of need to 'eval' to unserialize, Data::Dumper.\n\nIt should also be a bit faster: unscientific comparison gives that\nStorable is 3-4 times faster than using Data::Dumper on totally\nartificial data of 1050 elements table of hashes.\n\nBelow replies to comments for first (or second if you count original\nPasky version) version of this patch.\n\nNo interdiff there... on the other hand it includes gitweb/README\n\n%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%\n\n[The message below didn't made it to git mailing list due to not\nquoting J.H. name in email: it should be \n \"J.H.\" \nnot\n J.H. ]\n\nOn Mon, 17 Mar 2008, Rafael Garcia-Suarez wrote:\n> On 17/03/2008, Jakub Narebski wrote:\n>>\n>> The cache itself is stored in /tmp/gitweb.index.cache as a\n>> Data::Dumper dump of the perl data structure with the list of project\n>> details. When reusing the cache, the file is simply eval'd back into\n>> @projects.\n> \n> Correct me if I'm wrong (I haven't read the full source code...) Doesn't\n> that mean that anyone who can write this file can make gitweb eval\n> arbitrary perl code? And anyone can write a file in /tmp, usually... If\n> so, I would strongly advise against including this patch in its current\n> state, not without at least checking the ownership of the cache file.\n\nFirst, this patch is an RFC, mainly meant as better version of Pasky's\npatch with (the nearly) the same subject, to help repo.or.cz, when it\ncan be assumed that installation is secure.\n \nSecond, it was changed in this version of patch: gitweb now uses\nStorable to write Perl data to disk and read it back, so there is no\nneed to eval to read back the data from cache, as it was when using\nData::Dumper package. Additionally you can now define where to store\ncache; by default it is /tmp/gitweb/, and you can make gitweb write\nonly for projects with gitweb uid (if directory does not exists,\ngitweb would create it if possible with appropriate permissions).\n\n>> Note: instead of using Data::Dumper to serialize data we could use\n>> Storable module (distributed with Perl like Data::Dumper). From what\n>> I've checked it has larger initial cost, but might be better for\n>> larger number of projects, exactly the situation when projects list\n>> caching is needed.\n> \n> Storable would be more secure. (But check file ownership!)\n\nThis version uses Storable, which should be more secure (but see\nbelow)... and additionally bit faster.\n\nFor the simplicity of patch gitweb does not check cache file\nownership, nor permissions on cache file and on directory it is in,\nbut this should be fairly easy to add.\n\nBTW. store() and retrieve() from Storable \"die\" on many errors;\nin unsafe environment we would probably want to protect gitweb from\nthem failing due to incorrect file format, or nonreadable cache file,\netc.\n\n> I don't know how complex are the data structures you're going to\n> save.\n\nIt is array (list) or array reference (but ordering of items doesn't\nmatter) of records (hashes / hash references). Fields can be integer,\nstring (possibly containing quite characters and other strangeness),\nor undef.\n\n> My paranoid preference would be to use a text-only format. If CPAN\n> modules are out, devise your own dump/load routines, maybe? \n\nWhat format should it be? Data::Dumper format looks like this:\n\n [\n {\n 'owner' => 'Jakub Narebski',\n 'descr_long' => './t9500-gitweb-standalone-no-errors.sh test repository',\n 'mtime' => undef,\n 'age_string' => '7 days ago',\n 'path' => 'trash.git',\n 'age' => 652787,\n 'descr' => './t9500-gitweb-standalone... '\n },\n {\n 'owner' => 'Jakub Narebski',\n 'descr_long' => \"git with Jakub Narebski modifications, local copy.\",\n 'mtime' => undef,\n 'age_string' => '80 min ago',\n 'path' => 'git.git',\n 'age' => 4813,\n 'descr' => 'git with Jakub Narebski modif... '\n }\n ]\n\n\nAnother solution would be to use YAML [Tiny], but although Perl\npackages for YAML are in contrib repositories, they are not installed\nby default with Perl, and most probably are not present on the\nsystem. Additonally at least YAML::Tiny is slower than even\nData::Dumper when retrieving data, around 4-5 times slower... on the\nother hand YAML::Tiny is Pure Perl implementation; other solutions\nwhich use C libraries should be much faster.\n\nYAML output looks like this:\n\n ---\n -\n age: 652787\n age_string: '7 days ago'\n descr: './t9500-gitweb-standalone... '\n descr_long: './t9500-gitweb-standalone-no-errors.sh test repository'\n mtime: ~\n owner: 'Jakub Narebski'\n path: trash.git\n -\n age: 4813\n age_string: '80 min ago'\n descr: 'git with Jakub Narebski modif... '\n descr_long: 'git with Jakub Narebski modifications, local copy.'\n mtime: ~\n owner: 'Jakub Narebski'\n path: git.git\n\n\nYet another solution would be to use [much] restricted version of\nini-like git config format, and perhaps reuse git-cvsserver parsing of\nthis format, or simply call \"git-config --file -z -l\" and\ngitweb code to parse git-config output.\n\nThe output whould then look like this:\n\n [gitweb \"trash.git\"]\n \tage = 652787\n \tage_string = \"7 days ago\"\n \tdescr = './t9500-gitweb-standalone... '\n \tdescr_long = \"./t9500-gitweb-standalone-no-errors.sh test repository\"\n \tmtime\n \towner = Jakub Narebski\n\n [gitweb \"git.git\"]\n \tage = 4813\n \tage_string = \"80 min ago\"\n \tdescr = \"git with Jakub Narebski modif... \"\n \tdescr_long = \"git with Jakub Narebski modifications, local copy.\"\n \tmtime\n \towner = Jakub Narebski\n\n%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%\n\nOn Mon, 17 Mar 2008, Frank Lichtenheld wrote:\n> On Mon, Mar 17, 2008 at 04:09:29PM +0100, Jakub Narebski wrote:\n>>\n>> From: Petr Baudis \n>> $projlist_cache_lifetime gitweb configuration variable is introduced,\n>> by default set to zero. If set to non-zero, it describes the number of\n>> minutes for which the cache remains valid. Only single project root\n>> per system can use the cache. Any script running with the same uid as\n>> gitweb can change the cache trivially - this is for secure\n>> installations only.\n> \n> The more subtle threat is the fact that anyone with writing\n> rights to /tmp can give gitweb any data he wants if the file doesn't\n> exist yet.\n\nRight.\n \n> At the very least you should:\n> \n> - Allow to override /tmp (via ENV{TMPDIR} or via a configuration\n> variable)\n\nDone.\n\n> - Advise people to change that to something that is not\n> world-writable\n\nI wrote a bit about this in gitweb/README, feel free to add to it.\nAdditionally gitweb can create cache directory if it does not exist,\nand it does so with restrictive premissions.\n\n> - Check if the file is owned by the uid gitweb is running under and\n> not word-writable.\n\nCurrently not done, to make patch simpler; should be fairly easy to\nadd. I guess it would be best to create is_cache_safe() or something\nlike that subroutine.\n\n> [...]\n>> +\tmy @projects;\n>> +\tmy $stale = 0;\n>> +\tmy $now = time();\n>> +\tif ($cache_lifetime && -f $cache_file &&\n>> +\t stat($cache_file)->mtime + $cache_lifetime * 60 > $now &&\n>> +\t open(my $fd, '<', $cache_file)) {\n>> +\t\t$stale = $now - stat($cache_file)->mtime;\n> \n> One stat() call instead of three would be better for performance.\n\nThanks. Done.\n\n-- >8 --\nFrom: Petr Baudis \nSubject: [RFC/PATCH 2/3 v2] gitweb: Support caching projects list\n\nOn repo.or.cz (permanently I/O overloaded and hosting 1050 project +\nforks), the projects list (the default gitweb page) can take more than\na minute to generate. This naive patch adds simple support for caching\nthe projects list data structure so that all the projects do not need\nto get rescanned at every page access.\n\n$projlist_cache_lifetime gitweb configuration variable is introduced,\nby default set to zero. If set to non-zero, it describes the number of\nminutes for which the cache remains valid. Only single project root\nper system can use the cache. Any script running with the same uid as\ngitweb can change the cache trivially - this is for secure\ninstallations only.\n\nThe cache itself is stored in $cache_dir/$projlist_cache_name using\nStorable to store() Perl data structure with the list of project\ndetails. When reusing the cache, the data is retrieve()'d back into\n@projects.\n\nTo prevent contention when multiple accesses coincide with cache\nexpiration, the timeout is postponed to time()+120 when we start\nrefreshing. When showing cached version, a disclaimer is shown\nat the top of the projects list.\n\n[jn: moved from Data::Dumper to Storable for serialization of data]\n\nSigned-off-by: Petr Baudis \nSigned-off-by: Jakub Narebski \n---\n gitweb/README | 26 +++++++++++++++++++++++\n gitweb/gitweb.css | 6 +++++\n gitweb/gitweb.perl | 58 +++++++++++++++++++++++++++++++++++++++++++++++++--\n 3 files changed, 87 insertions(+), 3 deletions(-)\n\ndiff --git a/gitweb/README b/gitweb/README\nindex 2163071..efbc52e 100644\n--- a/gitweb/README\n+++ b/gitweb/README\n@@ -207,6 +207,32 @@ not include variables usually directly set during build):\n ('-M'); set it to ('-C') or ('-C', '-C') to also detect copies, or\n set it to () if you don't want to have renames detection.\n \n+Variables described below deal with caching in gitweb. If you don't\n+run gitweb installation on busy site with large number of repositories\n+(projects) you probably don't need caching; by default caching is\n+turned off.\n+ * $projlist_cache_lifetime\n+ Lifetime of in-gitweb cache for projects list page, in minutes.\n+ By default set to 0, which means tha projects list caching is\n+ turned off.\n+ * $cache_dir, $projlist_cache_name\n+ The cached list version (cache of Perl structure, not of final\n+ output) is stored in \"$cache_dir/$projlist_cache_name\". $cache_dir\n+ should be writable only by processes with the same uid as gitweb\n+ (usually web served uid); if $cache_dir does not exist gitweb would\n+ try to create it. Only single gitweb project root per system is\n+ supported, unless gitweb instances for different projects root have\n+ different configuration.\n+\n+ By default $cache_dir is set to \"$TMPDIR/gitweb\" if $TMPDIR\n+ environment variable does exist, \"/tmp/gitweb\" otherwise.\n+ Default name for $projlist_cache_name -s 'gitweb.index.cache';\n+\n+ NOTE: projects list cache file can be tweaked by other scripts\n+ running with the same uid as gitweb; use this ONLY at secure\n+ installations!!!\n+\n+\n Per-repository gitweb configuration\n ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n \ndiff --git a/gitweb/gitweb.css b/gitweb/gitweb.css\nindex 446a1c3..1e83896 100644\n--- a/gitweb/gitweb.css\n+++ b/gitweb/gitweb.css\n@@ -85,6 +85,12 @@ div.title, a.title {\n \tcolor: #000000;\n }\n \n+div.stale_info {\n+\tdisplay: block;\n+\ttext-align: right;\n+\tfont-style: italic;\n+}\n+\n div.readme {\n \tpadding: 8px;\n }\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 90ab894..0bc3f19 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -118,6 +118,20 @@ our $fallback_encoding = 'latin1';\n # - one might want to include '-B' option, e.g. '-B', '-M'\n our @diff_opts = ('-M'); # taken from git_commit\n \n+# projects list cache for busy sites with many projects;\n+# if you set this to non-zero, it will be used as the cached\n+# index lifetime in minutes\n+#\n+# the cached list version is stored in $cache_dir/$cache_name and can\n+# be tweaked by other scripts running with the same uid as gitweb -\n+# use this ONLY at secure installations; only single gitweb project\n+# root per system is supported, unless you tweak configuration!\n+our $projlist_cache_lifetime = 0; # in minutes\n+# FHS compliant $cache_dir would be \"/var/cache/gitweb\"\n+our $cache_dir =\n+\t(defined $ENV{'TMPDIR'} ? $ENV{'TMPDIR'} : '/tmp').'/gitweb';\n+our $projlist_cache_name = 'gitweb.index.cache';\n+\n # information about snapshot formats that gitweb is capable of serving\n our %known_snapshot_formats = (\n \t# name => {\n@@ -3510,16 +3524,54 @@ sub git_get_projects_details {\n }\n \n sub git_project_list_body {\n-\tmy ($projlist, $order, $from, $to, $extra, $no_header) = @_;\n+\tmy ($projlist, $order, $from, $to, $extra, $no_header, $cache_lifetime) = @_;\n \n \tmy ($check_forks) = gitweb_check_feature('forks');\n \n-\tmy @projects = git_get_projects_details($projlist, $check_forks);\n+\tuse File::stat;\n+\tuse POSIX qw(:fcntl_h);\n+\tuse Storable qw(store_fd retrieve);\n+\n+\tmy $cache_file = \"$cache_dir/$projlist_cache_name\";\n+\n+\tmy @projects;\n+\tmy $stale = 0;\n+\tmy $now = time();\n+\tmy $cache_mtime;\n+\tif ($cache_lifetime && -f $cache_file) {\n+\t\t$cache_mtime = stat($cache_file)->mtime;\n+\t}\n+\tif (defined $cache_mtime && # caching is on and $cache_file exists\n+\t $cache_mtime + $cache_lifetime*60 > $now &&\n+\t (my $dump = retrieve($cache_file))) {\n+\t\t$stale = $now - $cache_mtime;\n+\t\t@projects = @$dump;\n+\t} else {\n+\t\tif (defined $cache_mtime) {\n+\t\t\t# Postpone timeout by two minutes so that we get\n+\t\t\t# enough time to do our job, or to be more exact\n+\t\t\t# make cache expire after two minutes from now.\n+\t\t\tmy $time = $now - $cache_lifetime*60 + 120;\n+\t\t\tutime $time, $time, $cache_file;\n+\t\t}\n+\t\t@projects = git_get_projects_details($projlist, $check_forks);\n+\t\tif ($cache_lifetime &&\n+\t\t (-d $cache_dir || mkdir($cache_dir, 0700)) &&\n+\t\t sysopen(my $fd, \"$cache_file.lock\", O_WRONLY|O_CREAT|O_EXCL, 0600)) {\n+\t\t\tstore_fd(\\@projects, $fd);\n+\t\t\tclose $fd;\n+\t\t\trename \"$cache_file.lock\", $cache_file;\n+\t\t}\n+\t}\n \n \t$order ||= $default_projects_order;\n \t$from = 0 unless defined $from;\n \t$to = $#projects if (!defined $to || $#projects < $to);\n \n+\tif ($cache_lifetime && $stale > 0) {\n+\t\tprint \"
Cached version (${stale}s old)
\\n\";\n+\t}\n+\n \tprint \"
\\n\";\n \tunless ($no_header) {\n \t\tprint \"\\n\";\n@@ -3902,7 +3954,7 @@ sub git_project_list {\n \t\tclose $fd;\n \t\tprint \"\\n\";\n \t}\n-\tgit_project_list_body(\\@list, $order);\n+\tgit_project_list_body(\\@list, $order, undef, undef, undef, undef, $projlist_cache_lifetime);\n \tgit_footer_html();\n }\n \n-- \n1.5.4.4\n"},{"id":"72339","messageId":"20080318031406.GH10335@machine.or.cz","threadId":"12725","inReplyTo":"1205766570-13550-4-git-send-email-jnareb@gmail.com","subject":"Re: [RFC/PATCH 3/3] gitweb: Fill project details lazily when caching","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2008-03-18T03:14:06Z","receivedAt":"2008-03-18T03:14:06Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Mon, Mar 17, 2008 at 04:09:30PM +0100, Jakub Narebski wrote:\n> If caching is turned on project details can be filled in already from\n> the cache. When refreshing project info details for all project (when\n> cache is stale and has to be refreshed) generate projects info only if\n> modification time (as returned by lstat()) of projects repository\n> gitdir changed.\n> \n> This way we can avoid hitting repository refs, object database and\n> repository config at the cost of additional lstat.\n> \n> Signed-off-by: Jakub Narebski \n> ---\n> This is an idea for further improvement of 'projects list caching'.\n> Could you please: \n> \n> 1.) comment if it is a good idea, or why it works, or why it\n> couldn't work :), \n\nThe idea is nice, but I'm surely missing something obvious again - why\ndo you use lstat() as opposed to stat()? And more importantly, the mtime\nof projects repository unfortunately does not reflect almost any\nchanges per se; you would need to check mtime of description file,\nconfig file and the refs instead.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nWhatever you can do, or dream you can, begin it.\nBoldness has genius, power, and magic in it.\t-- J. W. von Goethe\n"},{"id":"72350","messageId":"200803181012.11273.jnareb@gmail.com","threadId":"12725","inReplyTo":"20080318031406.GH10335@machine.or.cz","subject":"Re: [RFC/PATCH 3/3] gitweb: Fill project details lazily when caching","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-03-18T09:12:09Z","receivedAt":"2008-03-18T09:12:09Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Tue, 18 March 2008, Petr Baudis wrote:\n> On Mon, Mar 17, 2008 at 04:09:30PM +0100, Jakub Narebski wrote:\n>>\n>> If caching is turned on project details can be filled in already from\n>> the cache. When refreshing project info details for all project (when\n>> cache is stale and has to be refreshed) generate projects info only if\n>> modification time (as returned by lstat()) of projects repository\n>> gitdir changed.\n>> \n>> This way we can avoid hitting repository refs, object database and\n>> repository config at the cost of additional lstat.\n>> \n>> Signed-off-by: Jakub Narebski \n>> ---\n>> This is an idea for further improvement of 'projects list caching'.\n>> Could you please: \n>> \n>> 1.) comment if it is a good idea, or why it works, or why it\n>> couldn't work :), \n> \n> The idea is nice, but I'm surely missing something obvious again - why\n> do you use lstat() as opposed to stat()?\n\nBecause in my home installation of gitweb (for tests) I have \n/home/local/scm/git.git symlinked to /home/jnareb/git/.git\nAnd I want to follow changes in repository; link itself doesn't\nchange.\n\n> And more importantly, the mtime \n> of projects repository unfortunately does not reflect almost any\n> changes per se; you would need to check mtime of description file,\n> config file and the refs instead.\n\nWell, I had hopes that because git uses \"write to temporary file, rename\ntemporary file to final name\" to have atomic file writes any change in\ngit repository would be reflected in mtime of topdir / GIT_DIR. I have\nchecked it superficially... by doing a fetch, and a commit. But while\nboth fetch and commit manipulate files in top dir (FETCH_HEAD, ORIG_HEAD,\nCOMMIT_EDITMSG) it is not the case for push, unfortunately. If all\npushes would result in pack transfer, it would be enough to watch for\nGIT_DIR/objects/pack/ directory.\n\nI think that nothing short of inotify or equivalent would work: it is\njust too many files/directories to watch for changes... I hope I am\nmistaken here...\n\n-- \nJakub Narebski\nPoland\n"},{"id":"72351","messageId":"20080318095252.GH18624@mail-vs.djpig.de","threadId":"12725","inReplyTo":"200803181012.11273.jnareb@gmail.com","subject":"Re: [RFC/PATCH 3/3] gitweb: Fill project details lazily when caching","fromName":"Frank Lichtenheld","fromEmail":"frank@lichtenheld.de","sentAt":"2008-03-18T09:52:52Z","receivedAt":"2008-03-18T09:52:52Z","isPatch":true,"sender":{"key":"frank@lichtenheld.de","avatar":"https://gravatar.com/avatar/b9f1d4b120e138f157c9e480d0818197c474628923786adb98f30017cdb99c3c?d=mp&s=160"},"body":"On Tue, Mar 18, 2008 at 10:12:09AM +0100, Jakub Narebski wrote:\n> On Tue, 18 March 2008, Petr Baudis wrote:\n> > The idea is nice, but I'm surely missing something obvious again - why\n> > do you use lstat() as opposed to stat()?\n> \n> Because in my home installation of gitweb (for tests) I have \n> /home/local/scm/git.git symlinked to /home/jnareb/git/.git\n> And I want to follow changes in repository; link itself doesn't\n> change.\n\nWhich means you have that backwards, since\n\n\"lstat() is identical to stat(), except that if path is a symbolic link,\nthen the link itself is stat-ed, not the file that it refers to.\"\n(from linux manpage)\n\nGruesse,\n-- \nFrank Lichtenheld \nwww: http://www.djpig.de/\n"}]}