threads / patch / 47496

v2, 7 partsCodespeed perf results

Subject: [PATCH v2 0/7] Codespeed perf results

## tl;dr

12 messages between Dec 26, 2017 and Dec 28, 2017. Diffs are folded; open one to read it.

replies: 11people: 2as markdown or json

Christian Couder· Dec 26, 2017, 21:59 UTC · lore

This patch series is built on top of cc/perf-run-config which recently graduated to master.

It makes it possible to send perf results to a Codespeed server. See https://github.com/tobami/codespeed/ and web sites like http://speed.pypy.org/ which are using Codespeed.

The end goal would be to have such a server always available to track how the different git commands perform over time on different kind of repos (small, medium, large, ...) with different optimizations on and off (split-index, libpcre2, BLK_SHA1, ...)

With this series and a config file like:
$ cat perf.conf
[perf]
        dirsOrRevs = v2.12.0 v2.13.0
        repeatCount = 10
	sendToCodespeed = http://localhost:8000
	repoName = Git repo
[perf "with libpcre"]
        makeOpts = "DEVELOPER=1 USE_LIBPCRE=YesPlease"
[perf "without libpcre"]
        makeOpts = "DEVELOPER=1"
One should be able to just launch:
$ ./run --config perf.conf p7810-grep.sh

and then get nice graphs in a Codespeed instance running on http://localhost:8000.

Caveat ~~~~~~

For now one has to create the "Git repo" environment in the Codespeed admin interface. (We send the perf.repoName config variable in the "environment" Codespeed field.) This is because Codespeed requires the environment fields to be created and does not provide a simple way to create these fields programmatically.

