{"thread":{"id":"47437","subject":"[PATCH 0/8] Codespeed perf results","startedAt":"2017-12-13T15:14:01Z","lastAt":"2017-12-14T17:31:30Z","messageCount":20,"participants":["Christian Couder","Philip Oakley","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"334745","messageId":"20171213151344.2138-1-chriscool@tuxfamily.org","threadId":"47437","inReplyTo":null,"subject":"[PATCH 0/8] Codespeed perf results","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-12-13T15:13:36Z","receivedAt":"2017-12-13T15:14:01Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This patch series is built on top of cc/perf-run-config which recently\ngraduated to master.\n\nIt makes it possible to send perf results to a Codespeed server. See\nhttps://github.com/tobami/codespeed/ and web sites like\nhttp://speed.pypy.org/ which are using Codespeed.\n\nThe end goal would be to have such a server always available to track\nhow the different git commands perform over time on different kind of\nrepos (small, medium, large, ...) with different optimizations on and\noff (split-index, libpcre2, BLK_SHA1, ...)\n\nWith this series and a config file like:\n\n$ cat perf.conf\n[perf]\n        dirsOrRevs = v2.12.0 v2.13.0\n        repeatCount = 10\n\tsendToCodespeed = http://localhost:8000\n\trepoName = Git repo\n[perf \"with libpcre\"]\n        makeOpts = \"DEVELOPER=1 USE_LIBPCRE=YesPlease\"\n[perf \"without libpcre\"]\n        makeOpts = \"DEVELOPER=1\"\n\nOne should be able to just launch:\n\n$ ./run --config perf.conf p7810-grep.sh\n\nand then get nice graphs in a Codespeed instance running on\nhttp://localhost:8000.\n\nCaveat\n~~~~~~\n\nFor now one has to create the \"Git repo\" environment in the Codespeed\nadmin interface. (We send the perf.repoName config variable in the\n\"environment\" Codespeed field.) This is because Codespeed requires the\nenvironment fields to be created and does not provide a simple way to\ncreate these fields programmatically.\n\nI might try to work around this problem in the future.\n\nLinks\n~~~~~\n\nThis patch series is available here:\n\nhttps://github.com/chriscool/git/commits/codespeed\n\nThe cc/perf-run-config patch series was discussed here:\n\nv1: https://public-inbox.org/git/20170713065050.19215-1-chriscool@tuxfamily.org/\nv2: https://public-inbox.org/git/CAP8UFD2j-UFh+9awz91gtZ-jusq7EUOExMgURO59vpf29jXS4A@mail.gmail.com/\n\nChristian Couder (8):\n  perf/aggregate: fix checking ENV{GIT_PERF_SUBSECTION}\n  perf/aggregate: refactor printing results\n  perf/aggregate: implement codespeed JSON output\n  perf/run: use $default_value instead of $4\n  perf/run: add conf_opts argument to get_var_from_env_or_config()\n  perf/run: learn about perf.codespeedOutput\n  perf/run: learn to send output to codespeed server\n  perf/run: read GIT_TEST_REPO_NAME from perf.repoName\n\n t/perf/aggregate.perl | 164 +++++++++++++++++++++++++++++++++++---------------\n t/perf/run            |  29 +++++++--\n 2 files changed, 140 insertions(+), 53 deletions(-)\n\n-- \n2.15.1.361.g8b07d831d0\n\n"},{"id":"334746","messageId":"20171213151344.2138-2-chriscool@tuxfamily.org","threadId":"47437","inReplyTo":"20171213151344.2138-1-chriscool@tuxfamily.org","subject":"[PATCH 1/8] perf/aggregate: fix checking ENV{GIT_PERF_SUBSECTION}","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-12-13T15:13:37Z","receivedAt":"2017-12-13T15:14:05Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n t/perf/aggregate.perl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl\nindex e401208488..769d418708 100755\n--- a/t/perf/aggregate.perl\n+++ b/t/perf/aggregate.perl\n@@ -70,7 +70,7 @@ if (not @tests) {\n }\n \n my $resultsdir = \"test-results\";\n-if ($ENV{GIT_PERF_SUBSECTION} ne \"\") {\n+if (exists $ENV{GIT_PERF_SUBSECTION} and $ENV{GIT_PERF_SUBSECTION} ne \"\") {\n \t$resultsdir .= \"/\" . $ENV{GIT_PERF_SUBSECTION};\n }\n \n-- \n2.15.1.361.g8b07d831d0\n\n"},{"id":"334747","messageId":"20171213151344.2138-3-chriscool@tuxfamily.org","threadId":"47437","inReplyTo":"20171213151344.2138-1-chriscool@tuxfamily.org","subject":"[PATCH 2/8] perf/aggregate: refactor printing results","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-12-13T15:13:38Z","receivedAt":"2017-12-13T15:14:08Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n t/perf/aggregate.perl | 96 +++++++++++++++++++++++++++------------------------\n 1 file changed, 50 insertions(+), 46 deletions(-)\n\ndiff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl\nindex 769d418708..3609cb5dc3 100755\n--- a/t/perf/aggregate.perl\n+++ b/t/perf/aggregate.perl\n@@ -100,13 +100,6 @@ sub read_descr {\n \treturn $line;\n }\n \n-my %descrs;\n-my $descrlen = 4; # \"Test\"\n-for my $t (@subtests) {\n-\t$descrs{$t} = $shorttests{$t}.\": \".read_descr(\"$resultsdir/$t.descr\");\n-\t$descrlen = length $descrs{$t} if length $descrs{$t}>$descrlen;\n-}\n-\n sub have_duplicate {\n \tmy %seen;\n \tfor (@_) {\n@@ -122,54 +115,65 @@ sub have_slash {\n \treturn 0;\n }\n \n-my %newdirabbrevs = %dirabbrevs;\n-while (!have_duplicate(values %newdirabbrevs)) {\n-\t%dirabbrevs = %newdirabbrevs;\n-\tlast if !have_slash(values %dirabbrevs);\n-\t%newdirabbrevs = %dirabbrevs;\n-\tfor (values %newdirabbrevs) {\n-\t\ts{^[^/]*/}{};\n+sub print_default_results {\n+\tmy %descrs;\n+\tmy $descrlen = 4; # \"Test\"\n+\tfor my $t (@subtests) {\n+\t\t$descrs{$t} = $shorttests{$t}.\": \".read_descr(\"$resultsdir/$t.descr\");\n+\t\t$descrlen = length $descrs{$t} if length $descrs{$t}>$descrlen;\n \t}\n-}\n \n-my %times;\n-my @colwidth = ((0)x@dirs);\n-for my $i (0..$#dirs) {\n-\tmy $d = $dirs[$i];\n-\tmy $w = length (exists $dirabbrevs{$d} ? $dirabbrevs{$d} : $dirnames{$d});\n-\t$colwidth[$i] = $w if $w > $colwidth[$i];\n-}\n-for my $t (@subtests) {\n-\tmy $firstr;\n+\tmy %newdirabbrevs = %dirabbrevs;\n+\twhile (!have_duplicate(values %newdirabbrevs)) {\n+\t\t%dirabbrevs = %newdirabbrevs;\n+\t\tlast if !have_slash(values %dirabbrevs);\n+\t\t%newdirabbrevs = %dirabbrevs;\n+\t\tfor (values %newdirabbrevs) {\n+\t\t\ts{^[^/]*/}{};\n+\t\t}\n+\t}\n+\n+\tmy %times;\n+\tmy @colwidth = ((0)x@dirs);\n \tfor my $i (0..$#dirs) {\n \t\tmy $d = $dirs[$i];\n-\t\t$times{$prefixes{$d}.$t} = [get_times(\"$resultsdir/$prefixes{$d}$t.times\")];\n-\t\tmy ($r,$u,$s) = @{$times{$prefixes{$d}.$t}};\n-\t\tmy $w = length format_times($r,$u,$s,$firstr);\n+\t\tmy $w = length (exists $dirabbrevs{$d} ? $dirabbrevs{$d} : $dirnames{$d});\n \t\t$colwidth[$i] = $w if $w > $colwidth[$i];\n-\t\t$firstr = $r unless defined $firstr;\n \t}\n-}\n-my $totalwidth = 3*@dirs+$descrlen;\n-$totalwidth += $_ for (@colwidth);\n-\n-binmode STDOUT, \":utf8\" or die \"PANIC on binmode: $!\";\n+\tfor my $t (@subtests) {\n+\t\tmy $firstr;\n+\t\tfor my $i (0..$#dirs) {\n+\t\t\tmy $d = $dirs[$i];\n+\t\t\t$times{$prefixes{$d}.$t} = [get_times(\"$resultsdir/$prefixes{$d}$t.times\")];\n+\t\t\tmy ($r,$u,$s) = @{$times{$prefixes{$d}.$t}};\n+\t\t\tmy $w = length format_times($r,$u,$s,$firstr);\n+\t\t\t$colwidth[$i] = $w if $w > $colwidth[$i];\n+\t\t\t$firstr = $r unless defined $firstr;\n+\t\t}\n+\t}\n+\tmy $totalwidth = 3*@dirs+$descrlen;\n+\t$totalwidth += $_ for (@colwidth);\n \n-printf \"%-${descrlen}s\", \"Test\";\n-for my $i (0..$#dirs) {\n-\tmy $d = $dirs[$i];\n-\tprintf \"   %-$colwidth[$i]s\", (exists $dirabbrevs{$d} ? $dirabbrevs{$d} : $dirnames{$d});\n-}\n-print \"\\n\";\n-print \"-\"x$totalwidth, \"\\n\";\n-for my $t (@subtests) {\n-\tprintf \"%-${descrlen}s\", $descrs{$t};\n-\tmy $firstr;\n+\tprintf \"%-${descrlen}s\", \"Test\";\n \tfor my $i (0..$#dirs) {\n \t\tmy $d = $dirs[$i];\n-\t\tmy ($r,$u,$s) = @{$times{$prefixes{$d}.$t}};\n-\t\tprintf \"   %-$colwidth[$i]s\", format_times($r,$u,$s,$firstr);\n-\t\t$firstr = $r unless defined $firstr;\n+\t\tprintf \"   %-$colwidth[$i]s\", (exists $dirabbrevs{$d} ? $dirabbrevs{$d} : $dirnames{$d});\n \t}\n \tprint \"\\n\";\n+\tprint \"-\"x$totalwidth, \"\\n\";\n+\tfor my $t (@subtests) {\n+\t\tprintf \"%-${descrlen}s\", $descrs{$t};\n+\t\tmy $firstr;\n+\t\tfor my $i (0..$#dirs) {\n+\t\t\tmy $d = $dirs[$i];\n+\t\t\tmy ($r,$u,$s) = @{$times{$prefixes{$d}.$t}};\n+\t\t\tprintf \"   %-$colwidth[$i]s\", format_times($r,$u,$s,$firstr);\n+\t\t\t$firstr = $r unless defined $firstr;\n+\t\t}\n+\t\tprint \"\\n\";\n+\t}\n }\n+\n+binmode STDOUT, \":utf8\" or die \"PANIC on binmode: $!\";\n+\n+print_default_results();\n-- \n2.15.1.361.g8b07d831d0\n\n"},{"id":"334748","messageId":"20171213151344.2138-5-chriscool@tuxfamily.org","threadId":"47437","inReplyTo":"20171213151344.2138-1-chriscool@tuxfamily.org","subject":"[PATCH 4/8] perf/run: use $default_value instead of $4","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-12-13T15:13:40Z","receivedAt":"2017-12-13T15:14:15Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n t/perf/run | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/perf/run b/t/perf/run\nindex 43e4de49ef..bbd703dc4f 100755\n--- a/t/perf/run\n+++ b/t/perf/run\n@@ -105,7 +105,7 @@ get_var_from_env_or_config () {\n \tenv_var=\"$1\"\n \tconf_sec=\"$2\"\n \tconf_var=\"$3\"\n-\t# $4 can be set to a default value\n+\tdefault_value=\"$4\" # optional\n \n \t# Do nothing if the env variable is already set\n \teval \"test -z \\\"\\${$env_var+x}\\\"\" || return\n@@ -123,7 +123,7 @@ get_var_from_env_or_config () {\n \tconf_value=$(git config -f \"$GIT_PERF_CONFIG_FILE\" \"$var\") &&\n \teval \"$env_var=\\\"$conf_value\\\"\" && return\n \n-\ttest -n \"${4+x}\" && eval \"$env_var=\\\"$4\\\"\"\n+\ttest -n \"${default_value+x}\" && eval \"$env_var=\\\"$default_value\\\"\"\n }\n \n run_subsection () {\n-- \n2.15.1.361.g8b07d831d0\n\n"},{"id":"334749","messageId":"20171213151344.2138-9-chriscool@tuxfamily.org","threadId":"47437","inReplyTo":"20171213151344.2138-1-chriscool@tuxfamily.org","subject":"[PATCH 8/8] perf/run: read GIT_TEST_REPO_NAME from perf.repoName","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-12-13T15:13:44Z","receivedAt":"2017-12-13T15:14:18Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n t/perf/run | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/t/perf/run b/t/perf/run\nindex 7b7011f19b..279c2d41f6 100755\n--- a/t/perf/run\n+++ b/t/perf/run\n@@ -137,6 +137,9 @@ run_subsection () {\n \tget_var_from_env_or_config \"GIT_PERF_MAKE_COMMAND\" \"perf\" \"makeCommand\"\n \tget_var_from_env_or_config \"GIT_PERF_MAKE_OPTS\" \"perf\" \"makeOpts\"\n \n+\tget_var_from_env_or_config \"GIT_TEST_REPO_NAME\" \"perf\" \"repoName\"\n+\texport GIT_PERF_REPO_NAME\n+\n \tGIT_PERF_AGGREGATING_LATER=t\n \texport GIT_PERF_AGGREGATING_LATER\n \n-- \n2.15.1.361.g8b07d831d0\n\n"},{"id":"334750","messageId":"20171213151344.2138-8-chriscool@tuxfamily.org","threadId":"47437","inReplyTo":"20171213151344.2138-1-chriscool@tuxfamily.org","subject":"[PATCH 7/8] perf/run: learn to send output to codespeed server","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-12-13T15:13:43Z","receivedAt":"2017-12-13T15:14:20Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n t/perf/run | 12 +++++++++++-\n 1 file changed, 11 insertions(+), 1 deletion(-)\n\ndiff --git a/t/perf/run b/t/perf/run\nindex 4454a2713d..7b7011f19b 100755\n--- a/t/perf/run\n+++ b/t/perf/run\n@@ -148,10 +148,20 @@ run_subsection () {\n \ttest \"$GIT_PERF_CODESPEED_OUTPUT\" = \"true\" && codespeed_opt=\"--codespeed\"\n \n \trun_dirs \"$@\"\n-\t./aggregate.perl $codespeed_opt \"$@\"\n+\n+\tif test -z \"$GIT_PERF_SEND_TO_CODESPEED\"\n+\tthen\n+\t\t./aggregate.perl $codespeed_opt \"$@\"\n+\telse\n+\t\tjson_res_file=\"test-results/$GIT_PERF_SUBSECTION/aggregate.json\"\n+\t\t./aggregate.perl --codespeed \"$@\" | tee \"$json_res_file\"\n+\t\tsend_data_url=\"$GIT_PERF_SEND_TO_CODESPEED/result/add/json/\"\n+\t\tcurl -v --request POST --data-urlencode \"json=$(cat \"$json_res_file\")\" \"$send_data_url\"\n+\tfi\n }\n \n get_var_from_env_or_config \"GIT_PERF_CODESPEED_OUTPUT\" \"perf\" \"codespeedOutput\" \"\" --bool\n+get_var_from_env_or_config \"GIT_PERF_SEND_TO_CODESPEED\" \"perf\" \"sendToCodespeed\"\n \n cd \"$(dirname $0)\"\n . ../../GIT-BUILD-OPTIONS\n-- \n2.15.1.361.g8b07d831d0\n\n"},{"id":"334751","messageId":"20171213151344.2138-4-chriscool@tuxfamily.org","threadId":"47437","inReplyTo":"20171213151344.2138-1-chriscool@tuxfamily.org","subject":"[PATCH 3/8] perf/aggregate: implement codespeed JSON output","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-12-13T15:13:39Z","receivedAt":"2017-12-13T15:14:22Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Codespeed (https://github.com/tobami/codespeed/) is an open source\nproject that can be used to track how some software performs over\ntime. It stores performance test results in a database and can show\nnice graphs and charts on a web interface.\n\nAs it can be interesting to Codespeed to see how Git performance\nevolves over time and releases, let's implement a Codespeed output\nin \"perf/aggregate.perl\".\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n t/perf/aggregate.perl | 68 +++++++++++++++++++++++++++++++++++++++++++++++++--\n 1 file changed, 66 insertions(+), 2 deletions(-)\n\ndiff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl\nindex 3609cb5dc3..6e15f62a9e 100755\n--- a/t/perf/aggregate.perl\n+++ b/t/perf/aggregate.perl\n@@ -35,10 +35,15 @@ sub format_times {\n \treturn $out;\n }\n \n-my (@dirs, %dirnames, %dirabbrevs, %prefixes, @tests);\n+my (@dirs, %dirnames, %dirabbrevs, %prefixes, @tests, $codespeed);\n while (scalar @ARGV) {\n \tmy $arg = $ARGV[0];\n \tmy $dir;\n+\tif ($arg eq \"--codespeed\") {\n+\t\t$codespeed = 1;\n+\t\tshift @ARGV;\n+\t\tnext;\n+\t}\n \tlast if -f $arg or $arg eq \"--\";\n \tif (! -d $arg) {\n \t\tmy $rev = Git::command_oneline(qw(rev-parse --verify), $arg);\n@@ -70,8 +75,10 @@ if (not @tests) {\n }\n \n my $resultsdir = \"test-results\";\n+my $results_section = \"\";\n if (exists $ENV{GIT_PERF_SUBSECTION} and $ENV{GIT_PERF_SUBSECTION} ne \"\") {\n \t$resultsdir .= \"/\" . $ENV{GIT_PERF_SUBSECTION};\n+\t$results_section = $ENV{GIT_PERF_SUBSECTION};\n }\n \n my @subtests;\n@@ -174,6 +181,63 @@ sub print_default_results {\n \t}\n }\n \n+sub print_codespeed_results {\n+\tmy ($results_section) = @_;\n+\n+\tmy $project = \"Git\";\n+\n+\tmy $executable;\n+\tif ($results_section eq \"\") {\n+\t\t$executable = `uname -o -p`;\n+\t} else {\n+\t\t$executable = $results_section;\n+\t\tchomp $executable;\n+\t}\n+\n+\tmy $environment;\n+\tif (exists $ENV{GIT_TEST_REPO_NAME} and $ENV{GIT_TEST_REPO_NAME} ne \"\") {\n+\t\t$environment = $ENV{GIT_TEST_REPO_NAME};\n+\t} elsif (exists $ENV{GIT_TEST_INSTALLED} and $ENV{GIT_TEST_INSTALLED} ne \"\") {\n+\t\t$environment = $ENV{GIT_TEST_INSTALLED};\n+\t\t$environment =~ s|/bin-wrappers$||;\n+\t} else {\n+\t\t$environment = `uname -r`;\n+\t\tchomp $environment;\n+\t}\n+\n+\tmy @data;\n+\n+\tfor my $t (@subtests) {\n+\t\tfor my $d (@dirs) {\n+\t\t\tmy $commitid = $prefixes{$d};\n+\t\t\t$commitid =~ s/^build_//;\n+\t\t\t$commitid =~ s/\\.$//;\n+\t\t\tmy ($result_value, $u, $s) = get_times(\"$resultsdir/$prefixes{$d}$t.times\");\n+\n+\t\t\tmy %vals = (\n+\t\t\t\t\"commitid\" => $commitid,\n+\t\t\t\t\"project\" => $project,\n+\t\t\t\t\"branch\" => $dirnames{$d},\n+\t\t\t\t\"executable\" => $executable,\n+\t\t\t\t\"benchmark\" => $shorttests{$t} . \" \" . read_descr(\"$resultsdir/$t.descr\"),\n+\t\t\t\t\"environment\" => $environment,\n+\t\t\t\t\"result_value\" => $result_value,\n+\t\t\t    );\n+\t\t\tpush @data, \\%vals;\n+\t\t}\n+\t}\n+\n+\t#use Data::Dumper qw/ Dumper /;\n+\t#print Dumper(\\@data);\n+\n+\tuse JSON;\n+\tprint to_json(\\@data, {utf8 => 1, pretty => 1}), \"\\n\";\n+}\n+\n binmode STDOUT, \":utf8\" or die \"PANIC on binmode: $!\";\n \n-print_default_results();\n+if ($codespeed) {\n+\tprint_codespeed_results($results_section);\n+} else {\n+\tprint_default_results();\n+}\n-- \n2.15.1.361.g8b07d831d0\n\n"},{"id":"334752","messageId":"20171213151344.2138-7-chriscool@tuxfamily.org","threadId":"47437","inReplyTo":"20171213151344.2138-1-chriscool@tuxfamily.org","subject":"[PATCH 6/8] perf/run: learn about perf.codespeedOutput","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-12-13T15:13:42Z","receivedAt":"2017-12-13T15:14:27Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n t/perf/run | 7 ++++++-\n 1 file changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/t/perf/run b/t/perf/run\nindex 04ea5090f9..4454a2713d 100755\n--- a/t/perf/run\n+++ b/t/perf/run\n@@ -144,10 +144,15 @@ run_subsection () {\n \t\tset -- . \"$@\"\n \tfi\n \n+\tcodespeed_opt=\n+\ttest \"$GIT_PERF_CODESPEED_OUTPUT\" = \"true\" && codespeed_opt=\"--codespeed\"\n+\n \trun_dirs \"$@\"\n-\t./aggregate.perl \"$@\"\n+\t./aggregate.perl $codespeed_opt \"$@\"\n }\n \n+get_var_from_env_or_config \"GIT_PERF_CODESPEED_OUTPUT\" \"perf\" \"codespeedOutput\" \"\" --bool\n+\n cd \"$(dirname $0)\"\n . ../../GIT-BUILD-OPTIONS\n \n-- \n2.15.1.361.g8b07d831d0\n\n"},{"id":"334753","messageId":"20171213151344.2138-6-chriscool@tuxfamily.org","threadId":"47437","inReplyTo":"20171213151344.2138-1-chriscool@tuxfamily.org","subject":"[PATCH 5/8] perf/run: add conf_opts argument to get_var_from_env_or_config()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-12-13T15:13:41Z","receivedAt":"2017-12-13T15:14:29Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n t/perf/run | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/t/perf/run b/t/perf/run\nindex bbd703dc4f..04ea5090f9 100755\n--- a/t/perf/run\n+++ b/t/perf/run\n@@ -106,6 +106,7 @@ get_var_from_env_or_config () {\n \tconf_sec=\"$2\"\n \tconf_var=\"$3\"\n \tdefault_value=\"$4\" # optional\n+\tconf_opts=\"$5\" # optional\n \n \t# Do nothing if the env variable is already set\n \teval \"test -z \\\"\\${$env_var+x}\\\"\" || return\n@@ -116,11 +117,11 @@ get_var_from_env_or_config () {\n \tif test -n \"$GIT_PERF_SUBSECTION\"\n \tthen\n \t\tvar=\"$conf_sec.$GIT_PERF_SUBSECTION.$conf_var\"\n-\t\tconf_value=$(git config -f \"$GIT_PERF_CONFIG_FILE\" \"$var\") &&\n+\t\tconf_value=$(git config $conf_opts -f \"$GIT_PERF_CONFIG_FILE\" \"$var\") &&\n \t\teval \"$env_var=\\\"$conf_value\\\"\" && return\n \tfi\n \tvar=\"$conf_sec.$conf_var\"\n-\tconf_value=$(git config -f \"$GIT_PERF_CONFIG_FILE\" \"$var\") &&\n+\tconf_value=$(git config $conf_opts -f \"$GIT_PERF_CONFIG_FILE\" \"$var\") &&\n \teval \"$env_var=\\\"$conf_value\\\"\" && return\n \n \ttest -n \"${default_value+x}\" && eval \"$env_var=\\\"$default_value\\\"\"\n-- \n2.15.1.361.g8b07d831d0\n\n"},{"id":"334756","messageId":"53BD7DB89F12403F974A7D2201CC7307@PhilipOakley","threadId":"47437","inReplyTo":"20171213151344.2138-1-chriscool@tuxfamily.org","subject":"Re: [PATCH 0/8] Codespeed perf results","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":null,"receivedAt":"2017-12-13T16:02:57Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Christian Couder\" <christian.couder@gmail.com>\n> This patch series is built on top of cc/perf-run-config which recently\n> graduated to master.\n>\n> It makes it possible to send perf results to a Codespeed server. See\n> https://github.com/tobami/codespeed/ and web sites like\n> http://speed.pypy.org/ which are using Codespeed.\n>\n> The end goal would be to have such a server always available to track\n> how the different git commands perform over time on different kind of\n> repos (small, medium, large, ...) with different optimizations on and\n> off (split-index, libpcre2, BLK_SHA1, ...)\n\nDumb question: is this expected to also be able to do a retrospective on the \nperformance of appropriate past releases? That would allow immediate \nperformance comparisons, rather than needing to wait for a few releases to \nsee the trends.\n\nPhilip\n\n>\n> With this series and a config file like:\n>\n> $ cat perf.conf\n> [perf]\n>        dirsOrRevs = v2.12.0 v2.13.0\n>        repeatCount = 10\n> sendToCodespeed = http://localhost:8000\n> repoName = Git repo\n> [perf \"with libpcre\"]\n>        makeOpts = \"DEVELOPER=1 USE_LIBPCRE=YesPlease\"\n> [perf \"without libpcre\"]\n>        makeOpts = \"DEVELOPER=1\"\n>\n> One should be able to just launch:\n>\n> $ ./run --config perf.conf p7810-grep.sh\n>\n> and then get nice graphs in a Codespeed instance running on\n> http://localhost:8000.\n>\n> Caveat\n> ~~~~~~\n>\n> For now one has to create the \"Git repo\" environment in the Codespeed\n> admin interface. (We send the perf.repoName config variable in the\n> \"environment\" Codespeed field.) This is because Codespeed requires the\n> environment fields to be created and does not provide a simple way to\n> create these fields programmatically.\n>\n> I might try to work around this problem in the future.\n>\n> Links\n> ~~~~~\n>\n> This patch series is available here:\n>\n> https://github.com/chriscool/git/commits/codespeed\n>\n> The cc/perf-run-config patch series was discussed here:\n>\n> v1: \n> https://public-inbox.org/git/20170713065050.19215-1-chriscool@tuxfamily.org/\n> v2: \n> https://public-inbox.org/git/CAP8UFD2j-UFh+9awz91gtZ-jusq7EUOExMgURO59vpf29jXS4A@mail.gmail.com/\n>\n> Christian Couder (8):\n>  perf/aggregate: fix checking ENV{GIT_PERF_SUBSECTION}\n>  perf/aggregate: refactor printing results\n>  perf/aggregate: implement codespeed JSON output\n>  perf/run: use $default_value instead of $4\n>  perf/run: add conf_opts argument to get_var_from_env_or_config()\n>  perf/run: learn about perf.codespeedOutput\n>  perf/run: learn to send output to codespeed server\n>  perf/run: read GIT_TEST_REPO_NAME from perf.repoName\n>\n> t/perf/aggregate.perl | 164 \n> +++++++++++++++++++++++++++++++++++---------------\n> t/perf/run            |  29 +++++++--\n> 2 files changed, 140 insertions(+), 53 deletions(-)\n>\n> -- \n> 2.15.1.361.g8b07d831d0\n> \n\n"},{"id":"334757","messageId":"CAP8UFD3Q4h-aYBDABSPOW948LQYVydWZ1hLPAD+kr9ZpXVZiaQ@mail.gmail.com","threadId":"47437","inReplyTo":"53BD7DB89F12403F974A7D2201CC7307@PhilipOakley","subject":"Re: [PATCH 0/8] Codespeed perf results","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-12-13T16:18:58Z","receivedAt":"2017-12-13T16:19:05Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Dec 13, 2017 at 5:02 PM, Philip Oakley <philipoakley@iee.org> wrote:\n\n>> The end goal would be to have such a server always available to track\n>> how the different git commands perform over time on different kind of\n>> repos (small, medium, large, ...) with different optimizations on and\n>> off (split-index, libpcre2, BLK_SHA1, ...)\n>\n> Dumb question: is this expected to also be able to do a retrospective on the\n> performance of appropriate past releases? That would allow immediate\n> performance comparisons, rather than needing to wait for a few releases to\n> see the trends.\n\nYeah, sure. For example in the perf.conf file above there is\n\"dirsOrRevs = v2.12.0 v2.13.0\" which means that tests will be\nperformed on v2.12.0 and v2.13.0.\n"},{"id":"334769","messageId":"CAPig+cSev+U9Cchyz1c4uQUsf8O4N8vVGTAsa5FiSA5mTOekxA@mail.gmail.com","threadId":"47437","inReplyTo":"20171213151344.2138-4-chriscool@tuxfamily.org","subject":"Re: [PATCH 3/8] perf/aggregate: implement codespeed JSON output","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2017-12-13T18:25:13Z","receivedAt":"2017-12-13T18:25:20Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Dec 13, 2017 at 10:13 AM, Christian Couder\n<christian.couder@gmail.com> wrote:\n> Codespeed (https://github.com/tobami/codespeed/) is an open source\n> project that can be used to track how some software performs over\n> time. It stores performance test results in a database and can show\n> nice graphs and charts on a web interface.\n>\n> As it can be interesting to Codespeed to see how Git performance\n> evolves over time and releases, let's implement a Codespeed output\n> in \"perf/aggregate.perl\".\n>\n> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n> ---\n> diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl\n> @@ -174,6 +181,63 @@ sub print_default_results {\n> +sub print_codespeed_results {\n> +       my ($results_section) = @_;\n> +\n> +       my $project = \"Git\";\n> +\n> +       my $executable;\n> +       if ($results_section eq \"\") {\n> +               $executable = `uname -o -p`;\n\nOption -o is not recognized on MacOS and, on at least a couple of my\nLinux installations, -p returns only \"unknown\".\n\nA combination, on the other hand, which seems to work nicely on MacOS,\nLinux, and BSD is: uname -s -m\n"},{"id":"334777","messageId":"xmqqfu8ewepg.fsf@gitster.mtv.corp.google.com","threadId":"47437","inReplyTo":"20171213151344.2138-4-chriscool@tuxfamily.org","subject":"Re: [PATCH 3/8] perf/aggregate: implement codespeed JSON output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-13T20:33:15Z","receivedAt":"2017-12-13T20:33:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n>  my $resultsdir = \"test-results\";\n> +my $results_section = \"\";\n>  if (exists $ENV{GIT_PERF_SUBSECTION} and $ENV{GIT_PERF_SUBSECTION} ne \"\") {\n>  \t$resultsdir .= \"/\" . $ENV{GIT_PERF_SUBSECTION};\n> +\t$results_section = $ENV{GIT_PERF_SUBSECTION};\n>  }\n\n...\n\n> +\tmy $executable;\n> +\tif ($results_section eq \"\") {\n> +\t\t$executable = `uname -o -p`;\n> +\t} else {\n> +\t\t$executable = $results_section;\n> +\t\tchomp $executable;\n> +\t}\n\nAside from portability of 'uname -o' Eric raised, I wonder if the\nplatform information is still useful even when perf-subsection is\nspecified.  With the above code, we can identify that a single\nresult is for (say) MacOS only when we are not limiting to a single\nsubsection, but wouldn't it be equally a valid desire to be able to\ntrack performance figures for a single subsection over time and\nbeing able to say \"On MacOS, subsection A's performance dropped\nbetween release X and X+1 quite a bit, but on Linux x86-64, there\nwas no such change\" or somesuch?\n\nIOW, shouldn't the \"executable\" label always contain the platform\ninformation, plus an optional subsection info when (and only when)\nthe result is limited to a subsection?\n\nBy the way, $results_section that is not an empty string at this\npoint must have come from $ENV{GIT_PERF_SUBSECTION}, and from the\nway the environment variable is used in t/perf/run, e.g.\n\n\t\t(\n\t\t\tGIT_PERF_SUBSECTION=\"$subsec\"\n\t\t\texport GIT_PERF_SUBSECTION\n\t\t\techo \"======== Run for subsection '$GIT_PERF_SUBSECTION' ========\"\n\t\t\trun_subsection \"$@\"\n\t\t)\n\nI do not think we expect it to have a trailing LF.  What's that\nchomp doing there?\n"},{"id":"334778","messageId":"xmqqbmj2wedd.fsf@gitster.mtv.corp.google.com","threadId":"47437","inReplyTo":"20171213151344.2138-5-chriscool@tuxfamily.org","subject":"Re: [PATCH 4/8] perf/run: use $default_value instead of $4","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-13T20:40:30Z","receivedAt":"2017-12-13T20:40:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n> ---\n>  t/perf/run | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/perf/run b/t/perf/run\n> index 43e4de49ef..bbd703dc4f 100755\n> --- a/t/perf/run\n> +++ b/t/perf/run\n> @@ -105,7 +105,7 @@ get_var_from_env_or_config () {\n>  \tenv_var=\"$1\"\n>  \tconf_sec=\"$2\"\n>  \tconf_var=\"$3\"\n> -\t# $4 can be set to a default value\n> +\tdefault_value=\"$4\" # optional\n>  \n>  \t# Do nothing if the env variable is already set\n>  \teval \"test -z \\\"\\${$env_var+x}\\\"\" || return\n> @@ -123,7 +123,7 @@ get_var_from_env_or_config () {\n>  \tconf_value=$(git config -f \"$GIT_PERF_CONFIG_FILE\" \"$var\") &&\n>  \teval \"$env_var=\\\"$conf_value\\\"\" && return\n>  \n> -\ttest -n \"${4+x}\" && eval \"$env_var=\\\"$4\\\"\"\n> +\ttest -n \"${default_value+x}\" && eval \"$env_var=\\\"$default_value\\\"\"\n\nThis conversion changes the behaviour.  Because default_value is\nalways set by your change in the previous hunk, we end up always\ndoing this eval.\n\nThe original says \"If $4 is set, then ${4+x} becomes x and if $4 is\nnot set, ${4+x} is empty, so let's check if ${4+x} is a non-empty\nstring to see if $4 is set.  If ${4+x} is a non-empty string, that\nmeans $4 was set so we do the eval.\n\nIf you want to be able to use this helper to specify a default value\nof an empty string (which the orignal that used $4 did), then the\nprevious hunk must be corrected so that it does not unconditionally\nset default_value to $4.  Perhaps like\n\n\tif test -n \"${4+x}\"\n\tthen\n\t\tdefault_value=$4\n\telse\n\t\tunset default_value || :\n\tfi\n\nor something.\n\n>  }\n>  \n>  run_subsection () {\n"},{"id":"334779","messageId":"xmqq4louwdpn.fsf@gitster.mtv.corp.google.com","threadId":"47437","inReplyTo":"xmqqbmj2wedd.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 4/8] perf/run: use $default_value instead of $4","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-13T20:54:44Z","receivedAt":"2017-12-13T20:54:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> If you want to be able to use this helper to specify a default value\n> of an empty string (which the orignal that used $4 did), then the\n> previous hunk must be corrected so that it does not unconditionally\n> set default_value to $4.  Perhaps like\n>\n> \tif test -n \"${4+x}\"\n> \tthen\n> \t\tdefault_value=$4\n> \telse\n> \t\tunset default_value || :\n> \tfi\n>\n> or something.\n\nAnd if you do not care about passing an empty string as the default\nvalue, then the earlier hunk can stay the same\n\n\tdefault_value=$4\n\nbut the eval part should become something like\n\n\ttest -n \"$default_value\" && eval ...\n\nGiven that you are planning to add yet another optional argument to\nthe helper, and when you pass that variable, you'd probably need to\npass \"\" as the \"default\" that is not exported, perhaps this \"give up\nability to pass an empty string as the default\" approach may be the\nonly thing you could do.\n\n"},{"id":"334792","messageId":"CAP8UFD03hycHW5LSeNjkEKOLgULTPnWkAeBUJH3aL5YOp5Zr1Q@mail.gmail.com","threadId":"47437","inReplyTo":"CAPig+cSev+U9Cchyz1c4uQUsf8O4N8vVGTAsa5FiSA5mTOekxA@mail.gmail.com","subject":"Re: [PATCH 3/8] perf/aggregate: implement codespeed JSON output","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-12-14T08:47:46Z","receivedAt":"2017-12-14T08:47:52Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Dec 13, 2017 at 7:25 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n> On Wed, Dec 13, 2017 at 10:13 AM, Christian Couder\n> <christian.couder@gmail.com> wrote:\n>> Codespeed (https://github.com/tobami/codespeed/) is an open source\n>> project that can be used to track how some software performs over\n>> time. It stores performance test results in a database and can show\n>> nice graphs and charts on a web interface.\n>>\n>> As it can be interesting to Codespeed to see how Git performance\n>> evolves over time and releases, let's implement a Codespeed output\n>> in \"perf/aggregate.perl\".\n>>\n>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n>> ---\n>> diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl\n>> @@ -174,6 +181,63 @@ sub print_default_results {\n>> +sub print_codespeed_results {\n>> +       my ($results_section) = @_;\n>> +\n>> +       my $project = \"Git\";\n>> +\n>> +       my $executable;\n>> +       if ($results_section eq \"\") {\n>> +               $executable = `uname -o -p`;\n>\n> Option -o is not recognized on MacOS and, on at least a couple of my\n> Linux installations, -p returns only \"unknown\".\n>\n> A combination, on the other hand, which seems to work nicely on MacOS,\n> Linux, and BSD is: uname -s -m\n\nOk I will take a look at this. Thanks!\n"},{"id":"334793","messageId":"CAP8UFD1OAwTNAe94DPuJJSAvTa7ojH3sdsK_fkfsW4LUAw+u=Q@mail.gmail.com","threadId":"47437","inReplyTo":"xmqqfu8ewepg.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/8] perf/aggregate: implement codespeed JSON output","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-12-14T08:57:05Z","receivedAt":"2017-12-14T08:57:10Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Dec 13, 2017 at 9:33 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Christian Couder <christian.couder@gmail.com> writes:\n>\n>>  my $resultsdir = \"test-results\";\n>> +my $results_section = \"\";\n>>  if (exists $ENV{GIT_PERF_SUBSECTION} and $ENV{GIT_PERF_SUBSECTION} ne \"\") {\n>>       $resultsdir .= \"/\" . $ENV{GIT_PERF_SUBSECTION};\n>> +     $results_section = $ENV{GIT_PERF_SUBSECTION};\n>>  }\n>\n> ...\n>\n>> +     my $executable;\n>> +     if ($results_section eq \"\") {\n>> +             $executable = `uname -o -p`;\n>> +     } else {\n>> +             $executable = $results_section;\n>> +             chomp $executable;\n>> +     }\n>\n> Aside from portability of 'uname -o' Eric raised, I wonder if the\n> platform information is still useful even when perf-subsection is\n> specified.  With the above code, we can identify that a single\n> result is for (say) MacOS only when we are not limiting to a single\n> subsection, but wouldn't it be equally a valid desire to be able to\n> track performance figures for a single subsection over time and\n> being able to say \"On MacOS, subsection A's performance dropped\n> between release X and X+1 quite a bit, but on Linux x86-64, there\n> was no such change\" or somesuch?\n\nYeah, I agree that it would be useful. Unfortunately it looks like the\nnumber of fields in Codespeed is limited and I am not sure what will\nbe more important for people in general. Another option would be to\nhave \"MacOS\" in the \"environment\" field. Or maybe there is a need for\na generic way to customize this. For now I just tried to come up with\nsomething sensible for what AEvar and me want to do.\n\n> IOW, shouldn't the \"executable\" label always contain the platform\n> information, plus an optional subsection info when (and only when)\n> the result is limited to a subsection?\n>\n> By the way, $results_section that is not an empty string at this\n> point must have come from $ENV{GIT_PERF_SUBSECTION}, and from the\n> way the environment variable is used in t/perf/run, e.g.\n>\n>                 (\n>                         GIT_PERF_SUBSECTION=\"$subsec\"\n>                         export GIT_PERF_SUBSECTION\n>                         echo \"======== Run for subsection '$GIT_PERF_SUBSECTION' ========\"\n>                         run_subsection \"$@\"\n>                 )\n>\n> I do not think we expect it to have a trailing LF.  What's that\n> chomp doing there?\n\nIt's a silly mistake I made when I reorganized the patches just before\nsending them. The chomp should be after \"$executable = `uname -o -p`;\"\n(so in the other branch of the \"if ... else\"). I will fix this in the\nnext version.\n"},{"id":"334794","messageId":"CAP8UFD3=YCfrZZ0Xq6YSKTwr8e5HV5iwPN4G=Xb0s1yz8RTr6Q@mail.gmail.com","threadId":"47437","inReplyTo":"xmqqbmj2wedd.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 4/8] perf/run: use $default_value instead of $4","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-12-14T08:59:27Z","receivedAt":"2017-12-14T08:59:35Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Dec 13, 2017 at 9:40 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Christian Couder <christian.couder@gmail.com> writes:\n>\n>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n>> ---\n>>  t/perf/run | 4 ++--\n>>  1 file changed, 2 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/t/perf/run b/t/perf/run\n>> index 43e4de49ef..bbd703dc4f 100755\n>> --- a/t/perf/run\n>> +++ b/t/perf/run\n>> @@ -105,7 +105,7 @@ get_var_from_env_or_config () {\n>>       env_var=\"$1\"\n>>       conf_sec=\"$2\"\n>>       conf_var=\"$3\"\n>> -     # $4 can be set to a default value\n>> +     default_value=\"$4\" # optional\n>>\n>>       # Do nothing if the env variable is already set\n>>       eval \"test -z \\\"\\${$env_var+x}\\\"\" || return\n>> @@ -123,7 +123,7 @@ get_var_from_env_or_config () {\n>>       conf_value=$(git config -f \"$GIT_PERF_CONFIG_FILE\" \"$var\") &&\n>>       eval \"$env_var=\\\"$conf_value\\\"\" && return\n>>\n>> -     test -n \"${4+x}\" && eval \"$env_var=\\\"$4\\\"\"\n>> +     test -n \"${default_value+x}\" && eval \"$env_var=\\\"$default_value\\\"\"\n>\n> This conversion changes the behaviour.  Because default_value is\n> always set by your change in the previous hunk, we end up always\n> doing this eval.\n>\n> The original says \"If $4 is set, then ${4+x} becomes x and if $4 is\n> not set, ${4+x} is empty, so let's check if ${4+x} is a non-empty\n> string to see if $4 is set.  If ${4+x} is a non-empty string, that\n> means $4 was set so we do the eval.\n>\n> If you want to be able to use this helper to specify a default value\n> of an empty string (which the orignal that used $4 did), then the\n> previous hunk must be corrected so that it does not unconditionally\n> set default_value to $4.  Perhaps like\n>\n>         if test -n \"${4+x}\"\n>         then\n>                 default_value=$4\n>         else\n>                 unset default_value || :\n>         fi\n>\n> or something.\n\nYeah, you are right I will fix this.\n"},{"id":"334795","messageId":"CAP8UFD3sCQcp_006AhQXrKicDmuLehRLQDN+X2upRy3sQOKOcQ@mail.gmail.com","threadId":"47437","inReplyTo":"xmqq4louwdpn.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 4/8] perf/run: use $default_value instead of $4","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-12-14T09:00:36Z","receivedAt":"2017-12-14T09:00:49Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Dec 13, 2017 at 9:54 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> If you want to be able to use this helper to specify a default value\n>> of an empty string (which the orignal that used $4 did), then the\n>> previous hunk must be corrected so that it does not unconditionally\n>> set default_value to $4.  Perhaps like\n>>\n>>       if test -n \"${4+x}\"\n>>       then\n>>               default_value=$4\n>>       else\n>>               unset default_value || :\n>>       fi\n>>\n>> or something.\n>\n> And if you do not care about passing an empty string as the default\n> value, then the earlier hunk can stay the same\n>\n>         default_value=$4\n>\n> but the eval part should become something like\n>\n>         test -n \"$default_value\" && eval ...\n>\n> Given that you are planning to add yet another optional argument to\n> the helper, and when you pass that variable, you'd probably need to\n> pass \"\" as the \"default\" that is not exported, perhaps this \"give up\n> ability to pass an empty string as the default\" approach may be the\n> only thing you could do.\n\nYeah, thanks for the explanations and suggestions.\n"},{"id":"334810","messageId":"xmqqfu8dusgk.fsf@gitster.mtv.corp.google.com","threadId":"47437","inReplyTo":"CAP8UFD1OAwTNAe94DPuJJSAvTa7ojH3sdsK_fkfsW4LUAw+u=Q@mail.gmail.com","subject":"Re: [PATCH 3/8] perf/aggregate: implement codespeed JSON output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-14T17:31:23Z","receivedAt":"2017-12-14T17:31:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n>> Aside from portability of 'uname -o' Eric raised, I wonder if the\n>> platform information is still useful even when perf-subsection is\n>> specified.  With the above code, we can identify that a single\n>> result is for (say) MacOS only when we are not limiting to a single\n>> subsection, but wouldn't it be equally a valid desire to be able to\n>> track performance figures for a single subsection over time and\n>> being able to say \"On MacOS, subsection A's performance dropped\n>> between release X and X+1 quite a bit, but on Linux x86-64, there\n>> was no such change\" or somesuch?\n>\n> Yeah, I agree that it would be useful. Unfortunately it looks like the\n> number of fields in Codespeed is limited and I am not sure what will\n> be more important for people in general.\n\nIs there a reason why such textual labels meant for human\nconsumption need to be two (or more) separate fields?  Would simply\nconcatenating them into a single string defeat whatever you wanted\nto achieve by having this information in $executable in the first\nplace?\n\n"}]}