# [PATCH 1/3] perf/aggregate: add --subsection option 6 messages from 2018-01-28 to 2018-01-29. Participants: Christian Couder, Philip Oakley, Eric Sunshine. Thread: https://gitlist.dev/t/47705 ## Christian Couder, 2018-01-28 11:18 Subject: [PATCH 1/3] perf/aggregate: add --subsection option Message-ID: <20180128111843.2690-1-chriscool@tuxfamily.org> URL: https://gitlist.dev/e/20180128111843.2690-1-chriscool%40tuxfamily.org ``` This makes it easier to use the aggregate script on the command line, to get results from subsections. Previously setting GIT_PERF_SUBSECTION was needed for this purpose. Signed-off-by: Christian Couder --- t/perf/aggregate.perl | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) This small patch series contains a few small Codespeed related usability improvements of the perf/aggregate.perl script on top the cc/codespeed patch series that was recently merged into master. diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl index 5c439f6bc2..bbf0f30898 100755 --- a/t/perf/aggregate.perl +++ b/t/perf/aggregate.perl @@ -36,7 +36,8 @@ sub format_times { return $out; } -my (@dirs, %dirnames, %dirabbrevs, %prefixes, @tests, $codespeed); +my (@dirs, %dirnames, %dirabbrevs, %prefixes, @tests, + $codespeed, $subsection); while (scalar @ARGV) { my $arg = $ARGV[0]; my $dir; @@ -45,6 +46,15 @@ while (scalar @ARGV) { shift @ARGV; next; } + if ($arg eq "--subsection") { + shift @ARGV; + $subsection = $ARGV[0]; + shift @ARGV; + if (! $subsection) { + die "empty subsection"; + } + next; + } last if -f $arg or $arg eq "--"; if (! -d $arg) { my $rev = Git::command_oneline(qw(rev-parse --verify), $arg); @@ -76,10 +86,15 @@ if (not @tests) { } my $resultsdir = "test-results"; -my $results_section = ""; -if (exists $ENV{GIT_PERF_SUBSECTION} and $ENV{GIT_PERF_SUBSECTION} ne "") { - $resultsdir .= "/" . $ENV{GIT_PERF_SUBSECTION}; - $results_section = $ENV{GIT_PERF_SUBSECTION}; + +if (! $subsection and + exists $ENV{GIT_PERF_SUBSECTION} and + $ENV{GIT_PERF_SUBSECTION} ne "") { + $subsection = $ENV{GIT_PERF_SUBSECTION}; +} + +if ($subsection) { + $resultsdir .= "/" . $subsection; } my @subtests; @@ -183,15 +198,15 @@ sub print_default_results { } sub print_codespeed_results { - my ($results_section) = @_; + my ($subsection) = @_; my $project = "Git"; my $executable = `uname -s -m`; chomp $executable; - if ($results_section ne "") { - $executable .= ", " . $results_section; + if ($subsection) { + $executable .= ", " . $subsection; } my $environment; @@ -233,7 +248,7 @@ sub print_codespeed_results { binmode STDOUT, ":utf8" or die "PANIC on binmode: $!"; if ($codespeed) { - print_codespeed_results($results_section); + print_codespeed_results($subsection); } else { print_default_results(); } -- 2.16.0.rc2.45.g09a1bbd803 ``` ## Christian Couder, 2018-01-28 11:18 Subject: [PATCH 2/3] perf/aggregate: add --reponame option Message-ID: <20180128111843.2690-2-chriscool@tuxfamily.org> URL: https://gitlist.dev/e/20180128111843.2690-2-chriscool%40tuxfamily.org In-Reply-To: <20180128111843.2690-1-chriscool@tuxfamily.org> ``` This makes it easier to use the aggregate script on the command line when one wants to get the "environment" fields set in the codespeed output. Previously setting GIT_REPO_NAME was needed for this purpose. Signed-off-by: Christian Couder --- t/perf/aggregate.perl | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl index bbf0f30898..d616d31ca8 100755 --- a/t/perf/aggregate.perl +++ b/t/perf/aggregate.perl @@ -37,7 +37,7 @@ sub format_times { } my (@dirs, %dirnames, %dirabbrevs, %prefixes, @tests, - $codespeed, $subsection); + $codespeed, $subsection, $reponame); while (scalar @ARGV) { my $arg = $ARGV[0]; my $dir; @@ -55,6 +55,15 @@ while (scalar @ARGV) { } next; } + if ($arg eq "--reponame") { + shift @ARGV; + $reponame = $ARGV[0]; + shift @ARGV; + if (! $reponame) { + die "empty reponame"; + } + next; + } last if -f $arg or $arg eq "--"; if (! -d $arg) { my $rev = Git::command_oneline(qw(rev-parse --verify), $arg); @@ -209,15 +218,17 @@ sub print_codespeed_results { $executable .= ", " . $subsection; } - my $environment; - if (exists $ENV{GIT_PERF_REPO_NAME} and $ENV{GIT_PERF_REPO_NAME} ne "") { - $environment = $ENV{GIT_PERF_REPO_NAME}; - } elsif (exists $ENV{GIT_TEST_INSTALLED} and $ENV{GIT_TEST_INSTALLED} ne "") { - $environment = $ENV{GIT_TEST_INSTALLED}; - $environment =~ s|/bin-wrappers$||; - } else { - $environment = `uname -r`; - chomp $environment; + my $environment = $reponame; + if (! $environment) { + if (exists $ENV{GIT_PERF_REPO_NAME} and $ENV{GIT_PERF_REPO_NAME} ne "") { + $environment = $ENV{GIT_PERF_REPO_NAME}; + } elsif (exists $ENV{GIT_TEST_INSTALLED} and $ENV{GIT_TEST_INSTALLED} ne "") { + $environment = $ENV{GIT_TEST_INSTALLED}; + $environment =~ s|/bin-wrappers$||; + } else { + $environment = `uname -r`; + chomp $environment; + } } my @data; -- 2.16.0.rc2.45.g09a1bbd803 ``` ## Christian Couder, 2018-01-28 11:18 Subject: [PATCH 3/3] perf/aggregate: sort JSON fields in output Message-ID: <20180128111843.2690-3-chriscool@tuxfamily.org> URL: https://gitlist.dev/e/20180128111843.2690-3-chriscool%40tuxfamily.org In-Reply-To: <20180128111843.2690-1-chriscool@tuxfamily.org> ``` It is much easier to diff the output against a preivous one when the fields are sorted. Signed-off-by: Christian Couder --- t/perf/aggregate.perl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl index d616d31ca8..fcc0313e65 100755 --- a/t/perf/aggregate.perl +++ b/t/perf/aggregate.perl @@ -253,7 +253,7 @@ sub print_codespeed_results { } } - print to_json(\@data, {utf8 => 1, pretty => 1}), "\n"; + print to_json(\@data, {utf8 => 1, pretty => 1, canonical => 1}), "\n"; } binmode STDOUT, ":utf8" or die "PANIC on binmode: $!"; -- 2.16.0.rc2.45.g09a1bbd803 ``` ## Philip Oakley, 2018-01-28 12:14 Subject: Re: [PATCH 3/3] perf/aggregate: sort JSON fields in output Message-ID: <752336D49C8F4FBE9D77E73330CF4C44@PhilipOakley> URL: https://gitlist.dev/e/752336D49C8F4FBE9D77E73330CF4C44%40PhilipOakley In-Reply-To: <20180128111843.2690-3-chriscool@tuxfamily.org> ``` From: "Christian Couder" > It is much easier to diff the output against a preivous s/preivous/previous/ > one when the fields are sorted. > > Signed-off-by: Christian Couder > --- > t/perf/aggregate.perl | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl > index d616d31ca8..fcc0313e65 100755 > --- a/t/perf/aggregate.perl > +++ b/t/perf/aggregate.perl > @@ -253,7 +253,7 @@ sub print_codespeed_results { > } > } > > - print to_json(\@data, {utf8 => 1, pretty => 1}), "\n"; > + print to_json(\@data, {utf8 => 1, pretty => 1, canonical => 1}), "\n"; > } > > binmode STDOUT, ":utf8" or die "PANIC on binmode: $!"; > -- > 2.16.0.rc2.45.g09a1bbd803 > ``` ## Eric Sunshine, 2018-01-28 19:57 Subject: Re: [PATCH 2/3] perf/aggregate: add --reponame option Message-ID: URL: https://gitlist.dev/e/CAPig%2BcR77RTTw3uJAwK3XmmvzE7umv3_7M%3DNkaDEb%3DuXyNL4FA%40mail.gmail.com In-Reply-To: <20180128111843.2690-2-chriscool@tuxfamily.org> ``` On Sun, Jan 28, 2018 at 6:18 AM, Christian Couder wrote: > This makes it easier to use the aggregate script > on the command line when one wants to get the > "environment" fields set in the codespeed output. > > Previously setting GIT_REPO_NAME was needed > for this purpose. > > Signed-off-by: Christian Couder > --- > diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl > @@ -209,15 +218,17 @@ sub print_codespeed_results { > - my $environment; > - if (exists $ENV{GIT_PERF_REPO_NAME} and $ENV{GIT_PERF_REPO_NAME} ne "") { > - $environment = $ENV{GIT_PERF_REPO_NAME}; > - } elsif (exists $ENV{GIT_TEST_INSTALLED} and $ENV{GIT_TEST_INSTALLED} ne "") { > - $environment = $ENV{GIT_TEST_INSTALLED}; > - $environment =~ s|/bin-wrappers$||; > - } else { > - $environment = `uname -r`; > - chomp $environment; > + my $environment = $reponame; > + if (! $environment) { > + if (exists $ENV{GIT_PERF_REPO_NAME} and $ENV{GIT_PERF_REPO_NAME} ne "") { > + $environment = $ENV{GIT_PERF_REPO_NAME}; > + } elsif (exists $ENV{GIT_TEST_INSTALLED} and $ENV{GIT_TEST_INSTALLED} ne "") { > + $environment = $ENV{GIT_TEST_INSTALLED}; > + $environment =~ s|/bin-wrappers$||; > + } else { > + $environment = `uname -r`; > + chomp $environment; > + } > } Not a big deal, but the extra indentation (and noisy diff) could be avoided like this: my $environment; if ($reponame) { $environment = $reponame; } else if (exists ...) { ...as before.... } ``` ## Christian Couder, 2018-01-29 07:34 Subject: Re: [PATCH 2/3] perf/aggregate: add --reponame option Message-ID: URL: https://gitlist.dev/e/CAP8UFD3-sTC%2Beknu2Vfjx%2BQX7j3AeDaqMMi8L2LQPuSCn3L7Jw%40mail.gmail.com In-Reply-To: ``` On Sun, Jan 28, 2018 at 8:57 PM, Eric Sunshine wrote: > > Not a big deal, but the extra indentation (and noisy diff) could be > avoided like this: > > my $environment; > if ($reponame) { > $environment = $reponame; > } else if (exists ...) { > ...as before.... > } Ok I will reroll with something like this and also the typo fix suggested by Philip. Thanks both. ```