git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 04/11] perf: display average instead of minimum time

From
Thomas Rast <trast@student.ethz.ch>
Date
Mar 12, 2012, 15:10 UTC
Message-ID
<ac09e8e93b110382409a8cd9d6c29189056da100.1331561353.git.trast@student.ethz.ch>
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
Previous: Thomas RastNext: Thomas Rast
Message 9 of 20 in “perf improvements past v1.7.10”
  1. 00/11 perf improvements past v1.7.10Thomas Rast, Mar 12, 2012
  2. 01/11 perf/aggregate: load Git.pm from the build treeThomas Rast, Mar 12, 2012
  3. 02/11 Introduce a performance test for git-rebaseThomas Rast, Mar 12, 2012
  4. Thomas RastMar 12, 2012
  5. Junio C HamanoMar 12, 2012
  6. Thomas RastMar 12, 2012
  7. Junio C HamanoMar 12, 2012
  8. 03/11 Introduce a performance test for git-blameThomas Rast, Mar 12, 2012
  9. 04/11 perf: display average instead of minimum timeThomas Rast, Mar 12, 2012
  10. 05/11 perf: suppress aggregation also in 'run'Thomas Rast, Mar 12, 2012
  11. 06/11 perf: dereference to a commit when buildingThomas Rast, Mar 12, 2012
  12. 07/11 perf: convert realtime to seconds when collecting runsThomas Rast, Mar 12, 2012
  13. 08/11 perf/aggregate: optionally include a t-test scoreThomas Rast, Mar 12, 2012
  14. 09/11 perf/run: allow skipping some revisionsThomas Rast, Mar 12, 2012
  15. Nguyen Thai Ngoc DuyMar 12, 2012
  16. Thomas RastMar 12, 2012
  17. Jakub NarebskiMar 12, 2012
  18. Thomas RastMar 12, 2012
  19. 10/11 perf: implement a test-selection featureThomas Rast, Mar 12, 2012
  20. 11/11 perf: add a bisection toolThomas Rast, Mar 12, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.