{"thread":{"id":"10339","subject":"[PATCH] gitweb: speed up project listing on large work trees by limiting find depth","startedAt":"2007-10-17T03:45:25Z","lastAt":"2007-10-17T05:25:14Z","messageCount":5,"participants":["Luke Lu","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"56195","messageId":"1192592725-28143-1-git-send-email-git@vicaya.com","threadId":"10339","inReplyTo":null,"subject":"[PATCH] gitweb: speed up project listing on large work trees by limiting find depth","fromName":"Luke Lu","fromEmail":"git@vicaya.com","sentAt":"2007-10-17T03:45:25Z","receivedAt":"2007-10-17T03:45:25Z","isPatch":true,"sender":{"key":"git@vicaya.com","avatar":null},"body":"Resubmitting patch after passing gitweb regression tests.\n\nSigned-off-by: Luke Lu <git@vicaya.com>\n---\n Makefile                               |    2 ++\n gitweb/gitweb.perl                     |   10 ++++++++++\n t/t9500-gitweb-standalone-no-errors.sh |    1 +\n 3 files changed, 13 insertions(+), 0 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 8db4dbe..3e9938e 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -165,6 +165,7 @@ GITWEB_CONFIG = gitweb_config.perl\n GITWEB_HOME_LINK_STR = projects\n GITWEB_SITENAME =\n GITWEB_PROJECTROOT = /pub/git\n+GITWEB_PROJECT_MAXDEPTH = 2007\n GITWEB_EXPORT_OK =\n GITWEB_STRICT_EXPORT =\n GITWEB_BASE_URL =\n@@ -831,6 +832,7 @@ gitweb/gitweb.cgi: gitweb/gitweb.perl\n \t    -e 's|++GITWEB_HOME_LINK_STR++|$(GITWEB_HOME_LINK_STR)|g' \\\n \t    -e 's|++GITWEB_SITENAME++|$(GITWEB_SITENAME)|g' \\\n \t    -e 's|++GITWEB_PROJECTROOT++|$(GITWEB_PROJECTROOT)|g' \\\n+\t    -e 's|\"++GITWEB_PROJECT_MAXDEPTH++\"|$(GITWEB_PROJECT_MAXDEPTH)|g' \\\n \t    -e 's|++GITWEB_EXPORT_OK++|$(GITWEB_EXPORT_OK)|g' \\\n \t    -e 's|++GITWEB_STRICT_EXPORT++|$(GITWEB_STRICT_EXPORT)|g' \\\n \t    -e 's|++GITWEB_BASE_URL++|$(GITWEB_BASE_URL)|g' \\\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 3064298..48e21da 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -35,6 +35,10 @@ our $GIT = \"++GIT_BINDIR++/git\";\n #our $projectroot = \"/pub/scm\";\n our $projectroot = \"++GITWEB_PROJECTROOT++\";\n \n+# fs traversing limit for getting project list\n+# the number is relative to the projectroot\n+our $project_maxdepth = \"++GITWEB_PROJECT_MAXDEPTH++\";\n+\n # target of the home link on top of all pages\n our $home_link = $my_uri || \"/\";\n \n@@ -1509,6 +1513,7 @@ sub git_get_projects_list {\n \t\t# remove the trailing \"/\"\n \t\t$dir =~ s!/+$!!;\n \t\tmy $pfxlen = length(\"$dir\");\n+\t\tmy $pfxdepth = ($dir =~ tr!/!!);\n \n \t\tFile::Find::find({\n \t\t\tfollow_fast => 1, # follow symbolic links\n@@ -1519,6 +1524,11 @@ sub git_get_projects_list {\n \t\t\t\treturn if (m!^[/.]$!);\n \t\t\t\t# only directories can be git repositories\n \t\t\t\treturn unless (-d $_);\n+\t\t\t\t# don't traverse too deep (Find is super slow on os x)\n+\t\t\t\tif (($File::Find::name =~ tr!/!!) - $pfxdepth > $project_maxdepth) {\n+\t\t\t\t\t$File::Find::prune = 1;\n+\t\t\t\t\treturn;\n+\t\t\t\t}\n \n \t\t\t\tmy $subdir = substr($File::Find::name, $pfxlen + 1);\n \t\t\t\t# we check related file in $projectroot\ndiff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\nindex 642b836..f7bad5b 100755\n--- a/t/t9500-gitweb-standalone-no-errors.sh\n+++ b/t/t9500-gitweb-standalone-no-errors.sh\n@@ -18,6 +18,7 @@ gitweb_init () {\n our \\$version = \"current\";\n our \\$GIT = \"git\";\n our \\$projectroot = \"$(pwd)\";\n+our \\$project_maxdepth = 8;\n our \\$home_link_str = \"projects\";\n our \\$site_name = \"[localhost]\";\n our \\$site_header = \"\";\n-- \n1.5.3.4\n"},{"id":"56196","messageId":"20071017040028.GT13801@spearce.org","threadId":"10339","inReplyTo":"1192592725-28143-1-git-send-email-git@vicaya.com","subject":"Re: [PATCH] gitweb: speed up project listing on large work trees by limiting find depth","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-10-17T04:00:28Z","receivedAt":"2007-10-17T04:00:28Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Luke Lu <git@vicaya.com> wrote:\n> Resubmitting patch after passing gitweb regression tests.\n...\n> @@ -1519,6 +1524,11 @@ sub git_get_projects_list {\n>  \t\t\t\treturn if (m!^[/.]$!);\n>  \t\t\t\t# only directories can be git repositories\n>  \t\t\t\treturn unless (-d $_);\n> +\t\t\t\t# don't traverse too deep (Find is super slow on os x)\n> +\t\t\t\tif (($File::Find::name =~ tr!/!!) - $pfxdepth > $project_maxdepth) {\n> +\t\t\t\t\t$File::Find::prune = 1;\n> +\t\t\t\t\treturn;\n> +\t\t\t\t}\n\nThanks.  I'm squashing this into your patch.  I'm not sure what\nthe impact is of altering $File::Find::name in the middle of the\nfind algorithm and I'm not sure we want to figure that out later.\nWe found out the hard way today that altering a non-local'd $_\nin the function is what was causing the breakage.\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 48e21da..9f47c3f 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1525,7 +1525,8 @@ sub git_get_projects_list {\n \t\t\t\t# only directories can be git repositories\n \t\t\t\treturn unless (-d $_);\n \t\t\t\t# don't traverse too deep (Find is super slow on os x)\n-\t\t\t\tif (($File::Find::name =~ tr!/!!) - $pfxdepth > $project_maxdepth) {\n+\t\t\t\tlocal $_ = $File::Find::name;\n+\t\t\t\tif (tr!/!! - $pfxdepth > $project_maxdepth) {\n \t\t\t\t\t$File::Find::prune = 1;\n \t\t\t\t\treturn;\n \t\t\t\t}\n-- \nShawn.\n"},{"id":"56197","messageId":"6B74E96C-37ED-4D6A-8A98-C90B61EFA181@vicaya.com","threadId":"10339","inReplyTo":"20071017040028.GT13801@spearce.org","subject":"Re: [PATCH] gitweb: speed up project listing on large work trees by limiting find depth","fromName":"Luke Lu","fromEmail":"git@vicaya.com","sentAt":"2007-10-17T04:19:08Z","receivedAt":"2007-10-17T04:19:08Z","isPatch":true,"sender":{"key":"git@vicaya.com","avatar":null},"body":"\nOn Oct 16, 2007, at 9:00 PM, Shawn O. Pearce wrote:\n\n> Luke Lu <git@vicaya.com> wrote:\n>> Resubmitting patch after passing gitweb regression tests.\n> ...\n>> @@ -1519,6 +1524,11 @@ sub git_get_projects_list {\n>>  \t\t\t\treturn if (m!^[/.]$!);\n>>  \t\t\t\t# only directories can be git repositories\n>>  \t\t\t\treturn unless (-d $_);\n>> +\t\t\t\t# don't traverse too deep (Find is super slow on os x)\n>> +\t\t\t\tif (($File::Find::name =~ tr!/!!) - $pfxdepth >  \n>> $project_maxdepth) {\n>> +\t\t\t\t\t$File::Find::prune = 1;\n>> +\t\t\t\t\treturn;\n>> +\t\t\t\t}\n>\n> Thanks.  I'm squashing this into your patch.  I'm not sure what\n> the impact is of altering $File::Find::name in the middle of the\n> find algorithm and I'm not sure we want to figure that out later.\n> We found out the hard way today that altering a non-local'd $_\n> in the function is what was causing the breakage.\n\nThis is generally a good advice. But tr!/!! doesn't alter the string  \nat all (OK, replicates it), unless you use the /d option. tr/stuff//  \nis an idiom to count stuff. Check perldoc perlop for details. I don't  \nthink it's necessary.\n\n>\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 48e21da..9f47c3f 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -1525,7 +1525,8 @@ sub git_get_projects_list {\n>  \t\t\t\t# only directories can be git repositories\n>  \t\t\t\treturn unless (-d $_);\n>  \t\t\t\t# don't traverse too deep (Find is super slow on os x)\n> -\t\t\t\tif (($File::Find::name =~ tr!/!!) - $pfxdepth >  \n> $project_maxdepth) {\n> +\t\t\t\tlocal $_ = $File::Find::name;\n> +\t\t\t\tif (tr!/!! - $pfxdepth > $project_maxdepth) {\n>  \t\t\t\t\t$File::Find::prune = 1;\n>  \t\t\t\t\treturn;\n>  \t\t\t\t}\n> -- \n> Shawn.\n"},{"id":"56198","messageId":"20071017042724.GU13801@spearce.org","threadId":"10339","inReplyTo":"6B74E96C-37ED-4D6A-8A98-C90B61EFA181@vicaya.com","subject":"Re: [PATCH] gitweb: speed up project listing on large work trees by limiting find depth","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-10-17T04:27:24Z","receivedAt":"2007-10-17T04:27:24Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Luke Lu <git@vicaya.com> wrote:\n> On Oct 16, 2007, at 9:00 PM, Shawn O. Pearce wrote:\n> >\n> >Thanks.  I'm squashing this into your patch.  I'm not sure what\n> >the impact is of altering $File::Find::name in the middle of the\n> >find algorithm and I'm not sure we want to figure that out later.\n> >We found out the hard way today that altering a non-local'd $_\n> >in the function is what was causing the breakage.\n> \n> This is generally a good advice. But tr!/!! doesn't alter the string  \n> at all (OK, replicates it), unless you use the /d option. tr/stuff//  \n> is an idiom to count stuff. Check perldoc perlop for details. I don't  \n> think it's necessary.\n\nOh.  Yea, I see what you mean now.  So the bug was really that you\nwere matching on $_ not $File::Find::name.  But according to perldoc\nFile::Find $_ and $File::Find::name are the same when no_chdir =>\n1 which your patch also sets.  So I'm really not seeing how the\nupdated version fixes the bug.\n \n> >diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> >index 48e21da..9f47c3f 100755\n> >--- a/gitweb/gitweb.perl\n> >+++ b/gitweb/gitweb.perl\n> >@@ -1525,7 +1525,8 @@ sub git_get_projects_list {\n> > \t\t\t\t# only directories can be git repositories\n> > \t\t\t\treturn unless (-d $_);\n> > \t\t\t\t# don't traverse too deep (Find is super \n> > \t\t\t\tslow on os x)\n> >-\t\t\t\tif (($File::Find::name =~ tr!/!!) - \n> >$pfxdepth >  $project_maxdepth) {\n> >+\t\t\t\tlocal $_ = $File::Find::name;\n> >+\t\t\t\tif (tr!/!! - $pfxdepth > $project_maxdepth) {\n> > \t\t\t\t\t$File::Find::prune = 1;\n> > \t\t\t\t\treturn;\n> > \t\t\t\t}\n\n-- \nShawn.\n"},{"id":"56202","messageId":"20071017052514.GW13801@spearce.org","threadId":"10339","inReplyTo":"562B5254-2BE7-43DF-AB62-499458E360CC@vicaya.com","subject":"Re: [PATCH] gitweb: speed up project listing on large work trees by limiting find depth","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-10-17T05:25:14Z","receivedAt":"2007-10-17T05:25:14Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Luke Lu <git@vicaya.com> wrote:\n> OK, let me try again :) I was using no_chdir => 1 to shorten the tr,  \n> as well as saving a syscall. However the code is expecting $_ to be  \n> relative elsewhere (line 1524) to check for the toplevel, so the  \n> check failed for the toplevel because of no_chdir, which caused  \n> substr to work on the toplevel, which is $pfxlen long. Note $pfxlen +  \n> 1 passes the end of the toplevel path, hence the errors, though the  \n> program still worked correctly, as $subdir is undefined in this case,  \n> which would by pass the rest of the code, which is logically correct.  \n> It'll probably crash, if it's written in C :)\n> \n> So, I got rid of no_chdir => 1 in the new patch and uses  \n> $File::Find::name directly, as otherwise I'd have to come up with a  \n> messier regex for checking toplevel at line 1524.\n\n*light dawns*.  Thank you for the explanation.\n\n-- \nShawn.\n"}]}