There are discussions on a Codespeed issue (https://github.com/tobami/codespeed/issues/232) about creating a proper API for Codespeed that could address this problem in the future.

Changes since previous version ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  - Fixed the way the 'executable' field sent to Codespeed is set in
    `perf/aggregate.perl` in patch 3/7. We now use `uname -s -m` to
    which we concatenate the perf.repoName config value if it is set,
    as suggested by Junio and Eric.
  - Fixed the name of the GIT_PERF_REPO_NAME variable in patches 3/7
    and 7/7. It was GIT_TEST_REPO_NAME in some places.
  - Fixed how the conf_opt argument is added to the
    get_var_from_env_or_config() function in patch 4/7. It was added
    as the last, so fifth, argument, but it is simpler and makes more
    sense to keep the default value argument as the last argument, so
    now the conf_opt argument is added as the fourth argument.
  - What patch 4/7 did was previously done in 2 patches in the
    previous version (patches 4/8 and 5/8), but as we are not doing
    exactly the same thing (see the above item) it is simpler to do it
    in only one patch instead of two.
  - We now use the --int type specifier when getting the
    perf.repeatCount config variable in patch 4/7.

Links ~~~~~

This patch series:
https://github.com/chriscool/git/commits/codespeed
Previous version (v1):
https://github.com/chriscool/git/commits/codespeed1
Discussions about v1:
https://public-inbox.org/git/CAP8UFD3Q4h-aYBDABSPOW948LQYVydWZ1hLPAD+kr9ZpXVZiaQ@mail.gmail.com/
Discussions about the cc/perf-run-config patch series:

v1: https://public-inbox.org/git/20170713065050.19215-1-chriscool@tuxfamily.org/ v2: https://public-inbox.org/git/CAP8UFD2j-UFh+9awz91gtZ-jusq7EUOExMgURO59vpf29jXS4A@mail.gmail.com/

Christian Couder (7):
  perf/aggregate: fix checking ENV{GIT_PERF_SUBSECTION}
  perf/aggregate: refactor printing results
  perf/aggregate: implement codespeed JSON output
  perf/run: add conf_opts argument to get_var_from_env_or_config()
  perf/run: learn about perf.codespeedOutput
  perf/run: learn to send output to codespeed server
  perf/run: read GIT_TEST_REPO_NAME from perf.repoName
 t/perf/aggregate.perl | 163 +++++++++++++++++++++++++++++++++++---------------
 t/perf/run            |  31 ++++++++--
 2 files changed, 140 insertions(+), 54 deletions(-)
-- 
2.15.1.361.g8b07d831d0
Christian Couder· Dec 26, 2017, 21:59 UTC · re: Christian Couder · lore

[PATCH v2 2/7] perf/aggregate: refactor printing results

As we want to implement another kind of output than the current output for the perf test results, let's refactor the existing code that outputs the results in its own print_default_results() function.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/perf/aggregate.perl | 96 +++++++++++++++++++++++++++------------------------
 1 file changed, 50 insertions(+), 46 deletions(-)
Show changes to t/perf/aggregate.perl +50 −46
diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
index 769d418708..3609cb5dc3 100755
--- a/t/perf/aggregate.perl
+++ b/t/perf/aggregate.perl
@@ -100,13 +100,6 @@ sub read_descr {
 	return $line;
 }
 
-my %descrs;
-my $descrlen = 4; # "Test"
-for my $t (@subtests) {
-	$descrs{$t} = $shorttests{$t}.": ".read_descr("$resultsdir/$t.descr");
-	$descrlen = length $descrs{$t} if length $descrs{$t}>$descrlen;
-}
-
 sub have_duplicate {
 	my %seen;
 	for (@_) {
@@ -122,54 +115,65 @@ sub have_slash {
 	return 0;
 }
 
-my %newdirabbrevs = %dirabbrevs;
-while (!have_duplicate(values %newdirabbrevs)) {
-	%dirabbrevs = %newdirabbrevs;
-	last if !have_slash(values %dirabbrevs);
-	%newdirabbrevs = %dirabbrevs;
-	for (values %newdirabbrevs) {
-		s{^[^/]*/}{};
+sub print_default_results {
+	my %descrs;
+	my $descrlen = 4; # "Test"
+	for my $t (@subtests) {
+		$descrs{$t} = $shorttests{$t}.": ".read_descr("$resultsdir/$t.descr");
+		$descrlen = length $descrs{$t} if length $descrs{$t}>$descrlen;
 	}
-}
 
-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});
-	$colwidth[$i] = $w if $w > $colwidth[$i];
-}
-for my $t (@subtests) {
-	my $firstr;
+	my %newdirabbrevs = %dirabbrevs;
+	while (!have_duplicate(values %newdirabbrevs)) {
+		%dirabbrevs = %newdirabbrevs;
+		last if !have_slash(values %dirabbrevs);
+		%newdirabbrevs = %dirabbrevs;
+		for (values %newdirabbrevs) {
+			s{^[^/]*/}{};
+		}
+	}
+
+	my %times;
+	my @colwidth = ((0)x@dirs);
 	for my $i (0..$#dirs) {
 		my $d = $dirs[$i];
-		$times{$prefixes{$d}.$t} = [get_times("$resultsdir/$prefixes{$d}$t.times")];
-		my ($r,$u,$s) = @{$times{$prefixes{$d}.$t}};
-		my $w = length format_times($r,$u,$s,$firstr);
+		my $w = length (exists $dirabbrevs{$d} ? $dirabbrevs{$d} : $dirnames{$d});
 		$colwidth[$i] = $w if $w > $colwidth[$i];
-		$firstr = $r unless defined $firstr;
 	}
-}
-my $totalwidth = 3*@dirs+$descrlen;
-$totalwidth += $_ for (@colwidth);
-
-binmode STDOUT, ":utf8" or die "PANIC on binmode: $!";
+	for my $t (@subtests) {
+		my $firstr;
+		for my $i (0..$#dirs) {
+			my $d = $dirs[$i];
+			$times{$prefixes{$d}.$t} = [get_times("$resultsdir/$prefixes{$d}$t.times")];
+			my ($r,$u,$s) = @{$times{$prefixes{$d}.$t}};
+			my $w = length format_times($r,$u,$s,$firstr);
+			$colwidth[$i] = $w if $w > $colwidth[$i];
+			$firstr = $r unless defined $firstr;
+		}
+	}
+	my $totalwidth = 3*@dirs+$descrlen;
+	$totalwidth += $_ for (@colwidth);
 
-printf "%-${descrlen}s", "Test";
-for my $i (0..$#dirs) {
-	my $d = $dirs[$i];
-	printf "   %-$colwidth[$i]s", (exists $dirabbrevs{$d} ? $dirabbrevs{$d} : $dirnames{$d});
-}
-print "\n";
-print "-"x$totalwidth, "\n";
-for my $t (@subtests) {
-	printf "%-${descrlen}s", $descrs{$t};
-	my $firstr;
+	printf "%-${descrlen}s", "Test";
 	for my $i (0..$#dirs) {
 		my $d = $dirs[$i];
-		my ($r,$u,$s) = @{$times{$prefixes{$d}.$t}};
-		printf "   %-$colwidth[$i]s", format_times($r,$u,$s,$firstr);
-		$firstr = $r unless defined $firstr;
+		printf "   %-$colwidth[$i]s", (exists $dirabbrevs{$d} ? $dirabbrevs{$d} : $dirnames{$d});
 	}
 	print "\n";
+	print "-"x$totalwidth, "\n";
+	for my $t (@subtests) {
+		printf "%-${descrlen}s", $descrs{$t};
+		my $firstr;
+		for my $i (0..$#dirs) {
+			my $d = $dirs[$i];
+			my ($r,$u,$s) = @{$times{$prefixes{$d}.$t}};
+			printf "   %-$colwidth[$i]s", format_times($r,$u,$s,$firstr);
+			$firstr = $r unless defined $firstr;
+		}
+		print "\n";
+	}
 }
+
+binmode STDOUT, ":utf8" or die "PANIC on binmode: $!";
+
+print_default_results();
-- 
2.15.1.361.g8b07d831d0
Christian Couder· Dec 26, 2017, 21:59 UTC · re: Christian Couder · lore

[PATCH v2 5/7] perf/run: learn about perf.codespeedOutput

Let's make it possible to set in a config file the output format (regular or codespeed) of the perf tests.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/perf/run | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
Show changes to t/perf/run +6 −1
diff --git a/t/perf/run b/t/perf/run
index 214d658172..4e62d6bb3f 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -144,10 +144,15 @@ run_subsection () {
 		set -- . "$@"
 	fi
 
+	codespeed_opt=
+	test "$GIT_PERF_CODESPEED_OUTPUT" = "true" && codespeed_opt="--codespeed"
+
 	run_dirs "$@"
-	./aggregate.perl "$@"
+	./aggregate.perl $codespeed_opt "$@"
 }
 
+get_var_from_env_or_config "GIT_PERF_CODESPEED_OUTPUT" "perf" "codespeedOutput" "--bool"
+
 cd "$(dirname $0)"
 . ../../GIT-BUILD-OPTIONS
 
-- 
2.15.1.361.g8b07d831d0
Christian Couder· Dec 26, 2017, 21:59 UTC · re: Christian Couder · lore

[PATCH v2 7/7] perf/run: read GIT_TEST_REPO_NAME from perf.repoName

The GIT_PERF_REPO_NAME env variable is used in the `aggregate.perl` script to set the 'environment' field in the JSON Codespeed output.

Let's make it easy to set this variable by setting it in a config file.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/perf/run | 3 +++
 1 file changed, 3 insertions(+)
Show changes to t/perf/run +3 −0
diff --git a/t/perf/run b/t/perf/run
index ef56396546..1a100d6134 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -137,6 +137,9 @@ run_subsection () {
 	get_var_from_env_or_config "GIT_PERF_MAKE_COMMAND" "perf" "makeCommand"
 	get_var_from_env_or_config "GIT_PERF_MAKE_OPTS" "perf" "makeOpts"
 
+	get_var_from_env_or_config "GIT_PERF_REPO_NAME" "perf" "repoName"
+	export GIT_PERF_REPO_NAME
+
 	GIT_PERF_AGGREGATING_LATER=t
 	export GIT_PERF_AGGREGATING_LATER
 
-- 
2.15.1.361.g8b07d831d0
Christian Couder· Dec 26, 2017, 21:59 UTC · re: Christian Couder · lore

[PATCH v2 1/7] perf/aggregate: fix checking ENV{GIT_PERF_SUBSECTION}

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/perf/aggregate.perl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to t/perf/aggregate.perl +1 −1
diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
index e401208488..769d418708 100755
--- a/t/perf/aggregate.perl
+++ b/t/perf/aggregate.perl
@@ -70,7 +70,7 @@ if (not @tests) {
 }
 
 my $resultsdir = "test-results";
-if ($ENV{GIT_PERF_SUBSECTION} ne "") {
+if (exists $ENV{GIT_PERF_SUBSECTION} and $ENV{GIT_PERF_SUBSECTION} ne "") {
 	$resultsdir .= "/" . $ENV{GIT_PERF_SUBSECTION};
 }
 
-- 
2.15.1.361.g8b07d831d0
Junio C Hamano· Dec 28, 2017, 19:40 UTC · re: Christian Couder · lore

Re: [PATCH v2 1/7] perf/aggregate: fix checking ENV{GIT_PERF_SUBSECTION}

Christian Couder <christian.couder@gmail.com> writes:
Show 17 quoted lines
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  t/perf/aggregate.perl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
> index e401208488..769d418708 100755
> --- a/t/perf/aggregate.perl
> +++ b/t/perf/aggregate.perl
> @@ -70,7 +70,7 @@ if (not @tests) {
>  }
>  
>  my $resultsdir = "test-results";
> -if ($ENV{GIT_PERF_SUBSECTION} ne "") {
> +if (exists $ENV{GIT_PERF_SUBSECTION} and $ENV{GIT_PERF_SUBSECTION} ne "") {
>  	$resultsdir .= "/" . $ENV{GIT_PERF_SUBSECTION};
>  }

It appears that this is the only remaining environment variable reference that could trigger comparison between undef and "" that may be flagged by use of strict & warnings? A good change.

Christian Couder· Dec 26, 2017, 21:59 UTC · re: Christian Couder · lore

[PATCH v2 3/7] perf/aggregate: implement codespeed JSON output

Codespeed (https://github.com/tobami/codespeed/) is an open source project that can be used to track how some software performs over time. It stores performance test results in a database and can show nice graphs and charts on a web interface.

As it can be interesting to Codespeed to see how Git performance evolves over time and releases, let's implement a Codespeed output in "perf/aggregate.perl".

Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/perf/aggregate.perl | 67 +++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 65 insertions(+), 2 deletions(-)
Show changes to t/perf/aggregate.perl +65 −2
diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
index 3609cb5dc3..34d74fc015 100755
--- a/t/perf/aggregate.perl
+++ b/t/perf/aggregate.perl
@@ -35,10 +35,15 @@ sub format_times {
 	return $out;
 }
 
-my (@dirs, %dirnames, %dirabbrevs, %prefixes, @tests);
+my (@dirs, %dirnames, %dirabbrevs, %prefixes, @tests, $codespeed);
 while (scalar @ARGV) {
 	my $arg = $ARGV[0];
 	my $dir;
+	if ($arg eq "--codespeed") {
+		$codespeed = 1;
+		shift @ARGV;
+		next;
+	}
 	last if -f $arg or $arg eq "--";
 	if (! -d $arg) {
 		my $rev = Git::command_oneline(qw(rev-parse --verify), $arg);
@@ -70,8 +75,10 @@ if (not @tests) {
 }
 
 my $resultsdir = "test-results";
+my $results_section = "";
 if (exists $ENV{GIT_PERF_SUBSECTION} and $ENV{GIT_PERF_SUBSECTION} ne "") {
 	$resultsdir .= "/" . $ENV{GIT_PERF_SUBSECTION};
+	$results_section = $ENV{GIT_PERF_SUBSECTION};
 }
 
 my @subtests;
@@ -174,6 +181,62 @@ sub print_default_results {
 	}
 }
 
+sub print_codespeed_results {
+	my ($results_section) = @_;
+
+	my $project = "Git";
+
+	my $executable = `uname -s -m`;
+	chomp $executable;
+
+	if ($results_section ne "") {
+		$executable .= ", " . $results_section;
+	}
+
+	my $environment;
+	if (exists $ENV{GIT_PERF_REPO_NAME} and $ENV{GIT_PERF_REPO_NAME} ne "") {
+		$environment = $ENV{GIT_PERF_REPO_NAME};
+	} elsif (exists $ENV{GIT_TEST_INSTALLED} and $ENV{GIT_TEST_INSTALLED} ne "") {
+		$environment = $ENV{GIT_TEST_INSTALLED};
+		$environment =~ s|/bin-wrappers$||;
+	} else {
+		$environment = `uname -r`;
+		chomp $environment;
+	}
+
+	my @data;
+
+	for my $t (@subtests) {
+		for my $d (@dirs) {
+			my $commitid = $prefixes{$d};
+			$commitid =~ s/^build_//;
+			$commitid =~ s/\.$//;
+			my ($result_value, $u, $s) = get_times("$resultsdir/$prefixes{$d}$t.times");
+
+			my %vals = (
+				"commitid" => $commitid,
+				"project" => $project,
+				"branch" => $dirnames{$d},
+				"executable" => $executable,
+				"benchmark" => $shorttests{$t} . " " . read_descr("$resultsdir/$t.descr"),
+				"environment" => $environment,
+				"result_value" => $result_value,
+			    );
+			push @data, \%vals;
+		}
+	}
+
+	#use Data::Dumper qw/ Dumper /;
+	#print Dumper(\@data);
+
+	use JSON;
+	print to_json(\@data, {utf8 => 1, pretty => 1}), "\n";
+}
+
 binmode STDOUT, ":utf8" or die "PANIC on binmode: $!";
 
-print_default_results();
+if ($codespeed) {
+	print_codespeed_results($results_section);
+} else {
+	print_default_results();
+}
-- 
2.15.1.361.g8b07d831d0
Junio C Hamano· Dec 28, 2017, 19:47 UTC · re: Christian Couder · lore

Re: [PATCH v2 3/7] perf/aggregate: implement codespeed JSON output

Christian Couder <christian.couder@gmail.com> writes:
Show 6 quoted lines
> ...
> +sub print_codespeed_results {
> +	my ($results_section) = @_;
> +
> +	#use Data::Dumper qw/ Dumper /;
> +	#print Dumper(\@data);

Perhaps lose these. It is OK to keep the code live and hide it behind --debug or something, but it seems to me that you are done debugging it pretty much so...

> +	use JSON;

Have this at the top, perhaps? It's not like this would let us avoid loading JSON module when not doing codespeed (you need 'require' for that, right?).

Show 11 quoted lines
> +	print to_json(\@data, {utf8 => 1, pretty => 1}), "\n";
> +}
> +
>  binmode STDOUT, ":utf8" or die "PANIC on binmode: $!";
>  
> -print_default_results();
> +if ($codespeed) {
> +	print_codespeed_results($results_section);
> +} else {
> +	print_default_results();
> +}
Christian Couder· Dec 26, 2017, 21:59 UTC · re: Christian Couder · lore

[PATCH v2 4/7] perf/run: add conf_opts argument to get_var_from_env_or_config()

Let's make it possible to use `git config` type specifiers like `--int` or `--bool`, so that config values are converted to the canonical form and easier to use.

This additional argument is now the fourth argument of get_var_from_env_or_config() instead of the fifth because we want the default value argument to be unset if it is not passed, and this is simpler if it is the last argument.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/perf/run | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
Show changes to t/perf/run +6 −5
diff --git a/t/perf/run b/t/perf/run
index 43e4de49ef..214d658172 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -105,7 +105,8 @@ get_var_from_env_or_config () {
 	env_var="$1"
 	conf_sec="$2"
 	conf_var="$3"
-	# $4 can be set to a default value
+	conf_opts="$4" # optional
+	# $5 can be set to a default value
 
 	# Do nothing if the env variable is already set
 	eval "test -z \"\${$env_var+x}\"" || return
@@ -116,18 +117,18 @@ get_var_from_env_or_config () {
 	if test -n "$GIT_PERF_SUBSECTION"
 	then
 		var="$conf_sec.$GIT_PERF_SUBSECTION.$conf_var"
-		conf_value=$(git config -f "$GIT_PERF_CONFIG_FILE" "$var") &&
+		conf_value=$(git config $conf_opts -f "$GIT_PERF_CONFIG_FILE" "$var") &&
 		eval "$env_var=\"$conf_value\"" && return
 	fi
 	var="$conf_sec.$conf_var"
-	conf_value=$(git config -f "$GIT_PERF_CONFIG_FILE" "$var") &&
+	conf_value=$(git config $conf_opts -f "$GIT_PERF_CONFIG_FILE" "$var") &&
 	eval "$env_var=\"$conf_value\"" && return
 
-	test -n "${4+x}" && eval "$env_var=\"$4\""
+	test -n "${5+x}" && eval "$env_var=\"$5\""
 }
 
 run_subsection () {
-	get_var_from_env_or_config "GIT_PERF_REPEAT_COUNT" "perf" "repeatCount" 3
+	get_var_from_env_or_config "GIT_PERF_REPEAT_COUNT" "perf" "repeatCount" "--int" 3
 	export GIT_PERF_REPEAT_COUNT
 
 	get_var_from_env_or_config "GIT_PERF_DIRS_OR_REVS" "perf" "dirsOrRevs"
-- 
2.15.1.361.g8b07d831d0
Christian Couder· Dec 26, 2017, 21:59 UTC · re: Christian Couder · lore

[PATCH v2 6/7] perf/run: learn to send output to codespeed server

Let's make it possible to set in a config file the URL of a codespeed server. And then let's make the `run` script send the perf test results to this URL at the end of the tests.

This should make is possible to easily automate the process of running perf tests and having their results available in Codespeed.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/perf/run | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
Show changes to t/perf/run +11 −1
diff --git a/t/perf/run b/t/perf/run
index 4e62d6bb3f..ef56396546 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -148,10 +148,20 @@ run_subsection () {
 	test "$GIT_PERF_CODESPEED_OUTPUT" = "true" && codespeed_opt="--codespeed"
 
 	run_dirs "$@"
-	./aggregate.perl $codespeed_opt "$@"
+
+	if test -z "$GIT_PERF_SEND_TO_CODESPEED"
+	then
+		./aggregate.perl $codespeed_opt "$@"
+	else
+		json_res_file="test-results/$GIT_PERF_SUBSECTION/aggregate.json"
+		./aggregate.perl --codespeed "$@" | tee "$json_res_file"
+		send_data_url="$GIT_PERF_SEND_TO_CODESPEED/result/add/json/"
+		curl -v --request POST --data-urlencode "json=$(cat "$json_res_file")" "$send_data_url"
+	fi
 }
 
 get_var_from_env_or_config "GIT_PERF_CODESPEED_OUTPUT" "perf" "codespeedOutput" "--bool"
+get_var_from_env_or_config "GIT_PERF_SEND_TO_CODESPEED" "perf" "sendToCodespeed"
 
 cd "$(dirname $0)"
 . ../../GIT-BUILD-OPTIONS
-- 
2.15.1.361.g8b07d831d0
Christian Couder· Dec 26, 2017, 22:07 UTC · re: Christian Couder · lore

Re: [PATCH v2 0/7] Codespeed perf results

On Tue, Dec 26, 2017 at 10:59 PM, Christian Couder <christian.couder@gmail.com> wrote:

Show 24 quoted lines
> Changes since previous version
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>   - Fixed the way the 'executable' field sent to Codespeed is set in
>     `perf/aggregate.perl` in patch 3/7. We now use `uname -s -m` to
>     which we concatenate the perf.repoName config value if it is set,
>     as suggested by Junio and Eric.
>
>   - Fixed the name of the GIT_PERF_REPO_NAME variable in patches 3/7
>     and 7/7. It was GIT_TEST_REPO_NAME in some places.
>
>   - Fixed how the conf_opt argument is added to the
>     get_var_from_env_or_config() function in patch 4/7. It was added
>     as the last, so fifth, argument, but it is simpler and makes more
>     sense to keep the default value argument as the last argument, so
>     now the conf_opt argument is added as the fourth argument.
>
>   - What patch 4/7 did was previously done in 2 patches in the
>     previous version (patches 4/8 and 5/8), but as we are not doing
>     exactly the same thing (see the above item) it is simpler to do it
>     in only one patch instead of two.
>
>   - We now use the --int type specifier when getting the
>     perf.repeatCount config variable in patch 4/7.
Here is the diff with v1:
Show changes to 2 files +13 −14

t/perf/aggregate.perl, t/perf/run

diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
index 6e15f62a9e..34d74fc015 100755
--- a/t/perf/aggregate.perl
+++ b/t/perf/aggregate.perl
@@ -186,17 +186,16 @@ sub print_codespeed_results {

     my $project = "Git";

-    my $executable;
-    if ($results_section eq "") {
-        $executable = `uname -o -p`;
-    } else {
-        $executable = $results_section;
-        chomp $executable;
+    my $executable = `uname -s -m`;
+    chomp $executable;
+
+    if ($results_section ne "") {
+        $executable .= ", " . $results_section;
     }

     my $environment;
-    if (exists $ENV{GIT_TEST_REPO_NAME} and $ENV{GIT_TEST_REPO_NAME} ne "") {
-        $environment = $ENV{GIT_TEST_REPO_NAME};
+    if (exists $ENV{GIT_PERF_REPO_NAME} and $ENV{GIT_PERF_REPO_NAME} ne "") {
+        $environment = $ENV{GIT_PERF_REPO_NAME};
     } elsif (exists $ENV{GIT_TEST_INSTALLED} and
$ENV{GIT_TEST_INSTALLED} ne "") {
         $environment = $ENV{GIT_TEST_INSTALLED};
         $environment =~ s|/bin-wrappers$||;
diff --git a/t/perf/run b/t/perf/run
index 279c2d41f6..1a100d6134 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -105,8 +105,8 @@ get_var_from_env_or_config () {
     env_var="$1"
     conf_sec="$2"
     conf_var="$3"
-    default_value="$4" # optional
-    conf_opts="$5" # optional
+    conf_opts="$4" # optional
+    # $5 can be set to a default value

     # Do nothing if the env variable is already set
     eval "test -z \"\${$env_var+x}\"" || return
@@ -124,11 +124,11 @@ get_var_from_env_or_config () {
     conf_value=$(git config $conf_opts -f "$GIT_PERF_CONFIG_FILE" "$var") &&
     eval "$env_var=\"$conf_value\"" && return

-    test -n "${default_value+x}" && eval "$env_var=\"$default_value\""
+    test -n "${5+x}" && eval "$env_var=\"$5\""
 }

 run_subsection () {
-    get_var_from_env_or_config "GIT_PERF_REPEAT_COUNT" "perf" "repeatCount" 3
+    get_var_from_env_or_config "GIT_PERF_REPEAT_COUNT" "perf"
"repeatCount" "--int" 3
     export GIT_PERF_REPEAT_COUNT

     get_var_from_env_or_config "GIT_PERF_DIRS_OR_REVS" "perf" "dirsOrRevs"
@@ -137,7 +137,7 @@ run_subsection () {
     get_var_from_env_or_config "GIT_PERF_MAKE_COMMAND" "perf" "makeCommand"
     get_var_from_env_or_config "GIT_PERF_MAKE_OPTS" "perf" "makeOpts"

-    get_var_from_env_or_config "GIT_TEST_REPO_NAME" "perf" "repoName"
+    get_var_from_env_or_config "GIT_PERF_REPO_NAME" "perf" "repoName"
     export GIT_PERF_REPO_NAME

     GIT_PERF_AGGREGATING_LATER=t
@@ -163,7 +163,7 @@ run_subsection () {
     fi
 }

-get_var_from_env_or_config "GIT_PERF_CODESPEED_OUTPUT" "perf"
"codespeedOutput" "" --bool
+get_var_from_env_or_config "GIT_PERF_CODESPEED_OUTPUT" "perf"
"codespeedOutput" "--bool"
 get_var_from_env_or_config "GIT_PERF_SEND_TO_CODESPEED" "perf"
"sendToCodespeed"

 cd "$(dirname $0)"
Junio C Hamano· Dec 28, 2017, 19:39 UTC · re: Christian Couder · lore

Re: [PATCH v2 0/7] Codespeed perf results

Christian Couder <christian.couder@gmail.com> writes:
Show 6 quoted lines
> This patch series is built on top of cc/perf-run-config which recently
> graduated to master.
>
> It makes it possible to send perf results to a Codespeed server. See
> https://github.com/tobami/codespeed/ and web sites like
> http://speed.pypy.org/ which are using Codespeed.
Thanks for an update.  Will queue.

← back to recent threads