* [PATCH v1 1/2] perf/aggregate: add display_dir()
2018-03-23 14:00 [PATCH v1 0/2] perf/aggregate: sort result by regression Christian Couder
@ 2018-03-23 14:00 ` Christian Couder
2018-03-23 14:00 ` [PATCH v1 2/2] perf/aggregate: add --sortbyregression option Christian Couder
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Christian Couder @ 2018-03-23 14:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Thomas Rast,
Ævar Arnfjörð Bjarmason, Christian Couder,
Eric Sunshine, Philip Oakley
This new helper function will be reused in a subsequent
commit.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
t/perf/aggregate.perl | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
index 821cf1498b..b9c0e3243d 100755
--- a/t/perf/aggregate.perl
+++ b/t/perf/aggregate.perl
@@ -147,6 +147,11 @@ sub have_slash {
return 0;
}
+sub display_dir {
+ my ($d) = @_;
+ return exists $dirabbrevs{$d} ? $dirabbrevs{$d} : $dirnames{$d};
+}
+
sub print_default_results {
my %descrs;
my $descrlen = 4; # "Test"
@@ -168,8 +173,7 @@ sub print_default_results {
my %times;
my @colwidth = ((0)x@dirs);
for my $i (0..$#dirs) {
- my $d = $dirs[$i];
- my $w = length (exists $dirabbrevs{$d} ? $dirabbrevs{$d} : $dirnames{$d});
+ my $w = length display_dir($dirs[$i]);
$colwidth[$i] = $w if $w > $colwidth[$i];
}
for my $t (@subtests) {
@@ -188,8 +192,7 @@ sub print_default_results {
printf "%-${descrlen}s", "Test";
for my $i (0..$#dirs) {
- my $d = $dirs[$i];
- printf " %-$colwidth[$i]s", (exists $dirabbrevs{$d} ? $dirabbrevs{$d} : $dirnames{$d});
+ printf " %-$colwidth[$i]s", display_dir($dirs[$i]);
}
print "\n";
print "-"x$totalwidth, "\n";
--
2.17.0.rc0.37.g8f476fabe9
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v1 2/2] perf/aggregate: add --sortbyregression option
2018-03-23 14:00 [PATCH v1 0/2] perf/aggregate: sort result by regression Christian Couder
2018-03-23 14:00 ` [PATCH v1 1/2] perf/aggregate: add display_dir() Christian Couder
@ 2018-03-23 14:00 ` Christian Couder
2018-03-23 14:08 ` [PATCH v1 0/2] perf/aggregate: sort result by regression Christian Couder
2018-03-23 21:16 ` Junio C Hamano
3 siblings, 0 replies; 6+ messages in thread
From: Christian Couder @ 2018-03-23 14:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Thomas Rast,
Ævar Arnfjörð Bjarmason, Christian Couder,
Eric Sunshine, Philip Oakley
One of the most interesting thing one can be interested
in when looking at performance test results is possible
performance regressions.
This new option makes it easy to spot such possible
regressions.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
t/perf/aggregate.perl | 48 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 47 insertions(+), 1 deletion(-)
diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
index b9c0e3243d..9d032b286e 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, $reponame);
+ $codespeed, $sortbyregression, $subsection, $reponame);
while (scalar @ARGV) {
my $arg = $ARGV[0];
my $dir;
@@ -46,6 +46,11 @@ while (scalar @ARGV) {
shift @ARGV;
next;
}
+ if ($arg eq "--sortbyregression") {
+ $sortbyregression = 1;
+ shift @ARGV;
+ next;
+ }
if ($arg eq "--subsection") {
shift @ARGV;
$subsection = $ARGV[0];
@@ -209,6 +214,45 @@ sub print_default_results {
}
}
+sub print_sortbyregression_results {
+ my ($subsection) = @_;
+
+ my @evolutions;
+ for my $t (@subtests) {
+ my ($prevr, $prevu, $prevs, $prevrev);
+ for my $i (0..$#dirs) {
+ my $d = $dirs[$i];
+ my ($r, $u, $s) = get_times("$resultsdir/$prefixes{$d}$t.times");
+ if ($i > 0 and defined $r and defined $prevr and $prevr > 0) {
+ my $percent = 100.0 * ($r - $prevr) / $prevr;
+ push @evolutions, { "percent" => $percent,
+ "test" => $t,
+ "prevrev" => $prevrev,
+ "rev" => $d,
+ "prevr" => $prevr,
+ "r" => $r,
+ "prevu" => $prevu,
+ "u" => $u,
+ "prevs" => $prevs,
+ "s" => $s};
+ }
+ ($prevr, $prevu, $prevs, $prevrev) = ($r, $u, $s, $d);
+ }
+ }
+
+ my @sorted_evolutions = sort { $b->{percent} <=> $a->{percent} } @evolutions;
+
+ for my $e (@sorted_evolutions) {
+ printf "%+.1f%%", $e->{percent};
+ print " " . $e->{test};
+ print " " . format_times($e->{prevr}, $e->{prevu}, $e->{prevs});
+ print " " . format_times($e->{r}, $e->{u}, $e->{s});
+ print " " . display_dir($e->{prevrev});
+ print " " . display_dir($e->{rev});
+ print "\n";
+ }
+}
+
sub print_codespeed_results {
my ($subsection) = @_;
@@ -263,6 +307,8 @@ binmode STDOUT, ":utf8" or die "PANIC on binmode: $!";
if ($codespeed) {
print_codespeed_results($subsection);
+} elsif ($sortbyregression) {
+ print_sortbyregression_results($subsection);
} else {
print_default_results();
}
--
2.17.0.rc0.37.g8f476fabe9
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v1 0/2] perf/aggregate: sort result by regression
2018-03-23 14:00 [PATCH v1 0/2] perf/aggregate: sort result by regression Christian Couder
` (2 preceding siblings ...)
2018-03-23 14:08 ` [PATCH v1 0/2] perf/aggregate: sort result by regression Christian Couder
@ 2018-03-23 21:16 ` Junio C Hamano
2018-03-24 7:05 ` Christian Couder
3 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2018-03-23 21:16 UTC (permalink / raw)
To: Christian Couder
Cc: git, Jeff King, Thomas Rast,
Ævar Arnfjörð Bjarmason, Christian Couder,
Eric Sunshine, Philip Oakley
Christian Couder <christian.couder@gmail.com> writes:
> This small patch series makes it easy to spot big performance
> regressions, so that they can later be investigated.
>
> For example:
>
> $ ./aggregate.perl --sortbyregression --subsection "without libpcre" v2.14.3 v2.15.1 v2.16.2 p4220-log-grep-engines.sh
Are we comfortable with the idea that other kinds of sorting, when
invented in the future, would have to say
$ ./aggregate.perl --sortbysomethingelse --subsection "without libpcre" \
A B C p4220-log-grep-engines.sh
or will we regret that and wish if we could write it as
$ ./aggregate.perl --sort-by=somethingelse --subsection "without libpcre" \
A B C p4220-log-grep-engines.sh
If the latter, perhaps we should use --soft-by=regression from day one.
Do we expect that "taking a lot more more rtime than the previous"
will stay to be the only kind of "regression" we care about in the
context of t/perf? If so, there is no need for further suggestion,
but if not, perhaps we should plan if/how we could also parameterize
the "rtime" part from the command line. E.g.
$ ./aggregate.perl --sort-by=regression:stime
might be a way to say "we only care about the stime part" in the
future, even though --sort-by=regression may be a short-hand to say
"we care about rtime regression" i.e. "--sort-by=regression:rtime".
^ permalink raw reply [flat|nested] 6+ messages in thread