{"thread":{"id":"5499","subject":"[PATCH 0/7] gitweb: Trying to improve history view speed","startedAt":"2006-09-06T13:04:40Z","lastAt":"2006-09-09T09:54:46Z","messageCount":28,"participants":["Jakub Narebski","Linus Torvalds","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"26405","messageId":"200609061504.40725.jnareb@gmail.com","threadId":"5499","inReplyTo":null,"subject":"[PATCH 0/7] gitweb: Trying to improve history view speed","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-06T13:04:40Z","receivedAt":"2006-09-06T13:04:40Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"This series of patches tries to improve gitweb speed somewhat.\n\nPatch 1/7 makes possible to easily enable/disable pickaxe search\n('pickaxe:' operator), by making pickaxe search a feature. \n\nPatch 2/7 paginates history output, which makes \"history\" view\nfor files with longer history appear much faster. Patch 7/7 fixes\nomission in pagination of history output. This patch is updated\nto newer mod_perl compatibile gitweb version, and corrected version\nof previous patch with the same title.\n\nPatch 3/7 makes it easy to make history output faster, if changing\nthe output (making output backward-incompatibile), by making it easy\nto remove '--full-history' option and/or add '--remove-empty' option.\n\nPatches 4/7, 5/7, 6/7 tries to make gitweb faster by eliminating\ncalls to git-rev-list, combining generating list of revision and\ncommit parsing into one subroutine, using one call to git-rev-list.\nUnfortunately, git-rev-list is broken: 'git rev-list \n--full-history --parents -- ' shows all merges in addition\nto what 'git rev-list --parents -- ' and \n'git rev-list --full-history -- ' shows, see\n\"git-rev-list --full-history --parents doesn't respect path limit \nand shows all merges\" thread\n Message-ID: \n http://permalink.gmane.org/gmane.comp.version-control.git/26514\nSo probably those patches should be dropped or put in freezer until\ngit-rev-list is corrected.\n\n\nBenchmark:\nFirst column is the patch number (0 means state before first patch),\ncolumns 2 to 4 are results of running gitweb from command line,\nusing /usr/bin/time -f \"%e %U %s\", columns 5 to 8 are taken from\nApacheBench 2.0.41-dev, run with -n 10 option, 5 and 6 for mod_cgi,\n7 and 8 for mod_perl (probably not configured correctly, as it is\nslower than CGI version).\n \n# 1:gitweb/new~n 2:%e 3:%U 4:%s 5:ab-n10_cgi_time[ms] 6:[+/-sd] 7:ab-n10_perl_time[ms] 8:[+/-sd]\n0 11.38 9.66 0 11350.681 96.8 11950.143 546.3\n1 11.37 9.71 0 18150.842 4327.8 14535.352 3149.1\n2 3.61 2.16 0 3719.344 261.9 3975.663 219.6\n3 3.62 2.20 0 3576.822 41.2 3929.396 201.6\n4 3.61 2.13 0 3620.246 188.3 3943.111 184.1\n4 3.61 2.13 0 3622.156 172.6 3716.499 53.0 \n#5 0/0 0/0 0/0 0/0 0/0 0/0 0/0\n6 2.60 1.56 0 2809.344 369.5 2823.286 245.9\n7 2.59 1.53 0 2621.073 234.2 2742.230 96.6\n\nShortlog:\n [PATCH 1/7] gitweb: Make pickaxe search a feature\n [PATCH 2/7] gitweb: Paginate history output\n [PATCH 3/7] gitweb: Use @hist_opts as git-rev-list parameters\n in git_history\n [PATCH 4/7] gitweb: Add parse_rev_list for later use\n [PATCH 5/7] gitweb: Use parse_rev_list in git_shortlog and git_history\n [PATCH 6/7] gitweb: Assume parsed revision list in git_shortlog_body\n and git_history_body\n [PATCH 7/7] gitweb: Set page to 0 if it is not defined, in git_history\n\nDiffstat:\n---\ngitweb/gitweb.perl | 180 +++++++++++++++++++++++++++++++++++++++++-----------\n 1 files changed, 141 insertions(+), 39 deletions(-)-\n\nP.S. Is putting diffstat in such a series of patches actually usefull?\n-- \nJakub Narebski\nShadeHawk on #git\nPoland\n"},{"id":"26408","messageId":"1157548091229-git-send-email-jnareb@gmail.com","threadId":"5499","inReplyTo":"200609061504.40725.jnareb@gmail.com","subject":"[PATCH 1/7] gitweb: Make pickaxe search a feature","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-06T13:08:05Z","receivedAt":"2006-09-06T13:08:05Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"As pickaxe search (selected using undocumented 'pickaxe:' operator in\nsearch query) is resource consuming, allow to turn it on/off using\nfeature meachanism.\n\nSigned-off-by: Jakub Narebski \n---\n gitweb/gitweb.perl | 33 +++++++++++++++++++++++++++++++--\n 1 files changed, 31 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex d6f546d..e95d16f 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -93,6 +93,11 @@ our %feature = (\n \t\t'override' => 0,\n \t\t# => [content-encoding, suffix, program]\n \t\t'default' => ['x-gzip', 'gz', 'gzip']},\n+\n+\t'pickaxe' => {\n+\t\t'sub' => \\&feature_pickaxe,\n+\t\t'override' => 0,\n+\t\t'default' => [0]},\n );\n \n sub gitweb_check_feature {\n@@ -146,6 +151,24 @@ sub feature_snapshot {\n \treturn ($ctype, $suffix, $command);\n }\n \n+# To enable system wide have in $GITWEB_CONFIG\n+# $feature{'pickaxe'}{'default'} = [1];\n+# To have project specific config enable override in $GITWEB_CONFIG\n+# $feature{'pickaxe'}{'override'} = 1;\n+# and in project config gitweb.pickaxe = 0|1;\n+\n+sub feature_pickaxe {\n+\tmy ($val) = git_get_project_config('pickaxe', '--bool');\n+\n+\tif ($val eq 'true') {\n+\t\treturn (1);\n+\t} elsif ($val eq 'false') {\n+\t\treturn (0);\n+\t}\n+\n+\treturn ($_[0]);\n+}\n+\n # rename detection options for git-diff and git-diff-tree\n # - default is '-M', with the cost proportional to\n # (number of removed files) * (number of new files).\n@@ -3131,8 +3154,7 @@ sub git_search {\n \tif (!%co) {\n \t\tdie_error(undef, \"Unknown commit object\");\n \t}\n-\t# pickaxe may take all resources of your box and run for several minutes\n-\t# with every query - so decide by yourself how public you make this feature :)\n+\n \tmy $commit_search = 1;\n \tmy $author_search = 0;\n \tmy $committer_search = 0;\n@@ -3144,6 +3166,13 @@ sub git_search {\n \t} elsif ($searchtext =~ s/^pickaxe\\\\://i) {\n \t\t$commit_search = 0;\n \t\t$pickaxe_search = 1;\n+\n+\t\t# pickaxe may take all resources of your box and run for several minutes\n+\t\t# with every query - so decide by yourself how public you make this feature\n+\t\tmy ($have_pickaxe) = gitweb_check_feature('pickaxe');\n+\t\tif (!$have_pickaxe) {\n+\t\t\tdie_error('403 Permission denied', \"Permission denied\");\n+\t\t}\n \t}\n \tgit_header_html();\n \tgit_print_page_nav('','', $hash,$co{'tree'},$hash);\n-- \n1.4.2\n"},{"id":"26406","messageId":"11575480912922-git-send-email-jnareb@gmail.com","threadId":"5499","inReplyTo":"1157548091229-git-send-email-jnareb@gmail.com","subject":"[PATCH 2/7] gitweb: Paginate history output","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-06T13:08:06Z","receivedAt":"2006-09-06T13:08:06Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"git_history output is now divided into pages, like git_shortlog,\ngit_tags and git_heads output. As whole git-rev-list output is now\nread into array before writing anything, it allows for better\nsignaling of errors.\n\nSigned-off-by: Jakub Narebski \n---\n gitweb/gitweb.perl | 61 +++++++++++++++++++++++++++++++++++++++++++---------\n 1 files changed, 51 insertions(+), 10 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex e95d16f..4c76032 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1936,12 +1936,15 @@ sub git_shortlog_body {\n \n sub git_history_body {\n \t# Warning: assumes constant type (blob or tree) during history\n-\tmy ($fd, $refs, $hash_base, $ftype, $extra) = @_;\n+\tmy ($revlist, $from, $to, $refs, $hash_base, $ftype, $extra) = @_;\n+\n+\t$from = 0 unless defined $from;\n+\t$to = $#{$revlist} unless (defined $to && $to <= $#{$revlist});\n \n \tprint \"\\n\";\n \tmy $alternate = 0;\n-\twhile (my $line = <$fd>) {\n-\t\tif ($line !~ m/^([0-9a-fA-F]{40})/) {\n+\tfor (my $i = $from; $i <= $to; $i++) {\n+\t\tif ($revlist->[$i] !~ m/^([0-9a-fA-F]{40})/) {\n \t\t\tnext;\n \t\t}\n \n@@ -3122,24 +3125,62 @@ sub git_history {\n \tif (!%co) {\n \t\tdie_error(undef, \"Unknown commit object\");\n \t}\n+\n \tmy $refs = git_get_references();\n-\tgit_header_html();\n-\tgit_print_page_nav('','', $hash_base,$co{'tree'},$hash_base);\n-\tgit_print_header_div('commit', esc_html($co{'title'}), $hash_base);\n+\tmy $limit = sprintf(\"--max-count=%i\", (100 * ($page+1)));\n+\n \tif (!defined $hash && defined $file_name) {\n \t\t$hash = git_get_hash_by_path($hash_base, $file_name);\n \t}\n \tif (defined $hash) {\n \t\t$ftype = git_get_type($hash);\n \t}\n-\tgit_print_page_path($file_name, $ftype, $hash_base);\n \n \topen my $fd, \"-|\",\n-\t\tgit_cmd(), \"rev-list\", \"--full-history\", $hash_base, \"--\", $file_name;\n+\t\tgit_cmd(), \"rev-list\", $limit, \"--full-history\", $hash_base, \"--\", $file_name\n+\t\t\tor die_error(undef, \"Open git-rev-list-failed\");\n+\tmy @revlist = map { chomp; $_ } <$fd>;\n+\tclose $fd\n+\t\tor die_error(undef, \"Reading git-rev-list failed\");\n \n-\tgit_history_body($fd, $refs, $hash_base, $ftype);\n+\tmy $paging_nav = '';\n+\tif ($page > 0) {\n+\t\t$paging_nav .=\n+\t\t\t$cgi->a({-href => href(action=>\"history\", hash=>$hash, hash_base=>$hash_base,\n+\t\t\t file_name=>$file_name)},\n+\t\t\t \"first\");\n+\t\t$paging_nav .= \" ⋅ \" .\n+\t\t\t$cgi->a({-href => href(action=>\"history\", hash=>$hash, hash_base=>$hash_base,\n+\t\t\t file_name=>$file_name, page=>$page-1),\n+\t\t\t -accesskey => \"p\", -title => \"Alt-p\"}, \"prev\");\n+\t} else {\n+\t\t$paging_nav .= \"first\";\n+\t\t$paging_nav .= \" ⋅ prev\";\n+\t}\n+\tif ($#revlist >= (100 * ($page+1)-1)) {\n+\t\t$paging_nav .= \" ⋅ \" .\n+\t\t\t$cgi->a({-href => href(action=>\"history\", hash=>$hash, hash_base=>$hash_base,\n+\t\t\t file_name=>$file_name, page=>$page+1),\n+\t\t\t -accesskey => \"n\", -title => \"Alt-n\"}, \"next\");\n+\t} else {\n+\t\t$paging_nav .= \" ⋅ next\";\n+\t}\n+\tmy $next_link = '';\n+\tif ($#revlist >= (100 * ($page+1)-1)) {\n+\t\t$next_link =\n+\t\t\t$cgi->a({-href => href(action=>\"history\", hash=>$hash, hash_base=>$hash_base,\n+\t\t\t file_name=>$file_name, page=>$page+1),\n+\t\t\t -title => \"Alt-n\"}, \"next\");\n+\t}\n+\n+\tgit_header_html();\n+\tgit_print_page_nav('history','', $hash_base,$co{'tree'},$hash_base, $paging_nav);\n+\tgit_print_header_div('commit', esc_html($co{'title'}), $hash_base);\n+\tgit_print_page_path($file_name, $ftype, $hash_base);\n+\n+\tgit_history_body(\\@revlist, ($page * 100), $#revlist,\n+\t $refs, $hash_base, $ftype, $next_link);\n \n-\tclose $fd;\n \tgit_footer_html();\n }\n \n-- \n1.4.2\n"},{"id":"26409","messageId":"11575480922090-git-send-email-jnareb@gmail.com","threadId":"5499","inReplyTo":"11575480912922-git-send-email-jnareb@gmail.com","subject":"[PATCH 3/7] gitweb: Use @hist_opts as git-rev-list parameters in git_history","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-06T13:08:07Z","receivedAt":"2006-09-06T13:08:07Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Add new global configuration variable @hist_opts, which holds\nadditional, history specific options (parameters) to git-rev-list\ncalled in git_history subroutine. Default value is '--full-history',\nas it was.\n\nSigned-off-by: Jakub Narebski \n---\n gitweb/gitweb.perl | 18 ++++++++++++++++--\n 1 files changed, 16 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 4c76032..2191853 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -176,9 +176,23 @@ # - more costly is '-C' (or '-C', '-M'),\n # (number of changed files + number of removed files) * (number of new files)\n # - even more costly is '-C', '--find-copies-harder' with cost\n # (number of files in the original tree) * (number of new files)\n-# - one might want to include '-B' option, e.g. '-B', '-M'\n+# - one might want to include '-B' option, e.g. have it ('-B', '-M'),\n+# or for example '-l' together with '-M' and perhaps '-C'\n our @diff_opts = ('-M'); # taken from git_commit\n \n+# options fo git-rev-list used in git_history\n+# - default is '--full-history', which is slowest but works with merges,\n+# and was done to not change the output from previous version when\n+# path limiting was done by piping revisions to git-diff-tree --stdin\n+# - less costly is (), i.e. without '--full-history', which of course\n+# changes output and provides _simplified_ history of a file\n+# - for files which appeared late in the history less costly is\n+# --full-history, --remove-empty, although it changes output in\n+# the rare case when name vanished the appeared thorough the history;\n+# it improves performance of course only the last page of history\n+# - least costly, but changing output, is having --remove-mepty only\n+our @hist_opts = ('--full-history');\n+\n our $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || \"++GITWEB_CONFIG++\";\n do $GITWEB_CONFIG if -e $GITWEB_CONFIG;\n \n@@ -3137,7 +3151,7 @@ sub git_history {\n \t}\n \n \topen my $fd, \"-|\",\n-\t\tgit_cmd(), \"rev-list\", $limit, \"--full-history\", $hash_base, \"--\", $file_name\n+\t\tgit_cmd(), \"rev-list\", $limit, @hist_opts, $hash_base, \"--\", $file_name\n \t\t\tor die_error(undef, \"Open git-rev-list-failed\");\n \tmy @revlist = map { chomp; $_ } <$fd>;\n \tclose $fd\n-- \n1.4.2\n"},{"id":"26410","messageId":"11575480922634-git-send-email-jnareb@gmail.com","threadId":"5499","inReplyTo":"11575480922090-git-send-email-jnareb@gmail.com","subject":"[PATCH 4/7] gitweb: Add parse_rev_list for later use","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-06T13:08:08Z","receivedAt":"2006-09-06T13:08:08Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Add parse_rev_list to generate _parsed_ list of revisions, combining\ngetting the list of revisions, and parsing of individual revisions\ninto one subroutine. It is to avoid code like below\n\n\topen my $fd, \"-|\", git_cmd(), \"rev-list\", $limit, $hash\n\t\tor die_error(undef, \"Open git-rev-list failed\");\n\tmy @revlist = map { chomp; $_ } <$fd>;\n\n\t...\n\n\tforeach my $commit (@revlist) {\n\t\tmy %co = parse_commit($commit);\n\nwhere parse_commit subroutine calls git-rev-list with '--max-count=1'\nto parse individual commit. Using parse_rev_list will avoid\nunnecessary forks.\n\nSigned-off-by: Jakub Narebski \n---\n gitweb/gitweb.perl | 29 +++++++++++++++++++++++++++++\n 1 files changed, 29 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 2191853..8aeca52 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1125,6 +1125,35 @@ sub git_get_refs_list {\n \treturn \\@reflist;\n }\n \n+# To use only one invocation of git-rev-list, instead of getting\n+# the list of revisions and then using git-rev-list per revision\n+# to parse individual commits.\n+#\n+# parse_rev_list parameters are passed to git-rev-list, so they should\n+# include at least starting revision; just in case we default to HEAD\n+sub parse_rev_list {\n+\tmy @rev_opts = @_;\n+\tmy @revlist;\n+\n+\t@rev_opts = (\"HEAD\") unless @rev_opts;\n+\n+\tlocal $/ = \"\\0\";\n+\topen my $fd, \"-|\", git_cmd(), \"rev-list\", \"--header\", \"--parents\", @rev_opts\n+\t\tor return \\@revlist;\n+\n+\twhile (my $revinfo = <$fd>) {\n+\t\tchomp $revinfo;\n+\t\tmy @commit_lines = split '\\n', $revinfo;\n+\t\tmy %co = parse_commit(undef, \\@commit_lines);\n+\n+\t\tpush @revlist, \\%co;\n+\t}\n+\n+\tclose $fd;\n+\n+\treturn wantarray ? @revlist : \\@revlist;\n+}\n+\n ## ----------------------------------------------------------------------\n ## filesystem-related functions\n \n-- \n1.4.2\n"},{"id":"26407","messageId":"11575480921279-git-send-email-jnareb@gmail.com","threadId":"5499","inReplyTo":"11575480922634-git-send-email-jnareb@gmail.com","subject":"[PATCH 5/7] gitweb: Use parse_rev_list in git_shortlog and git_history","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-06T13:08:09Z","receivedAt":"2006-09-06T13:08:09Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Use parse_rev_list in git_shortlog and git_history, and modify\ngit_shortlog_body and git_history_body to accept parse_rev_list\noutput; in the future we can remove support for older unparsed\nrevision list from git_shortlog_body and git_history_body.\n\nOther places when we can use parse_rev_list are git_log and git_rss.\n\nSigned-off-by: Jakub Narebski \n---\n gitweb/gitweb.perl | 23 ++++++++++-------------\n 1 files changed, 10 insertions(+), 13 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 8aeca52..e665d94 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1946,7 +1946,7 @@ sub git_shortlog_body {\n \t\tmy $commit = $revlist->[$i];\n \t\t#my $ref = defined $refs ? format_ref_marker($refs, $commit) : '';\n \t\tmy $ref = format_ref_marker($refs, $commit);\n-\t\tmy %co = parse_commit($commit);\n+\t\tmy %co = ref $commit ? %$commit : parse_commit($commit);\n \t\tif ($alternate) {\n \t\t\tprint \"\\n\";\n \t\t} else {\n@@ -1987,12 +1987,13 @@ sub git_history_body {\n \tprint \"
\\n\";\n \tmy $alternate = 0;\n \tfor (my $i = $from; $i <= $to; $i++) {\n-\t\tif ($revlist->[$i] !~ m/^([0-9a-fA-F]{40})/) {\n+\t\tif (ref($revlist->[$i]) ne \"HASH\" &&\n+\t\t $revlist->[$i] !~ m/^([0-9a-fA-F]{40})/) {\n \t\t\tnext;\n \t\t}\n \n \t\tmy $commit = $1;\n-\t\tmy %co = parse_commit($commit);\n+\t\tmy %co = ref $commit ? %$commit : parse_commit($commit);\n \t\tif (!%co) {\n \t\t\tnext;\n \t\t}\n@@ -3179,12 +3180,9 @@ sub git_history {\n \t\t$ftype = git_get_type($hash);\n \t}\n \n-\topen my $fd, \"-|\",\n-\t\tgit_cmd(), \"rev-list\", $limit, @hist_opts, $hash_base, \"--\", $file_name\n-\t\t\tor die_error(undef, \"Open git-rev-list-failed\");\n-\tmy @revlist = map { chomp; $_ } <$fd>;\n-\tclose $fd\n-\t\tor die_error(undef, \"Reading git-rev-list failed\");\n+\tmy @revlist = parse_rev_list($limit, @hist_opts, $hash_base, \"--\", $file_name)\n+\t\tor die_error(undef, \"Parsing git-rev-list failed\");\n+\n \n \tmy $paging_nav = '';\n \tif ($page > 0) {\n@@ -3387,10 +3385,9 @@ sub git_shortlog {\n \tmy $refs = git_get_references();\n \n \tmy $limit = sprintf(\"--max-count=%i\", (100 * ($page+1)));\n-\topen my $fd, \"-|\", git_cmd(), \"rev-list\", $limit, $hash\n-\t\tor die_error(undef, \"Open git-rev-list failed\");\n-\tmy @revlist = map { chomp; $_ } <$fd>;\n-\tclose $fd;\n+\tmy @revlist = parse_rev_list($limit, $hash)\n+\t\tor die_error(undef, \"Parsing git-rev-list failed\");\n+\n \n \tmy $paging_nav = format_paging_nav('shortlog', $hash, $head, $page, $#revlist);\n \tmy $next_link = '';\n-- \n1.4.2\n"},{"id":"26412","messageId":"11575480922345-git-send-email-jnareb@gmail.com","threadId":"5499","inReplyTo":"11575480921279-git-send-email-jnareb@gmail.com","subject":"[PATCH 6/7] gitweb: Assume parsed revision list in git_shortlog_body and git_history_body","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-06T13:08:10Z","receivedAt":"2006-09-06T13:08:10Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Assume that git_shortlog and git_history uses parse_rev_list\nsubroutine, and git_shortlog_body and git_history_body gets parsed\nrevision list as a parameter.\n\nSigned-off-by: Jakub Narebski \n---\n gitweb/gitweb.perl | 35 ++++++++++++-----------------------\n 1 files changed, 12 insertions(+), 23 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex e665d94..be4db08 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1943,21 +1943,19 @@ sub git_shortlog_body {\n \tprint \"
\\n\";\n \tmy $alternate = 0;\n \tfor (my $i = $from; $i <= $to; $i++) {\n-\t\tmy $commit = $revlist->[$i];\n-\t\t#my $ref = defined $refs ? format_ref_marker($refs, $commit) : '';\n+\t\tmy $co = $revlist->[$i];\n+\t\tmy $commit = $co->{'id'};\n \t\tmy $ref = format_ref_marker($refs, $commit);\n-\t\tmy %co = ref $commit ? %$commit : parse_commit($commit);\n \t\tif ($alternate) {\n \t\t\tprint \"\\n\";\n \t\t} else {\n \t\t\tprint \"\\n\";\n \t\t}\n \t\t$alternate ^= 1;\n-\t\t# git_summary() used print \"\\n\" .\n-\t\tprint \"\\n\" .\n-\t\t \"\\n\" .\n+\t\tprint \"\\n\" .\n+\t\t \"\\n\" .\n \t\t \"\\n\" .\n \t\t \"
$co{'age_string'}$co{'age_string_date'}\" . esc_html(chop_str($co{'author_name'}, 10)) . \"{'age_string_age'}\\\">$co->{'age_string_date'}\" . esc_html(chop_str($co->{'author_name'}, 10)) . \"\";\n-\t\tprint format_subject_html($co{'title'}, $co{'title_short'},\n+\t\tprint format_subject_html($co->{'title'}, $co->{'title_short'},\n \t\t href(action=>\"commit\", hash=>$commit), $ref);\n \t\tprint \"\" .\n@@ -1987,17 +1985,8 @@ sub git_history_body {\n \tprint \"\\n\";\n \tmy $alternate = 0;\n \tfor (my $i = $from; $i <= $to; $i++) {\n-\t\tif (ref($revlist->[$i]) ne \"HASH\" &&\n-\t\t $revlist->[$i] !~ m/^([0-9a-fA-F]{40})/) {\n-\t\t\tnext;\n-\t\t}\n-\n-\t\tmy $commit = $1;\n-\t\tmy %co = ref $commit ? %$commit : parse_commit($commit);\n-\t\tif (!%co) {\n-\t\t\tnext;\n-\t\t}\n-\n+\t\tmy $co = $revlist->[$i];\n+\t\tmy $commit = $co->{'id'};\n \t\tmy $ref = format_ref_marker($refs, $commit);\n \n \t\tif ($alternate) {\n@@ -2006,12 +1995,12 @@ sub git_history_body {\n \t\t\tprint \"\\n\";\n \t\t}\n \t\t$alternate ^= 1;\n-\t\tprint \"\\n\" .\n-\t\t # shortlog uses chop_str($co{'author_name'}, 10)\n-\t\t \"\\n\" .\n+\t\tprint \"\\n\" .\n+\t\t # shortlog uses chop_str($co->{'author_name'}, 10)\n+\t\t \"\\n\" .\n \t\t \"\\n\" .\n \t\t \"
$co{'age_string_date'}\" . esc_html(chop_str($co{'author_name'}, 15, 3)) . \"{'age_string_age'}\\\">$co->{'age_string_date'}\" . esc_html(chop_str($co->{'author_name'}, 15, 3)) . \"\";\n-\t\t# originally git_history used chop_str($co{'title'}, 50)\n-\t\tprint format_subject_html($co{'title'}, $co{'title_short'},\n+\t\t# originally git_history used chop_str($co->{'title'}, 50)\n+\t\tprint format_subject_html($co->{'title'}, $co->{'title_short'},\n \t\t href(action=>\"commit\", hash=>$commit), $ref);\n \t\tprint \"\" .\n-- \n1.4.2\n"},{"id":"26411","messageId":"11575480921132-git-send-email-jnareb@gmail.com","threadId":"5499","inReplyTo":"11575480922345-git-send-email-jnareb@gmail.com","subject":"[PATCH 7/7] gitweb: Set page to 0 if it is not defined, in git_history","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-06T13:08:11Z","receivedAt":"2006-09-06T13:08:11Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Signed-off-by: Jakub Narebski \n---\n gitweb/gitweb.perl | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex be4db08..edded74 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3153,6 +3153,9 @@ sub git_history {\n \tif (!defined $hash_base) {\n \t\t$hash_base = git_get_head_hash($project);\n \t}\n+\tif (!defined $page) {\n+\t\t$page = 0;\n+\t}\n \tmy $ftype;\n \tmy %co = parse_commit($hash_base);\n \tif (!%co) {\n-- \n1.4.2\n"},{"id":"26421","messageId":"Pine.LNX.4.64.0609060847521.27779@g5.osdl.org","threadId":"5499","inReplyTo":"200609061504.40725.jnareb@gmail.com","subject":"Re: [PATCH 0/7] gitweb: Trying to improve history view speed","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-09-06T15:57:24Z","receivedAt":"2006-09-06T15:57:24Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 6 Sep 2006, Jakub Narebski wrote:\n>\n> Unfortunately, git-rev-list is broken: 'git rev-list \n> --full-history --parents -- ' shows all merges in addition\n> to what 'git rev-list --parents -- ' and \n> 'git rev-list --full-history -- ' shows, see\n> \"git-rev-list --full-history --parents doesn't respect path limit \n> and shows all merges\" thread\n\nIf you ask for \"--full-history\" and \"--parents\", then pretty much by \n_definition_ you need every single merge, because otherwise your history \nwouldn't be fully connected.\n\nWithout that, things like \"gitk\" and \"qgit\" wouldn't work.\n\n> So probably those patches should be dropped or put in freezer until\n> git-rev-list is corrected.\n\ngit-rev-list _is_ correct, and if you want something else, you need to \neither use a different set of flags (like _only_ using \"--full-history\") \nor ask for a totally new flag (like \"--most-history\").\n\nSo the rule is:\n\n - using \"--full-history\" + \"--parents\" means that you want (surprise \n surprise) full history with parenthood, which means that you get all \n the connecting merges too. And since you asked for the _full_ history, \n that means EVERY SINGLE MERGE.\n\n - using _just_ \"--parents\" means that you want a connected history with \n parenthood information, but since you didn't ask for the _full_ \n history, it will optimize away the merges that didn't change the file, \n and only follow the changed side. You still get merges, but now you get \n only those merges where both (all) sides actually mattered.\n\n - using _just_ \"--full-history\" (without asking for parenthood) means \n that you're not asking for a connected history (since you're not asking \n for parents), and as such, it will only show individual _commits_ that \n change the file. That does potentially include merges, but again, it \n only includes merges that actually _changed_ something.\n\nIn other words, \"--parents\" means a lot more than just \"show what the \nparents\" were. In particular, it means (and always has meant, apart from \nbugs) that we show the _rewritten_ parents after we've done history \nmunging, and that we always output enough commits to actually make sense \nof that history from the result.\n\nSo what you are asking for is pretty nonsensical. You ask for parenthood \ninfo, but then you seem to not want to actually connect the dots. So why \ndo you ask for parents in the first place? If you don't want to connect \nthe commits to their parents, you shouldn't ask for it.\n\n\t\tLinus\n"},{"id":"26425","messageId":"edmv57$lt7$1@sea.gmane.org","threadId":"5499","inReplyTo":"Pine.LNX.4.64.0609060847521.27779@g5.osdl.org","subject":"Re: [PATCH 0/7] gitweb: Trying to improve history view speed","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-06T17:06:04Z","receivedAt":"2006-09-06T17:06:04Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Linus Torvalds wrote:\n\n> So the rule is:\n> \n> - using \"--full-history\" + \"--parents\" means that you want (surprise \n> surprise) full history with parenthood, which means that you get all \n> the connecting merges too. And since you asked for the _full_ history, \n> that means EVERY SINGLE MERGE.\n\nI just don't quite understand where filtering takes place \nin this case.\n\nEvery single merge is for parents to be connected, or what?\n \n> So what you are asking for is pretty nonsensical. You ask for parenthood \n> info, but then you seem to not want to actually connect the dots. So why \n> do you ask for parents in the first place? If you don't want to connect \n> the commits to their parents, you shouldn't ask for it.\n\nWhen I though about it, git_history needs not parents of a commit; from\nparsed commit it needs only date, author and title/summary (and of course\ncommit sha1), so we can skip '--parents' option to git-rev-list, and have\nnice _history of a file_, using only one call to git-rev-list to make it.\n\nThis means that parse_rev_list has to be changes, and that '--parents'\noption must be specified explicitely as argument to parse_rev_list.\n\nBesides [primitive] benchmarking shows that there we gain only a little:\n1.53s user+sys vs 2.13s user+sys when run from command line, \n2.8s mean vs 3.6s mean when tested using ApacheBench...\nthat is _after_ paginating history output.\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"26431","messageId":"Pine.LNX.4.64.0609061125000.27779@g5.osdl.org","threadId":"5499","inReplyTo":"edmv57$lt7$1@sea.gmane.org","subject":"Re: [PATCH 0/7] gitweb: Trying to improve history view speed","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-09-06T18:30:59Z","receivedAt":"2006-09-06T18:30:59Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 6 Sep 2006, Jakub Narebski wrote:\n\n> Linus Torvalds wrote:\n> \n> > So the rule is:\n> > \n> > - using \"--full-history\" + \"--parents\" means that you want (surprise \n> > surprise) full history with parenthood, which means that you get all \n> > the connecting merges too. And since you asked for the _full_ history, \n> > that means EVERY SINGLE MERGE.\n> \n> I just don't quite understand where filtering takes place \n> in this case.\n\nWell, since you asked for full history, by definition it doesn't take \nplace for merges, since you'll be wanting to follow both sides of the \nmerge (to see the full history) regardless of whether the parents of that \nmerge seem interesting or not.\n\n> Every single merge is for parents to be connected, or what?\n\nWell, \"--parents\" on its own means that we want a connected graph. It's \njust that if you don't ask for full history, then the connected graph \nresult is obviously much smaller.\n\nThink \"graphical visualizer\", and you'll see what's going on. Do a\n\n\tgitk git.c\n\tgitk --full-history git.c\n\nand see the difference, and you'll understand (gitk already asks for \n\"--parents\" on its own).\n\nBasically, \"--full-history\" means that we traverse every parent of a \nmerge, whether it looks interesting or not.\n\n> When I though about it, git_history needs not parents of a commit; from\n> parsed commit it needs only date, author and title/summary (and of course\n> commit sha1), so we can skip '--parents' option to git-rev-list, and have\n> nice _history of a file_, using only one call to git-rev-list to make it.\n\nYes. Once you get rid of \"--parents\", git-rev-list should indeed do \nexactly what you want (because it no longer tries to keep things \nconnected, and thus happily drops uninteresting merges).\n\nOf course, there could well be a bug _there_, since \"--full-history\" isn't \nused very much (but \"git whatchanged\" uses it, so it should have gotten \n_some_ testing).\n\n\t\t\tLinus\n"},{"id":"26433","messageId":"edn54s$c4s$1@sea.gmane.org","threadId":"5499","inReplyTo":"Pine.LNX.4.64.0609061125000.27779@g5.osdl.org","subject":"Re: [PATCH 0/7] gitweb: Trying to improve history view speed","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-06T18:48:18Z","receivedAt":"2006-09-06T18:48:18Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Linus Torvalds wrote:\n\n>> Every single merge is for parents to be connected, or what?\n> \n> Well, \"--parents\" on its own means that we want a connected graph. It's \n> just that if you don't ask for full history, then the connected graph \n> result is obviously much smaller.\n\nWell, I just didn't realize that --parents gives parents in _simplified_\nhistory, unless --full-history is used. Hence my confusion.\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"26435","messageId":"Pine.LNX.4.64.0609061202230.27779@g5.osdl.org","threadId":"5499","inReplyTo":"edn54s$c4s$1@sea.gmane.org","subject":"Re: [PATCH 0/7] gitweb: Trying to improve history view speed","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-09-06T19:04:32Z","receivedAt":"2006-09-06T19:04:32Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 6 Sep 2006, Jakub Narebski wrote:\n> \n> Well, I just didn't realize that --parents gives parents in _simplified_\n> history, unless --full-history is used. Hence my confusion.\n\nRight. That's really the main reason for \"--parents\" existing in the first \nplace. I added it exactly so that \"gitk\" would work with pathname \nlimiting.\n\nIf you don't want the simplified history parents, you might as well just \nparse the parents information directly from the commit data yourself \n(although with grafting, there's arguably _some_ reason to have \ngit-rev-list do it for you regardless of simplification).\n\n\t\t\tLinus\n"},{"id":"26443","messageId":"11575761821830-git-send-email-jnareb@gmail.com","threadId":"5499","inReplyTo":"11575480921132-git-send-email-jnareb@gmail.com","subject":"[PATCH 8/8] gitweb: Remove --parents from call to git-rev-list in parse_rev_list","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-06T20:56:22Z","receivedAt":"2006-09-06T20:56:22Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"This option is removed by default, because with --full-history option\n(which is by default used in git_history) --parents means full history\nwith parenthood, which means that we get all the connecting merges\ntoo. And since we asked for the _full_ history, that means EVERY\nSINGLE MERGE. Even those that do not change given file (or\ndirectory).\n\nSigned-off-by: Jakub Narebski \n---\nBenchmarks (7 means patch before, 8 means this patch):\n# 1:gitweb/new~n 2:%e 3:%U 4:%s 5:ab-n10_cgi_time[ms] 6:[+/-sd] 7:ab-n10_perl_time[ms] 8:[+/-sd]\n7 2.59 1.53 0 2621.073 234.2 2742.230 96.6\n8 2.89 1.80 0 3081.722 246.6 3306.196 367.2\n8 2.95 1.81 0 2952.253 155.9 3175.441 128.0\n\n gitweb/gitweb.perl | 9 ++++++++-\n 1 files changed, 8 insertions(+), 1 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex edded74..68ddbe6 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1131,6 +1131,13 @@ # to parse individual commits.\n #\n # parse_rev_list parameters are passed to git-rev-list, so they should\n # include at least starting revision; just in case we default to HEAD\n+#\n+# if you want in parsed info to have full equivalent of first generating\n+# list of interesting revisions, then calling parse_commit on each of revs,\n+# i.e. if you want to have full parent info which includes grafts,\n+# you have to include '--parents' in the parse_rev_list parameters;\n+# it is excluded by default as it changes notion which revs are interesting\n+# e.g. '--full-history' with '--parents' include EVERY SINGLE MERGE.\n sub parse_rev_list {\n \tmy @rev_opts = @_;\n \tmy @revlist;\n@@ -1138,7 +1145,7 @@ sub parse_rev_list {\n \t@rev_opts = (\"HEAD\") unless @rev_opts;\n \n \tlocal $/ = \"\\0\";\n-\topen my $fd, \"-|\", git_cmd(), \"rev-list\", \"--header\", \"--parents\", @rev_opts\n+\topen my $fd, \"-|\", git_cmd(), \"rev-list\", \"--header\", @rev_opts\n \t\tor return \\@revlist;\n \n \twhile (my $revinfo = <$fd>) {\n-- \n1.4.2\n"},{"id":"26444","messageId":"Pine.LNX.4.64.0609061404490.27779@g5.osdl.org","threadId":"5499","inReplyTo":"11575761821830-git-send-email-jnareb@gmail.com","subject":"Re: [PATCH 8/8] gitweb: Remove --parents from call to git-rev-list in parse_rev_list","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-09-06T21:08:18Z","receivedAt":"2006-09-06T21:08:18Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 6 Sep 2006, Jakub Narebski wrote:\n>\n> Benchmarks (7 means patch before, 8 means this patch):\n\nBtw, you should possibly look at cold-cache numbers, and numbers for \nprojects that aren't fully packed. They can often be _dramatically_ \ndifferent.\n\nThat said, the dramatic change would probably be if there were some way to \navoid using \"--full-history\" (rather than \"--parents\", which doesn't add \n_that_ much overhead), since that \"follow all parents\" behaviour of \nfull-history is usually what really makes a big deal.\n\nBut I guess for gitweb, you do want to use --full-history in this case ;(\n\n\t\t\tLinus\n"},{"id":"26445","messageId":"edndud$csb$1@sea.gmane.org","threadId":"5499","inReplyTo":"Pine.LNX.4.64.0609061404490.27779@g5.osdl.org","subject":"Re: [PATCH 8/8] gitweb: Remove --parents from call to git-rev-list in parse_rev_list","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-06T21:18:26Z","receivedAt":"2006-09-06T21:18:26Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Linus Torvalds wrote:\n\n> On Wed, 6 Sep 2006, Jakub Narebski wrote:\n>>\n>> Benchmarks (7 means patch before, 8 means this patch):\n> \n> Btw, you should possibly look at cold-cache numbers, and numbers for \n> projects that aren't fully packed. They can often be _dramatically_ \n> different.\n\nBy the way I forgot that the case 8 is for the repository with 1 commit\nmore, although that shouldn't matter much for paginated output. Still, it\nis one commit more unpacked. Benchmark for 7 was also for partially packed\nrepository.\n \n> That said, the dramatic change would probably be if there were some way to \n> avoid using \"--full-history\" (rather than \"--parents\", which doesn't add \n> _that_ much overhead), since that \"follow all parents\" behaviour of \n> full-history is usually what really makes a big deal.\n> \n> But I guess for gitweb, you do want to use --full-history in this case ;(\n\nIt is now easy with patch 3/7 \"Use @hist_opts as git-rev-list parameters in\ngit_history\" to remove '--full-history' from git-rev-list parameters in\ngit_history subroutine. Or add '--remove-empty' which matters only for the\nlast page of file/directory history output.\n\nI had some simple benchmark that shown that the earlier version with\nfiltering via piping git-rev-list to git-diff-tree --stdin -- \nwas slightly faster than git-rev-list --full-history -- \n(current version). If I remember correctly of course. And this version can\nbe easily extended to include renames (but not file to directory changes).\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"26450","messageId":"7vy7sw64vp.fsf@assigned-by-dhcp.cox.net","threadId":"5499","inReplyTo":"edndud$csb$1@sea.gmane.org","subject":"Re: [PATCH 8/8] gitweb: Remove --parents from call to git-rev-list in parse_rev_list","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-06T21:51:06Z","receivedAt":"2006-09-06T21:51:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> Linus Torvalds wrote:\n>\n>> But I guess for gitweb, you do want to use --full-history in this case ;(\n>\n> It is now easy with patch 3/7 \"Use @hist_opts as git-rev-list parameters in\n> git_history\" to remove '--full-history' from git-rev-list parameters in\n> git_history subroutine.\n\nWell, generating not-so-useful output fast is, eh, not-so-useful\nanyway, so being able to add or not add to the set of options on\na whim is not so much of value. The real value lies in deciding\nif it is needed (and I think --full-history is needed for gitweb)\nand if so coming up with a way to get the same result with\ncheaper operations.\n"},{"id":"26451","messageId":"edng0d$jng$1@sea.gmane.org","threadId":"5499","inReplyTo":"11575761821830-git-send-email-jnareb@gmail.com","subject":"Re: [PATCH 8/8] gitweb: Remove --parents from call to git-rev-list in parse_rev_list","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-06T21:53:38Z","receivedAt":"2006-09-06T21:53:38Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Jakub Narebski wrote:\n\n> Benchmarks (7 means patch before, 8 means this patch):\n> # 1:gitweb/new~n 2:%e 3:%U 4:%s 5:ab-n10_cgi_time[ms] 6:[+/-sd] 7:ab-n10_perl_time[ms] 8:[+/-sd]\n> 7 2.59  1.53 0  2621.073  234.2  2742.230   96.6\n> 8 2.89  1.80 0  3081.722  246.6  3306.196  367.2\n> 8 2.95  1.81 0  2952.253  155.9  3175.441  128.0\n\nCorrected benchmark (on the same state of repository) are\n\n7 2.67 1.60 0 2694.654 275.2 2866.028 168.4\n8 2.84 1.86 0 2892.864 263.0 3135.065 68.1\n\nwhere the benchmarking commands are\n$ /usr/bin/time -f \"%e %U %s\" \\\n GATEWAY_INTERFACE=\"CGI/1.1\" HTTP_ACCEPT=\"*/*\" REQUEST_METHOD=\"GET\" \\\n QUERY_STRING=\"p=git.git;a=history;f=gitweb/gitweb.perl\" \\\n GITWEB_CONFIG=\"~/git/gitweb/gitweb_config.perl \\\n perl -- ~/git/gitweb/gitweb.perl\n$ ab -n 10 \"http://localhost/cgi-bin/gitweb/gitweb.cgi?p=git.git;a=history;f=gitweb/gitweb.perl\"\n$ ab -n 10 \"http://localhost/perl/gitweb/gitweb.cgi?p=git.git;a=history;f=gitweb/gitweb.perl\"\n\nand we get version 7 using\n$ git checkout HEAD^ -- gitweb/gitweb.perl\nand updating gitweb.cgi in correct place.\n\nSo actually removing '--parents' option did make gitweb _slower_ (sic!),\nif only slightly.\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"26452","messageId":"7vu03k64eu.fsf@assigned-by-dhcp.cox.net","threadId":"5499","inReplyTo":"200609061504.40725.jnareb@gmail.com","subject":"Re: [PATCH 0/7] gitweb: Trying to improve history view speed","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-06T22:01:13Z","receivedAt":"2006-09-06T22:01:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> gitweb/gitweb.perl | 180 +++++++++++++++++++++++++++++++++++++++++-----------\n> 1 files changed, 141 insertions(+), 39 deletions(-)-\n>\n> P.S. Is putting diffstat in such a series of patches actually usefull?\n\nVery much -- it shows me that you are not damaging the core but\nonly touching gitweb.\n"},{"id":"26461","messageId":"7vlkow5x77.fsf@assigned-by-dhcp.cox.net","threadId":"5499","inReplyTo":"1157548091229-git-send-email-jnareb@gmail.com","subject":"Re: [PATCH 1/7] gitweb: Make pickaxe search a feature","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-07T00:37:00Z","receivedAt":"2006-09-07T00:37:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> As pickaxe search (selected using undocumented 'pickaxe:' operator in\n> search query) is resource consuming, allow to turn it on/off using\n> feature meachanism.\n>\n> Signed-off-by: Jakub Narebski \n\nI do not have a problem against making it configurable.\n\n> +\t'pickaxe' => {\n> +\t\t'sub' => \\&feature_pickaxe,\n> +\t\t'override' => 0,\n> +\t\t'default' => [0]},\n> );\n\nThe patch suggests that it is turned off by default right now; I\nhave not checked it myself, but is that the case?\n"},{"id":"26499","messageId":"edoli0$oql$1@sea.gmane.org","threadId":"5499","inReplyTo":"7vlkow5x77.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/7] gitweb: Make pickaxe search a feature","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-07T08:34:33Z","receivedAt":"2006-09-07T08:34:33Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n\n> Jakub Narebski writes:\n> \n>> As pickaxe search (selected using undocumented 'pickaxe:' operator in\n>> search query) is resource consuming, allow to turn it on/off using\n>> feature meachanism.\n> \n> I do not have a problem against making it configurable.\n> \n>> + 'pickaxe' => {\n>> + 'sub' => \\&feature_pickaxe,\n>> + 'override' => 0,\n>> + 'default' => [0]},\n>> );\n> \n> The patch suggests that it is turned off by default right now; I\n> have not checked it myself, but is that the case?\n\nNo, it is not. Currently it is turned on, _but_ undocumented.\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"26500","messageId":"edolqe$oql$2@sea.gmane.org","threadId":"5499","inReplyTo":"edng0d$jng$1@sea.gmane.org","subject":"Re: [PATCH 8/8] gitweb: Remove --parents from call to git-rev-list in parse_rev_list","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-07T08:39:02Z","receivedAt":"2006-09-07T08:39:02Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"By the way, I didn't do benchmarking for _later_ pages. Perhaps gain from\nusing only one git-rev-list invocation are outweighted for the unnecessary\noutput and parsing of skipped revisions.\n\nSo perhaps this part of series (patches 4, 5, 6, 8) should be skipped for\nnow... at least until new benchmarks. Unless someone wants to use\ngit-rev-list for generating list of revisions, and parse_rev_list onlky for\nrevisions which would be on page; alternatively change the 'p' page\nparameter to 'j' jump_to (= ). \n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"26503","messageId":"7vmz9c2goh.fsf@assigned-by-dhcp.cox.net","threadId":"5499","inReplyTo":"edoli0$oql$1@sea.gmane.org","subject":"Re: [PATCH 1/7] gitweb: Make pickaxe search a feature","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-07T09:02:06Z","receivedAt":"2006-09-07T09:02:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> Junio C Hamano wrote:\n>\n>> Jakub Narebski writes:\n>> \n>>> As pickaxe search (selected using undocumented 'pickaxe:' operator in\n>>> search query) is resource consuming, allow to turn it on/off using\n>>> feature meachanism.\n>> \n>> I do not have a problem against making it configurable.\n>> \n>>> + 'pickaxe' => {\n>>> + 'sub' => \\&feature_pickaxe,\n>>> + 'override' => 0,\n>>> + 'default' => [0]},\n>>> );\n>> \n>> The patch suggests that it is turned off by default right now; I\n>> have not checked it myself, but is that the case?\n>\n> No, it is not. Currently it is turned on, _but_ undocumented.\n\nThen it would be nice to make it documented and keep the default\nperhaps?\n"},{"id":"26506","messageId":"edong1$sn1$1@sea.gmane.org","threadId":"5499","inReplyTo":"7vmz9c2goh.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/7] gitweb: Make pickaxe search a feature","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-07T09:07:38Z","receivedAt":"2006-09-07T09:07:38Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n\n> Jakub Narebski writes:\n> \n>> Junio C Hamano wrote:\n\n>>> The patch suggests that it is turned off by default right now; I\n>>> have not checked it myself, but is that the case?\n>>\n>> No, it is not. Currently it is turned on, _but_ undocumented.\n> \n> Then it would be nice to make it documented and keep the default\n> perhaps?\n \nKeep the default is easy (although I thought that we can turn off expensive\npickaxe because it was undocumented and I suppose not many people used\nthat). Should I send corrected patch (i.e. making pickaxe search a feature,\nbut default on and perhaps overridable), or should I send correcting patch\n(i.e. make pickaxe search turned on by default patch)?\n\nDocumenting gitweb search operators (author:, committer:, pickaxe:) is not:\nwhere to put the documentation?\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"26622","messageId":"edtuot$p76$2@sea.gmane.org","threadId":"5499","inReplyTo":"200609061504.40725.jnareb@gmail.com","subject":"Re: [PATCH 0/7] gitweb: Trying to improve history view speed","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-09T08:42:41Z","receivedAt":"2006-09-09T08:42:41Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"\n\nJakub Narebski wrote:\n\n> Shortlog:\n> [PATCH 1/7] gitweb: Make pickaxe search a feature\n> [PATCH 2/7] gitweb: Paginate history output\n> [PATCH 3/7] gitweb: Use @hist_opts as git-rev-list parameters\n> in git_history\n> [PATCH 4/7] gitweb: Add parse_rev_list for later use\n> [PATCH 5/7] gitweb: Use parse_rev_list in git_shortlog and git_history\n> [PATCH 6/7] gitweb: Assume parsed revision list in git_shortlog_body\n> and git_history_body\n> [PATCH 7/7] gitweb: Set page to 0 if it is not defined, in git_history\n\nShould I resend corrected patch 1 (Make pickaxe search a feature; but\ndefault to on), and patches 2 and 7 as one patch (Paginate history output)?\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"26624","messageId":"7vvenxwglc.fsf@assigned-by-dhcp.cox.net","threadId":"5499","inReplyTo":"edtuot$p76$2@sea.gmane.org","subject":"Re: [PATCH 0/7] gitweb: Trying to improve history view speed","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-09T09:10:23Z","receivedAt":"2006-09-09T09:10:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> Jakub Narebski wrote:\n>\n>> Shortlog:\n>> [PATCH 1/7] gitweb: Make pickaxe search a feature\n>> [PATCH 2/7] gitweb: Paginate history output\n>> [PATCH 3/7] gitweb: Use @hist_opts as git-rev-list parameters\n>> in git_history\n>> [PATCH 4/7] gitweb: Add parse_rev_list for later use\n>> [PATCH 5/7] gitweb: Use parse_rev_list in git_shortlog and git_history\n>> [PATCH 6/7] gitweb: Assume parsed revision list in git_shortlog_body\n>> and git_history_body\n>> [PATCH 7/7] gitweb: Set page to 0 if it is not defined, in git_history\n>\n> Should I resend corrected patch 1 (Make pickaxe search a feature; but\n> default to on), and patches 2 and 7 as one patch (Paginate history output)?\n\nI looked at and understood 1 and 2+7, although I cannot claim I\nlooked at 2+7 deeply enough; cursory look told me they should be\nOk. So is 3, but I suspect gitweb without --full-history might\nsurprise some users.\n\nI do not know about 4, 5 and 6. I didn't look at them at all\nthe first time you sent them out, since I got an impression that\nyou did not understand how git-rev-list was supposed to work\nwhen you did them.\n\nNow Linus explained it to you, I suspect they would probably\nneed to be rethought?\n"},{"id":"26625","messageId":"edu180$vvs$1@sea.gmane.org","threadId":"5499","inReplyTo":"7vvenxwglc.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 0/7] gitweb: Trying to improve history view speed","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-09T09:24:53Z","receivedAt":"2006-09-09T09:24:53Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n\n> Jakub Narebski writes:\n> \n>> Jakub Narebski wrote:\n>>\n>>> Shortlog:\n>>> [PATCH 1/7] gitweb: Make pickaxe search a feature\n>>> [PATCH 2/7] gitweb: Paginate history output\n>>> [PATCH 3/7] gitweb: Use @hist_opts as git-rev-list parameters\n>>> in git_history\n>>> [PATCH 4/7] gitweb: Add parse_rev_list for later use\n>>> [PATCH 5/7] gitweb: Use parse_rev_list in git_shortlog and git_history\n>>> [PATCH 6/7] gitweb: Assume parsed revision list in git_shortlog_body\n>>> and git_history_body\n>>> [PATCH 7/7] gitweb: Set page to 0 if it is not defined, in git_history\n\n> I do not know about 4, 5 and 6. I didn't look at them at all\n> the first time you sent them out, since I got an impression that\n> you did not understand how git-rev-list was supposed to work\n> when you did them.\n> \n> Now Linus explained it to you, I suspect they would probably\n> need to be rethought?\n\nWell, first they don't offer that much of speed improvement even for the\nfirst page of output. And probably would be slower than current\nimplementation for the next-to-last, or the last page. So yes, patches 4-6\nare to be rethought, if not dropped at all. \n\nBy the way, what do you all do with the \"failed experiments\", to have them\nsaved somewhere, but to not make trouble for normal operations?\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"26626","messageId":"7vr6ylwejd.fsf@assigned-by-dhcp.cox.net","threadId":"5499","inReplyTo":"edu180$vvs$1@sea.gmane.org","subject":"Re: [PATCH 0/7] gitweb: Trying to improve history view speed","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-09T09:54:46Z","receivedAt":"2006-09-09T09:54:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> By the way, what do you all do with the \"failed experiments\", to have them\n> saved somewhere, but to not make trouble for normal operations?\n\nI usually keep them under .git/refs/tags/hold/. Yesterday (as I\nsaid in another thread) I dropped the 64-bit packfile index\ntopic I had in jc/pack-toc topic from \"pu\" by:\n\n\tgit tag hold/jc/pack-toc jc/pack-toc\n\tgit branch -D jc/pack-toc\n\nto shelve it and kill it.\n\nIn theory, I should occasionally come back to them and see if\nthey are worth keeping, but in practice they just keep piling up\nX-<. I should find time to clean them up.\n"}]}