{"thread":{"id":"34664","subject":"[PATCH 0/3] optimize git-contacts","startedAt":"2013-08-09T21:39:53Z","lastAt":"2013-08-13T21:10:47Z","messageCount":5,"participants":["Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"224956","messageId":"1376084396-53617-1-git-send-email-sunshine@sunshineco.com","threadId":"34664","inReplyTo":null,"subject":"[PATCH 0/3] optimize git-contacts","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-08-09T21:39:53Z","receivedAt":"2013-08-09T21:39:53Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"This patch series aims to make contrib/git-contacts faster by reducing\nthe number of expensive git-blame invocations it makes. It does so by\ntaking advantage of git-blame's new ability to accept multiple -L ranges\nin a single invocation [1] (es/blame-L-twice in 'next').\n\nSome numbers, given a randomly chosen range of commits:\n\n  (before)\n  % time git-contacts 25fba78d36be6297^..23c339c0f262aad2 >/dev/null\n  real  0m6.142s\n  user  0m5.429s\n  sys   0m0.356s\n\n  (after)\n  % time git-contacts 25fba78d36be6297^..23c339c0f262aad2 >/dev/null\n  real  0m2.285s\n  user  0m2.093s\n  sys   0m0.165s\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/231739\n\nEric Sunshine (3):\n  contacts: validate hunk length earlier\n  contacts: gather all blame sources prior to invoking git-blame\n  contacts: reduce git-blame invocations\n\n contrib/contacts/git-contacts | 31 ++++++++++++++++++++++---------\n 1 file changed, 22 insertions(+), 9 deletions(-)\n\n-- \n1.8.4.rc2.460.ga591f4a\n"},{"id":"224957","messageId":"1376084396-53617-2-git-send-email-sunshine@sunshineco.com","threadId":"34664","inReplyTo":"1376084396-53617-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 1/3] contacts: validate hunk length earlier","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-08-09T21:39:54Z","receivedAt":"2013-08-09T21:39:54Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"Rather than calling get_blame() with a zero-length hunk only to have it\nrejected immediately, perform hunk-length validation earlier in order to\navoid calling get_blame() unnecessarily.\n\nThis is a preparatory step to simplify later patches which reduce the\nnumber of git-blame invocations by collecting together all lines to\nblame within a single file at a particular revision. By validating the\nblame range early, the subsequent patch can more easily avoid adding\nempty ranges at collection time.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n contrib/contacts/git-contacts | 5 ++---\n 1 file changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/contrib/contacts/git-contacts b/contrib/contacts/git-contacts\nindex d80f7d1..4fbb2ef 100755\n--- a/contrib/contacts/git-contacts\n+++ b/contrib/contacts/git-contacts\n@@ -60,8 +60,6 @@ sub import_commits {\n \n sub get_blame {\n \tmy ($commits, $source, $start, $len, $from) = @_;\n-\t$len = 1 unless defined($len);\n-\treturn if $len == 0;\n \topen my $f, '-|',\n \t\tqw(git blame --porcelain -C), '-L', \"$start,+$len\",\n \t\t'--since', $since, \"$from^\", '--', $source or die;\n@@ -90,7 +88,8 @@ sub scan_patches {\n \t\t} elsif (/^--- /) {\n \t\t\tdie \"Cannot parse hunk source: $_\\n\";\n \t\t} elsif (/^@@ -(\\d+)(?:,(\\d+))?/ && $source) {\n-\t\t\tget_blame($commits, $source, $1, $2, $id);\n+\t\t\tmy $len = defined($2) ? $2 : 1;\n+\t\t\tget_blame($commits, $source, $1, $len, $id) if $len;\n \t\t}\n \t}\n }\n-- \n1.8.4.rc2.460.ga591f4a\n"},{"id":"224959","messageId":"1376084396-53617-3-git-send-email-sunshine@sunshineco.com","threadId":"34664","inReplyTo":"1376084396-53617-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 2/3] contacts: gather all blame sources prior to invoking git-blame","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-08-09T21:39:55Z","receivedAt":"2013-08-09T21:39:55Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"git-contacts invokes git-blame immediately upon encountering a patch\nhunk. No attempt is made to consolidate invocations for multiple hunks\nreferencing the same file at the same revision. This can become\nexpensive quickly.\n\nAny effort to reduce the number of times git-blame is run will need to\nto know in advance which line ranges to blame per file per revision.\nMake this information available by collecting all sources as a distinct\nstep from invoking git-blame.  A subsequent patch will utilize the\ninformation to optimize git-blame invocations.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n contrib/contacts/git-contacts | 25 ++++++++++++++++++++-----\n 1 file changed, 20 insertions(+), 5 deletions(-)\n\ndiff --git a/contrib/contacts/git-contacts b/contrib/contacts/git-contacts\nindex 4fbb2ef..b4d3526 100755\n--- a/contrib/contacts/git-contacts\n+++ b/contrib/contacts/git-contacts\n@@ -74,8 +74,20 @@ sub get_blame {\n \tclose $f;\n }\n \n+sub blame_sources {\n+\tmy ($sources, $commits) = @_;\n+\tfor my $s (keys %$sources) {\n+\t\tfor my $id (keys %{$sources->{$s}}) {\n+\t\t\tfor my $range (@{$sources->{$s}{$id}}) {\n+\t\t\t\tget_blame($commits, $s,\n+\t\t\t\t\t  $range->[0], $range->[1], $id);\n+\t\t\t}\n+\t\t}\n+\t}\n+}\n+\n sub scan_patches {\n-\tmy ($commits, $id, $f) = @_;\n+\tmy ($sources, $id, $f) = @_;\n \tmy $source;\n \twhile (<$f>) {\n \t\tif (/^From ([0-9a-f]{40}) Mon Sep 17 00:00:00 2001$/) {\n@@ -89,7 +101,7 @@ sub scan_patches {\n \t\t\tdie \"Cannot parse hunk source: $_\\n\";\n \t\t} elsif (/^@@ -(\\d+)(?:,(\\d+))?/ && $source) {\n \t\t\tmy $len = defined($2) ? $2 : 1;\n-\t\t\tget_blame($commits, $source, $1, $len, $id) if $len;\n+\t\t\tpush @{$sources->{$source}{$id}}, [$1, $len] if $len;\n \t\t}\n \t}\n }\n@@ -162,13 +174,16 @@ for (@ARGV) {\n \t}\n }\n \n-my %commits;\n+my %sources;\n for (@files) {\n-\tscan_patch_file(\\%commits, $_);\n+\tscan_patch_file(\\%sources, $_);\n }\n if (@rev_args) {\n-\tscan_rev_args(\\%commits, \\@rev_args)\n+\tscan_rev_args(\\%sources, \\@rev_args)\n }\n+\n+my %commits;\n+blame_sources(\\%sources, \\%commits);\n import_commits(\\%commits);\n \n my $contacts = {};\n-- \n1.8.4.rc2.460.ga591f4a\n"},{"id":"224958","messageId":"1376084396-53617-4-git-send-email-sunshine@sunshineco.com","threadId":"34664","inReplyTo":"1376084396-53617-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 3/3] contacts: reduce git-blame invocations","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-08-09T21:39:56Z","receivedAt":"2013-08-09T21:39:56Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"git-contacts invokes git-blame once for each patch hunk it encounters.\nNo attempt is made to consolidate invocations for multiple hunks\nreferencing the same file at the same revision. This can become\nexpensive quickly.\n\nReduce the number of git-blame invocations by taking advantage of the\nability to specify multiple -L ranges for a single invocation.\n\nWithout this patch, on a randomly chosen range of commits:\n\n  % time git-contacts 25fba78d36be6297^..23c339c0f262aad2 >/dev/null\n  real  0m6.142s\n  user  0m5.429s\n  sys   0m0.356s\n\nWith this patch:\n\n  % time git-contacts 25fba78d36be6297^..23c339c0f262aad2 >/dev/null\n  real  0m2.285s\n  user  0m2.093s\n  sys   0m0.165s\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n contrib/contacts/git-contacts | 11 +++++------\n 1 file changed, 5 insertions(+), 6 deletions(-)\n\ndiff --git a/contrib/contacts/git-contacts b/contrib/contacts/git-contacts\nindex b4d3526..fb6429b 100755\n--- a/contrib/contacts/git-contacts\n+++ b/contrib/contacts/git-contacts\n@@ -59,9 +59,11 @@ sub import_commits {\n }\n \n sub get_blame {\n-\tmy ($commits, $source, $start, $len, $from) = @_;\n+\tmy ($commits, $source, $from, $ranges) = @_;\n+\treturn unless @$ranges;\n \topen my $f, '-|',\n-\t\tqw(git blame --porcelain -C), '-L', \"$start,+$len\",\n+\t\tqw(git blame --porcelain -C),\n+\t\tmap({\"-L$_->[0],+$_->[1]\"} @$ranges),\n \t\t'--since', $since, \"$from^\", '--', $source or die;\n \twhile (<$f>) {\n \t\tif (/^([0-9a-f]{40}) \\d+ \\d+ \\d+$/) {\n@@ -78,10 +80,7 @@ sub blame_sources {\n \tmy ($sources, $commits) = @_;\n \tfor my $s (keys %$sources) {\n \t\tfor my $id (keys %{$sources->{$s}}) {\n-\t\t\tfor my $range (@{$sources->{$s}{$id}}) {\n-\t\t\t\tget_blame($commits, $s,\n-\t\t\t\t\t  $range->[0], $range->[1], $id);\n-\t\t\t}\n+\t\t\tget_blame($commits, $s, $id, $sources->{$s}{$id});\n \t\t}\n \t}\n }\n-- \n1.8.4.rc2.460.ga591f4a\n"},{"id":"225165","messageId":"7veh9xwb3c.fsf@alter.siamese.dyndns.org","threadId":"34664","inReplyTo":"1376084396-53617-4-git-send-email-sunshine@sunshineco.com","subject":"Re: [PATCH 3/3] contacts: reduce git-blame invocations","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-13T21:10:47Z","receivedAt":"2013-08-13T21:10:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> git-contacts invokes git-blame once for each patch hunk it encounters.\n> No attempt is made to consolidate invocations for multiple hunks\n> referencing the same file at the same revision. This can become\n> expensive quickly.\n>\n> Reduce the number of git-blame invocations by taking advantage of the\n> ability to specify multiple -L ranges for a single invocation.\n>\n> Without this patch, on a randomly chosen range of commits:\n>\n>   % time git-contacts 25fba78d36be6297^..23c339c0f262aad2 >/dev/null\n>   real  0m6.142s\n>   user  0m5.429s\n>   sys   0m0.356s\n>\n> With this patch:\n>\n>   % time git-contacts 25fba78d36be6297^..23c339c0f262aad2 >/dev/null\n>   real  0m2.285s\n>   user  0m2.093s\n>   sys   0m0.165s\n>\n> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>\n\nNice.  Will queue.\n\n\n> ---\n>  contrib/contacts/git-contacts | 11 +++++------\n>  1 file changed, 5 insertions(+), 6 deletions(-)\n>\n> diff --git a/contrib/contacts/git-contacts b/contrib/contacts/git-contacts\n> index b4d3526..fb6429b 100755\n> --- a/contrib/contacts/git-contacts\n> +++ b/contrib/contacts/git-contacts\n> @@ -59,9 +59,11 @@ sub import_commits {\n>  }\n>  \n>  sub get_blame {\n> -\tmy ($commits, $source, $start, $len, $from) = @_;\n> +\tmy ($commits, $source, $from, $ranges) = @_;\n> +\treturn unless @$ranges;\n>  \topen my $f, '-|',\n> -\t\tqw(git blame --porcelain -C), '-L', \"$start,+$len\",\n> +\t\tqw(git blame --porcelain -C),\n> +\t\tmap({\"-L$_->[0],+$_->[1]\"} @$ranges),\n>  \t\t'--since', $since, \"$from^\", '--', $source or die;\n>  \twhile (<$f>) {\n>  \t\tif (/^([0-9a-f]{40}) \\d+ \\d+ \\d+$/) {\n> @@ -78,10 +80,7 @@ sub blame_sources {\n>  \tmy ($sources, $commits) = @_;\n>  \tfor my $s (keys %$sources) {\n>  \t\tfor my $id (keys %{$sources->{$s}}) {\n> -\t\t\tfor my $range (@{$sources->{$s}{$id}}) {\n> -\t\t\t\tget_blame($commits, $s,\n> -\t\t\t\t\t  $range->[0], $range->[1], $id);\n> -\t\t\t}\n> +\t\t\tget_blame($commits, $s, $id, $sources->{$s}{$id});\n>  \t\t}\n>  \t}\n>  }\n"}]}