git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Rast <trast@student.ethz.ch>
To: <git@vger.kernel.org>
Subject: [PATCH 04/11] perf: display average instead of minimum time
Date: Mon, 12 Mar 2012 16:10:00 +0100	[thread overview]
Message-ID: <ac09e8e93b110382409a8cd9d6c29189056da100.1331561353.git.trast@student.ethz.ch> (raw)
In-Reply-To: <cover.1331561353.git.trast@student.ethz.ch>

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@student.ethz.ch>
---
 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

  parent reply	other threads:[~2012-03-12 15:11 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-12 15:09 [PATCH 00/11] perf improvements past v1.7.10 Thomas Rast
2012-03-12 15:09 ` [PATCH 01/11] perf/aggregate: load Git.pm from the build tree Thomas Rast
2012-03-12 15:09 ` [PATCH 02/11] Introduce a performance test for git-rebase Thomas Rast
2012-03-12 17:41   ` Thomas Rast
2012-03-12 19:45     ` Junio C Hamano
2012-03-12 20:20       ` Thomas Rast
2012-03-12 20:59         ` Junio C Hamano
2012-03-12 15:09 ` [PATCH 03/11] Introduce a performance test for git-blame Thomas Rast
2012-03-12 15:10 ` Thomas Rast [this message]
2012-03-12 15:10 ` [PATCH 05/11] perf: suppress aggregation also in 'run' Thomas Rast
2012-03-12 15:10 ` [PATCH 06/11] perf: dereference to a commit when building Thomas Rast
2012-03-12 15:10 ` [PATCH 07/11] perf: convert realtime to seconds when collecting runs Thomas Rast
2012-03-12 15:10 ` [PATCH 08/11] perf/aggregate: optionally include a t-test score Thomas Rast
2012-03-12 15:10 ` [PATCH 09/11] perf/run: allow skipping some revisions Thomas Rast
2012-03-12 15:28 ` [PATCH 00/11] perf improvements past v1.7.10 Nguyen Thai Ngoc Duy
2012-03-12 16:35   ` Thomas Rast
2012-03-12 16:09 ` Jakub Narebski
2012-03-12 16:30   ` Thomas Rast
2012-03-12 16:27 ` [PATCH 10/11] perf: implement a test-selection feature Thomas Rast
2012-03-12 16:27 ` [PATCH 11/11] perf: add a bisection tool Thomas Rast

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ac09e8e93b110382409a8cd9d6c29189056da100.1331561353.git.trast@student.ethz.ch \
    --to=trast@student.ethz.ch \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).