threads / patch / 47705

patch, 3 partsperf/aggregate: add --subsection option

Subject: [PATCH 1/3] perf/aggregate: add --subsection option

## tl;dr

6 messages between Jan 28, 2018 and Jan 29, 2018. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Christian Couder· Jan 28, 2018, 11:18 UTC · lore

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 <chriscool@tuxfamily.org>
---
 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.

Show changes to t/perf/aggregate.perl +24 −9

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· Jan 28, 2018, 11:18 UTC · re: Christian Couder · lore

[PATCH 2/3] perf/aggregate: add --reponame option

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 <chriscool@tuxfamily.org>
---
 t/perf/aggregate.perl | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

Show changes to t/perf/aggregate.perl +21 −10

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

Eric Sunshine· Jan 28, 2018, 19:57 UTC · re: Christian Couder · lore

Re: [PATCH 2/3] perf/aggregate: add --reponame option

On Sun, Jan 28, 2018 at 6:18 AM, Christian Couder <christian.couder@gmail.com> wrote:

Show 32 quoted lines

> 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 <chriscool@tuxfamily.org>
> ---
> 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· Jan 29, 2018, 07:34 UTC · re: Eric Sunshine · lore

Re: [PATCH 2/3] perf/aggregate: add --reponame option

On Sun, Jan 28, 2018 at 8:57 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:

Show 10 quoted lines

>
> 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.

Christian Couder· Jan 28, 2018, 11:18 UTC · re: Christian Couder · lore

[PATCH 3/3] perf/aggregate: sort JSON fields in output

It is much easier to diff the output against a preivous one when the fields are sorted.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/perf/aggregate.perl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to t/perf/aggregate.perl +1 −1

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· Jan 28, 2018, 12:14 UTC · re: Christian Couder · lore

Re: [PATCH 3/3] perf/aggregate: sort JSON fields in output

From: "Christian Couder" <christian.couder@gmail.com>

> It is much easier to diff the output against a preivous

s/preivous/previous/

Show 23 quoted lines

> one when the fields are sorted.
> 
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
> 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
>