threads / patch / 46819

v2, 9 partsperf/run: add '--config' option to the 'run' script

Subject: [PATCH v2 1/9] perf/run: add '--config' option to the 'run' script

## tl;dr

14 messages between Sep 23, 2017 and Nov 24, 2017. Diffs are folded; open one to read it.

replies: 13people: 4as markdown or json

Christian Couder· Sep 23, 2017, 19:55 UTC · lore

It is error prone and tiring to use many long environment variables to give parameters to the 'run' script.

Let's make it easy to store some parameters in a config file and to pass them to the run script.

The GIT_PERF_CONFIG_FILE variable will be set to the argument of the '--config' option. This variable is not used yet. It will be used in a following commit.

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 −2
diff --git a/t/perf/run b/t/perf/run
index beb4acc0e428d..1e7c2a59e45dc 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -2,9 +2,14 @@
 
 case "$1" in
 	--help)
-		echo "usage: $0 [other_git_tree...] [--] [test_scripts]"
+		echo "usage: $0 [--config file] [other_git_tree...] [--] [test_scripts]"
 		exit 0
 		;;
+	--config)
+		shift
+		GIT_PERF_CONFIG_FILE=$(cd "$(dirname "$1")"; pwd)/$(basename "$1")
+		export GIT_PERF_CONFIG_FILE
+		shift ;;
 esac
 
 die () {

--
https://github.com/git/git/pull/408
Christian Couder· Sep 23, 2017, 19:55 UTC · re: Christian Couder · lore

[PATCH v2 4/9] perf/run: add calls to get_var_from_env_or_config()

These calls make it possible to have the make command or the make options in a config file, instead of in environment variables.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/perf/run | 3 +++
 1 file changed, 3 insertions(+)
Show changes to t/perf/run +3 −1
diff --git a/t/perf/run b/t/perf/run
index ad442fe64a828..6bd15e701756b 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -116,6 +116,9 @@ export GIT_PERF_REPEAT_COUNT
 get_var_from_env_or_config "GIT_PERF_DIRS_OR_REVS" "perf.dirsOrRevs"
 set -- $GIT_PERF_DIRS_OR_REVS "$@"
 
+get_var_from_env_or_config "GIT_PERF_MAKE_COMMAND" "perf.makeCommand"
+get_var_from_env_or_config "GIT_PERF_MAKE_OPTS" "perf.makeOpts"
+
 GIT_PERF_AGGREGATING_LATER=t
 export GIT_PERF_AGGREGATING_LATER
 

--
https://github.com/git/git/pull/408
Christian Couder· Sep 23, 2017, 19:55 UTC · re: Christian Couder · lore

[PATCH v2 8/9] perf/run: show name of rev being built

It is nice for the user to not just show the sha1 of the current revision being built but also the actual name of this revision.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/perf/run | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
Show changes to t/perf/run +3 −3
diff --git a/t/perf/run b/t/perf/run
index cb8687bfcf98d..43e4de49ef2be 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -37,6 +37,7 @@ unpack_git_rev () {
 
 build_git_rev () {
 	rev=$1
+	name="$2"
 	for config in config.mak config.mak.autogen config.status
 	do
 		if test -e "../../$config"
@@ -44,7 +45,7 @@ build_git_rev () {
 			cp "../../$config" "build/$rev/"
 		fi
 	done
-	echo "=== Building $rev ==="
+	echo "=== Building $rev ($name) ==="
 	(
 		cd build/$rev &&
 		if test -n "$GIT_PERF_MAKE_COMMAND"
@@ -71,7 +72,7 @@ run_dirs_helper () {
 		if [ ! -d build/$rev ]; then
 			unpack_git_rev $rev
 		fi
-		build_git_rev $rev
+		build_git_rev $rev "$mydir"
 		mydir=build/$rev
 	fi
 	if test "$mydir" = .; then

--
https://github.com/git/git/pull/408
Christian Couder· Sep 23, 2017, 19:55 UTC · re: Christian Couder · lore

[PATCH v2 9/9] perf: store subsection results in "test-results/$GIT_PERF_SUBSECTION/"

When tests are run for a subsection defined in a config file, it is better if the results for the current subsection are not overwritting the results of a previous subsection.

So let's store the results for a subsection in a subdirectory of "test-results/" with the subsection name.

The aggregate.perl, when it is run for a subsection, should then aggregate the results found in "test-results/$GIT_PERF_SUBSECTION/".

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/perf/aggregate.perl | 11 ++++++++---
 t/perf/perf-lib.sh    |  1 +
 2 files changed, 9 insertions(+), 3 deletions(-)
Show changes to 2 files +9 −4

t/perf/aggregate.perl, t/perf/perf-lib.sh

diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
index 1dbc85b21407d..e40120848837c 100755
--- a/t/perf/aggregate.perl
+++ b/t/perf/aggregate.perl
@@ -69,12 +69,17 @@ sub format_times {
 	@tests = glob "p????-*.sh";
 }
 
+my $resultsdir = "test-results";
+if ($ENV{GIT_PERF_SUBSECTION} ne "") {
+	$resultsdir .= "/" . $ENV{GIT_PERF_SUBSECTION};
+}
+
 my @subtests;
 my %shorttests;
 for my $t (@tests) {
 	$t =~ s{(?:.*/)?(p(\d+)-[^/]+)\.sh$}{$1} or die "bad test name: $t";
 	my $n = $2;
-	my $fname = "test-results/$t.subtests";
+	my $fname = "$resultsdir/$t.subtests";
 	open my $fp, "<", $fname or die "cannot open $fname: $!";
 	for (<$fp>) {
 		chomp;
@@ -98,7 +103,7 @@ sub read_descr {
 my %descrs;
 my $descrlen = 4; # "Test"
 for my $t (@subtests) {
-	$descrs{$t} = $shorttests{$t}.": ".read_descr("test-results/$t.descr");
+	$descrs{$t} = $shorttests{$t}.": ".read_descr("$resultsdir/$t.descr");
 	$descrlen = length $descrs{$t} if length $descrs{$t}>$descrlen;
 }
 
@@ -138,7 +143,7 @@ sub have_slash {
 	my $firstr;
 	for my $i (0..$#dirs) {
 		my $d = $dirs[$i];
-		$times{$prefixes{$d}.$t} = [get_times("test-results/$prefixes{$d}$t.times")];
+		$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];
diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index 2f88fc12a9b5c..e4c343a6b795b 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -56,6 +56,7 @@ MODERN_GIT=$GIT_BUILD_DIR/bin-wrappers/git
 export MODERN_GIT
 
 perf_results_dir=$TEST_OUTPUT_DIRECTORY/test-results
+test -n "$GIT_PERF_SUBSECTION" && perf_results_dir="$perf_results_dir/$GIT_PERF_SUBSECTION"
 mkdir -p "$perf_results_dir"
 rm -f "$perf_results_dir"/$(basename "$0" .sh).subtests
 

--
https://github.com/git/git/pull/408
Christian Couder· Sep 23, 2017, 19:55 UTC · re: Christian Couder · lore

[PATCH v2 2/9] perf/run: add get_var_from_env_or_config()

Add get_var_from_env_or_config() to easily set variables from a config file if they are defined there and not already set.

This can also set them to a default value if one is provided.

As an example, use this function to set GIT_PERF_REPEAT_COUNT from the perf.repeatCount config option or from the default value.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/perf/perf-lib.sh |  3 ---
 t/perf/run         | 21 +++++++++++++++++++++
 2 files changed, 21 insertions(+), 3 deletions(-)
Show changes to 2 files +21 −4

t/perf/perf-lib.sh, t/perf/run

diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index b50211b2591d1..2f88fc12a9b5c 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -59,9 +59,6 @@ perf_results_dir=$TEST_OUTPUT_DIRECTORY/test-results
 mkdir -p "$perf_results_dir"
 rm -f "$perf_results_dir"/$(basename "$0" .sh).subtests
 
-if test -z "$GIT_PERF_REPEAT_COUNT"; then
-	GIT_PERF_REPEAT_COUNT=3
-fi
 die_if_build_dir_not_repo () {
 	if ! ( cd "$TEST_DIRECTORY/.." &&
 		    git rev-parse --build-dir >/dev/null 2>&1 ); then
diff --git a/t/perf/run b/t/perf/run
index 1e7c2a59e45dc..41580ac6df44a 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -34,6 +34,7 @@ unpack_git_rev () {
 	(cd "$(git rev-parse --show-cdup)" && git archive --format=tar $rev) |
 	(cd build/$rev && tar x)
 }
+
 build_git_rev () {
 	rev=$1
 	for config in config.mak config.mak.autogen config.status
@@ -92,6 +93,26 @@ run_dirs () {
 	done
 }
 
+get_var_from_env_or_config () {
+	env_var="$1"
+	conf_var="$2"
+	# $3 can be set to a default value
+
+	# Do nothing if the env variable is already set
+	eval "test -z \"\${$env_var+x}\"" || return
+
+	# Check if the variable is in the config file
+	test -n "$GIT_PERF_CONFIG_FILE" &&
+	conf_value=$(git config -f "$GIT_PERF_CONFIG_FILE" "$conf_var") &&
+	eval "$env_var=\"$conf_value\"" || {
+		test -n "${3+x}" &&
+		eval "$env_var=\"$3\""
+	}
+}
+
+get_var_from_env_or_config "GIT_PERF_REPEAT_COUNT" "perf.repeatCount" 3
+export GIT_PERF_REPEAT_COUNT
+
 GIT_PERF_AGGREGATING_LATER=t
 export GIT_PERF_AGGREGATING_LATER
 

--
https://github.com/git/git/pull/408
Christian Couder· Sep 23, 2017, 19:55 UTC · re: Christian Couder · lore

[PATCH v2 5/9] perf/run: add get_subsections()

This function makes it possible to find subsections, so that we will be able to run different tests for different subsections in a later commit.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/perf/run | 7 +++++++
 1 file changed, 7 insertions(+)
Show changes to t/perf/run +7 −1
diff --git a/t/perf/run b/t/perf/run
index 6bd15e701756b..4c966c0ae420e 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -93,6 +93,13 @@ run_dirs () {
 	done
 }
 
+get_subsections () {
+	section="$1"
+	test -z "$GIT_PERF_CONFIG_FILE" && return
+	git config -f "$GIT_PERF_CONFIG_FILE" --name-only --get-regex "$section\..*\.[^.]+" |
+	sed -e "s/$section\.\(.*\)\..*/\1/" | sort | uniq
+}
+
 get_var_from_env_or_config () {
 	env_var="$1"
 	conf_var="$2"

--
https://github.com/git/git/pull/408
Christian Couder· Sep 23, 2017, 19:55 UTC · re: Christian Couder · lore

[PATCH v2 6/9] perf/run: update get_var_from_env_or_config() for subsections

As we will set some config options in subsections, let's teach get_var_from_env_or_config() to get the config options from the subsections if they are set there.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/perf/run | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)
Show changes to t/perf/run +20 −13
diff --git a/t/perf/run b/t/perf/run
index 4c966c0ae420e..bd39398b9cc7d 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -102,29 +102,37 @@ get_subsections () {
 
 get_var_from_env_or_config () {
 	env_var="$1"
-	conf_var="$2"
-	# $3 can be set to a default value
+	conf_sec="$2"
+	conf_var="$3"
+	# $4 can be set to a default value
 
 	# Do nothing if the env variable is already set
 	eval "test -z \"\${$env_var+x}\"" || return
 
+	test -z "$GIT_PERF_CONFIG_FILE" && return
+
 	# Check if the variable is in the config file
-	test -n "$GIT_PERF_CONFIG_FILE" &&
-	conf_value=$(git config -f "$GIT_PERF_CONFIG_FILE" "$conf_var") &&
-	eval "$env_var=\"$conf_value\"" || {
-		test -n "${3+x}" &&
-		eval "$env_var=\"$3\""
-	}
+	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") &&
+		eval "$env_var=\"$conf_value\"" && return
+	fi
+	var="$conf_sec.$conf_var"
+	conf_value=$(git config -f "$GIT_PERF_CONFIG_FILE" "$var") &&
+	eval "$env_var=\"$conf_value\"" && return
+
+	test -n "${4+x}" && eval "$env_var=\"$4\""
 }
 
-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" 3
 export GIT_PERF_REPEAT_COUNT
 
-get_var_from_env_or_config "GIT_PERF_DIRS_OR_REVS" "perf.dirsOrRevs"
+get_var_from_env_or_config "GIT_PERF_DIRS_OR_REVS" "perf" "dirsOrRevs"
 set -- $GIT_PERF_DIRS_OR_REVS "$@"
 
-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_MAKE_COMMAND" "perf" "makeCommand"
+get_var_from_env_or_config "GIT_PERF_MAKE_OPTS" "perf" "makeOpts"
 
 GIT_PERF_AGGREGATING_LATER=t
 export GIT_PERF_AGGREGATING_LATER

--
https://github.com/git/git/pull/408
Christian Couder· Sep 23, 2017, 19:55 UTC · re: Christian Couder · lore

[PATCH v2 3/9] perf/run: add GIT_PERF_DIRS_OR_REVS

This environment variable can be set to some revisions or directories whose Git versions should be tested, in addition to the revisions or directories passed as arguments to the 'run' script.

This enables a "perf.dirsOrRevs" configuration variable to be used to set revisions or directories whose Git versions should be tested.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/perf/run | 3 +++
 1 file changed, 3 insertions(+)
Show changes to t/perf/run +3 −1
diff --git a/t/perf/run b/t/perf/run
index 41580ac6df44a..ad442fe64a828 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -113,6 +113,9 @@ get_var_from_env_or_config () {
 get_var_from_env_or_config "GIT_PERF_REPEAT_COUNT" "perf.repeatCount" 3
 export GIT_PERF_REPEAT_COUNT
 
+get_var_from_env_or_config "GIT_PERF_DIRS_OR_REVS" "perf.dirsOrRevs"
+set -- $GIT_PERF_DIRS_OR_REVS "$@"
+
 GIT_PERF_AGGREGATING_LATER=t
 export GIT_PERF_AGGREGATING_LATER
 

--
https://github.com/git/git/pull/408
Christian Couder· Sep 23, 2017, 19:55 UTC · re: Christian Couder · lore

[PATCH v2 7/9] perf/run: add run_subsection()

Let's actually use the subsections we find in the config file to run the perf tests separately for each subsection.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/perf/run | 47 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 35 insertions(+), 12 deletions(-)
Show changes to t/perf/run +35 −13
diff --git a/t/perf/run b/t/perf/run
index bd39398b9cc7d..cb8687bfcf98d 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -125,23 +125,46 @@ get_var_from_env_or_config () {
 	test -n "${4+x}" && eval "$env_var=\"$4\""
 }
 
-get_var_from_env_or_config "GIT_PERF_REPEAT_COUNT" "perf" "repeatCount" 3
-export GIT_PERF_REPEAT_COUNT
+run_subsection () {
+	get_var_from_env_or_config "GIT_PERF_REPEAT_COUNT" "perf" "repeatCount" 3
+	export GIT_PERF_REPEAT_COUNT
 
-get_var_from_env_or_config "GIT_PERF_DIRS_OR_REVS" "perf" "dirsOrRevs"
-set -- $GIT_PERF_DIRS_OR_REVS "$@"
+	get_var_from_env_or_config "GIT_PERF_DIRS_OR_REVS" "perf" "dirsOrRevs"
+	set -- $GIT_PERF_DIRS_OR_REVS "$@"
 
-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_MAKE_COMMAND" "perf" "makeCommand"
+	get_var_from_env_or_config "GIT_PERF_MAKE_OPTS" "perf" "makeOpts"
 
-GIT_PERF_AGGREGATING_LATER=t
-export GIT_PERF_AGGREGATING_LATER
+	GIT_PERF_AGGREGATING_LATER=t
+	export GIT_PERF_AGGREGATING_LATER
+
+	if test $# = 0 -o "$1" = -- -o -f "$1"; then
+		set -- . "$@"
+	fi
+
+	run_dirs "$@"
+	./aggregate.perl "$@"
+}
 
 cd "$(dirname $0)"
 . ../../GIT-BUILD-OPTIONS
 
-if test $# = 0 -o "$1" = -- -o -f "$1"; then
-	set -- . "$@"
+mkdir -p test-results
+get_subsections "perf" >test-results/run_subsections.names
+
+if test $(wc -l <test-results/run_subsections.names) -eq 0
+then
+	(
+		run_subsection "$@"
+	)
+else
+	while read -r subsec
+	do
+		(
+			GIT_PERF_SUBSECTION="$subsec"
+			export GIT_PERF_SUBSECTION
+			echo "======== Run for subsection '$GIT_PERF_SUBSECTION' ========"
+			run_subsection "$@"
+		)
+	done <test-results/run_subsections.names
 fi
-run_dirs "$@"
-./aggregate.perl "$@"

--
https://github.com/git/git/pull/408
Junio C Hamano· Oct 16, 2017, 05:25 UTC · re: Christian Couder · lore

Re: [PATCH v2 1/9] perf/run: add '--config' option to the 'run' script

Christian Couder <chriscool@tuxfamily.org> writes:
> It is error prone and tiring to use many long environment
> variables to give parameters to the 'run' script.

This topic has been sitting in the list archive without getting much reaction from list participants. Is anybody happy with these patches?

Christian Couder· Nov 17, 2017, 07:58 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 1/9] perf/run: add '--config' option to the 'run' script

On Mon, Oct 16, 2017 at 7:25 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 8 quoted lines
> Christian Couder <chriscool@tuxfamily.org> writes:
>
>> It is error prone and tiring to use many long environment
>> variables to give parameters to the 'run' script.
>
> This topic has been sitting in the list archive without getting much
> reaction from list participants.  Is anybody happy with these
> patches?
(Sorry for not responding earlier to this.)
Hopefully AEvar CC'ed will take a look at this soon.
Ævar Arnfjörð Bjarmason· Nov 24, 2017, 10:28 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 1/9] perf/run: add '--config' option to the 'run' script

On Mon, Oct 16 2017, Junio C. Hamano jotted:
Show 8 quoted lines
> Christian Couder <chriscool@tuxfamily.org> writes:
>
>> It is error prone and tiring to use many long environment
>> variables to give parameters to the 'run' script.
>
> This topic has been sitting in the list archive without getting much
> reaction from list participants.  Is anybody happy with these
> patches?

Very late reply, sorry. But I've looked these over and they look good to me.

Kevin Daudt· Oct 18, 2017, 10:58 UTC · re: Christian Couder · lore

Re: [PATCH v2 1/9] perf/run: add '--config' option to the 'run' script

On Sat, Sep 23, 2017 at 07:55:56PM +0000, Christian Couder wrote:
Show 30 quoted lines
> It is error prone and tiring to use many long environment
> variables to give parameters to the 'run' script.
> 
> Let's make it easy to store some parameters in a config
> file and to pass them to the run script.
> 
> The GIT_PERF_CONFIG_FILE variable will be set to the
> argument of the '--config' option. This variable is not
> used yet. It will be used in a following commit.
> 
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  t/perf/run | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/t/perf/run b/t/perf/run
> index beb4acc0e428d..1e7c2a59e45dc 100755
> --- a/t/perf/run
> +++ b/t/perf/run
> @@ -2,9 +2,14 @@
>  
>  case "$1" in
>  	--help)
> -		echo "usage: $0 [other_git_tree...] [--] [test_scripts]"
> +		echo "usage: $0 [--config file] [other_git_tree...] [--] [test_scripts]"
>  		exit 0
>  		;;
> +	--config)
> +		shift
> +		GIT_PERF_CONFIG_FILE=$(cd "$(dirname "$1")"; pwd)/$(basename "$1")

Is the idea of this construct to do some kind of normalization? Otherwise it seems to just result in $1 again.

Show 8 quoted lines
> +		export GIT_PERF_CONFIG_FILE
> +		shift ;;
>  esac
>  
>  die () {
> 
> --
> https://github.com/git/git/pull/408
Christian Couder· Oct 18, 2017, 21:25 UTC · re: Kevin Daudt · lore

Re: [PATCH v2 1/9] perf/run: add '--config' option to the 'run' script

On Wed, Oct 18, 2017 at 12:58 PM, Kevin Daudt <me@ikke.info> wrote:
> On Sat, Sep 23, 2017 at 07:55:56PM +0000, Christian Couder wrote:
Show 18 quoted lines
>> diff --git a/t/perf/run b/t/perf/run
>> index beb4acc0e428d..1e7c2a59e45dc 100755
>> --- a/t/perf/run
>> +++ b/t/perf/run
>> @@ -2,9 +2,14 @@
>>
>>  case "$1" in
>>       --help)
>> -             echo "usage: $0 [other_git_tree...] [--] [test_scripts]"
>> +             echo "usage: $0 [--config file] [other_git_tree...] [--] [test_scripts]"
>>               exit 0
>>               ;;
>> +     --config)
>> +             shift
>> +             GIT_PERF_CONFIG_FILE=$(cd "$(dirname "$1")"; pwd)/$(basename "$1")
>
> Is the idea of this construct to do some kind of normalization?
> Otherwise it seems to just result in $1 again.
Yeah, the idea is to get a full path when you are given a relative path.

← back to recent threads