The perf tests initially only saved the minimum measurement (chosen according to wall time). This is common for timings, as it tends to measure the shortest time in which the machine can run the test. It is also a bit more forgiving for the user if he ran some other task during one of the tests. However, experiments with p3400-rebase.sh showed that for longer-running tasks with a lot of kernel involvement (such as a shell script and its constant forking) the variance is so high that the minimum becomes extremely unstable. The best-of-10 (!) timings fluctuated by around 5% in extreme cases. Switch to the average, as that tends to cancel out the variance for sufficiently large numbers of $GIT_PERF_REPEAT_COUNT. The catch is that now we can no longer write off the initial (possibly) cache-cold timing. So the test is now run 1+n times, and the first run is discarded. Since we're already rewriting that paragraph, also correctly state the default value of GIT_PERF_REPEAT_COUNT, which is 3. The shift of the averaging logic to aggregate.perl is not, strictly speaking, necessary. However, min_time.perl would have had to be renamed anyway. It also sets the stage for the next patch. Signed-off-by: Thomas Rast <trast@xxxxxxxxxxxxxxx> --- t/perf/README | 8 +++++--- t/perf/aggregate.perl | 25 +++++++++++++++++-------- t/perf/min_time.perl | 21 --------------------- t/perf/perf-lib.sh | 5 +++-- 4 files changed, 25 insertions(+), 34 deletions(-) delete mode 100755 t/perf/min_time.perl diff --git a/t/perf/README b/t/perf/README index b2dbad4..fa94eb5 100644 --- a/t/perf/README +++ b/t/perf/README @@ -54,9 +54,11 @@ anything beforehand. You can set the following variables (also in your config.mak): - GIT_PERF_REPEAT_COUNT - Number of times a test should be repeated for best-of-N - measurements. Defaults to 5. + GIT_PERF_REPEAT_COUNT='n' + Number of times a test should be repeated. The test is run + 'n+1' times: once to warm up the caches and 'n' more times to + gather the measurements. The first run is discarded, and the + other 'n' runs are averaged. Defaults to 3. GIT_PERF_MAKE_OPTS Options to use when automatically building a git tree for diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl index c0afa0b..9c781fa 100755 --- a/t/perf/aggregate.perl +++ b/t/perf/aggregate.perl @@ -10,13 +10,22 @@ sub get_times { my $name = shift; open my $fh, "<", $name or return undef; - my $line = <$fh>; - return undef if not defined $line; + my $sum_rt = 0.0; + my $sum_u = 0.0; + my $sum_s = 0.0; + my $n = 0; + while (<$fh>) { + /^(?:(\d+):)?(\d+):(\d+(?:\.\d+)?) (\d+(?:\.\d+)?) (\d+(?:\.\d+)?)$/ + or die "bad input line: $_"; + my $rt = ((defined $1 ? $1 : 0.0)*60+$2)*60+$3; + $sum_rt += $rt; + $sum_u += $4; + $sum_s += $5; + $n++; + } + return undef if !$n; close $fh or die "cannot close $name: $!"; - $line =~ /^(?:(\d+):)?(\d+):(\d+(?:\.\d+)?) (\d+(?:\.\d+)?) (\d+(?:\.\d+)?)$/ - or die "bad input line: $line"; - my $rt = ((defined $1 ? $1 : 0.0)*60+$2)*60+$3; - return ($rt, $4, $5); + return ($sum_rt/$n, $sum_u/$n, $sum_s/$n); } sub format_times { @@ -140,8 +149,8 @@ sub have_slash { for my $i (0..$#dirs) { my $d = $dirs[$i]; $times{$prefixes{$d}.$t} = [get_times("test-results/$prefixes{$d}$t.times")]; - my ($r,$u,$s) = @{$times{$prefixes{$d}.$t}}; - my $w = length format_times($r,$u,$s,$firstr); + my ($r,$u,$s,$sign) = @{$times{$prefixes{$d}.$t}}; + my $w = length format_times($r,$u,$s,$sign,$firstr); $colwidth[$i] = $w if $w > $colwidth[$i]; $firstr = $r unless defined $firstr; } diff --git a/t/perf/min_time.perl b/t/perf/min_time.perl deleted file mode 100755 index c1a2717..0000000 --- a/t/perf/min_time.perl +++ /dev/null @@ -1,21 +0,0 @@ -#!/usr/bin/perl - -my $minrt = 1e100; -my $min; - -while (<>) { - # [h:]m:s.xx U.xx S.xx - /^(?:(\d+):)?(\d+):(\d+(?:\.\d+)?) (\d+(?:\.\d+)?) (\d+(?:\.\d+)?)$/ - or die "bad input line: $_"; - my $rt = ((defined $1 ? $1 : 0.0)*60+$2)*60+$3; - if ($rt < $minrt) { - $min = $_; - $minrt = $rt; - } -} - -if (!defined $min) { - die "no input found"; -} - -print $min; diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh index 5580c22..a13f105 100644 --- a/t/perf/perf-lib.sh +++ b/t/perf/perf-lib.sh @@ -163,7 +163,7 @@ test_perf () { else echo "perf $test_count - $1:" fi - for i in $(seq 1 $GIT_PERF_REPEAT_COUNT); do + for i in $(seq 0 $GIT_PERF_REPEAT_COUNT); do say >&3 "running: $2" if test_run_perf_ "$2" then @@ -184,7 +184,8 @@ test_perf () { test_ok_ "$1" fi base="$perf_results_dir"/"$perf_results_prefix$(basename "$0" .sh)"."$test_count" - "$TEST_DIRECTORY"/perf/min_time.perl test_time.* >"$base".times + rm test_time.0 + cat test_time.* >"$base".times fi echo >&3 "" } -- 1.7.10.rc0.230.g16d90 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html