Skip to content

Commit b8dcc45

Browse files
peffgitster
authored andcommitted
perf-lib: use a single filename for all measurement types
The perf tests write files recording the results of tests. These results are later aggregated by 'aggregate.perl'. If the tests are run multiple times, those results are overwritten by the new results. This works just fine as long as there are only perf tests measuring the times, whose results are stored in "$base".times files. However 22bec79 ("t/perf: add infrastructure for measuring sizes", 2018-08-17) introduced a new type of test for measuring the size of something. The results of this are written to "$base".size files. "$base" is essentially made up of the basename of the script plus the test number. So if test numbers shift because a new test was introduced earlier in the script we might end up with both a ".times" and a ".size" file for the same test. In the aggregation script the ".times" file is preferred over the ".size" file, so some size tests might end with performance numbers from a previous run of the test. This is mainly relevant when writing perf tests that check both performance and sizes, and can get quite confusing during developement. We could fix this by doing a more thorough job of cleaning out old ".times" and ".size" files before running each test. However, an even easier solution is to just use the same filename for both types of measurement, meaning we'll always overwrite the previous result. We don't even need to change the file format to distinguish the two; aggregate.perl already decides which is which based on a regex of the content (this may become ambiguous if we add new types in the future, but we could easily add a header field to the file at that point). Based on an initial patch from Thomas Gummerer, who discovered the problem and did all of the analysis (which I stole for the commit message above): https://public-inbox.org/git/[email protected]/ Helped-by: Thomas Gummerer <[email protected]> Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 5fa0f52 commit b8dcc45

File tree

2 files changed

+5
-11
lines changed

2 files changed

+5
-11
lines changed

t/perf/aggregate.perl

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -219,13 +219,7 @@ sub print_default_results {
219219
for my $i (0..$#dirs) {
220220
my $d = $dirs[$i];
221221
my $base = "$resultsdir/$prefixes{$d}$t";
222-
$times{$prefixes{$d}.$t} = [];
223-
foreach my $type (qw(times size)) {
224-
if (-e "$base.$type") {
225-
$times{$prefixes{$d}.$t} = [get_times("$base.$type")];
226-
last;
227-
}
228-
}
222+
$times{$prefixes{$d}.$t} = [get_times("$base.result")];
229223
my ($r,$u,$s) = @{$times{$prefixes{$d}.$t}};
230224
my $w = length format_times($r,$u,$s,$firstr);
231225
$colwidth[$i] = $w if $w > $colwidth[$i];
@@ -267,7 +261,7 @@ sub print_sorted_results {
267261
my ($prevr, $prevu, $prevs, $prevrev);
268262
for my $i (0..$#dirs) {
269263
my $d = $dirs[$i];
270-
my ($r, $u, $s) = get_times("$resultsdir/$prefixes{$d}$t.times");
264+
my ($r, $u, $s) = get_times("$resultsdir/$prefixes{$d}$t.result");
271265
if ($i > 0 and defined $r and defined $prevr and $prevr > 0) {
272266
my $percent = 100.0 * ($r - $prevr) / $prevr;
273267
push @evolutions, { "percent" => $percent,
@@ -327,7 +321,7 @@ sub print_codespeed_results {
327321
my $commitid = $prefixes{$d};
328322
$commitid =~ s/^build_//;
329323
$commitid =~ s/\.$//;
330-
my ($result_value, $u, $s) = get_times("$resultsdir/$prefixes{$d}$t.times");
324+
my ($result_value, $u, $s) = get_times("$resultsdir/$prefixes{$d}$t.result");
331325

332326
my %vals = (
333327
"commitid" => $commitid,

t/perf/perf-lib.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ test_perf_ () {
214214
else
215215
test_ok_ "$1"
216216
fi
217-
"$TEST_DIRECTORY"/perf/min_time.perl test_time.* >"$base".times
217+
"$TEST_DIRECTORY"/perf/min_time.perl test_time.* >"$base".result
218218
}
219219

220220
test_perf () {
@@ -223,7 +223,7 @@ test_perf () {
223223

224224
test_size_ () {
225225
say >&3 "running: $2"
226-
if test_eval_ "$2" 3>"$base".size; then
226+
if test_eval_ "$2" 3>"$base".result; then
227227
test_ok_ "$1"
228228
else
229229
test_failure_ "$@"

0 commit comments

Comments
 (0)