threads / patch / 32637

patchtest-lib.sh: unfilter GIT_PERF_*

Subject: [PATCH] test-lib.sh: unfilter GIT_PERF_*

## tl;dr

7 messages between Jan 15, 2013 and Jan 16, 2013. Diffs are folded; open one to read it.

replies: 6people: 4as markdown or json

Nguyễn Thái Ngọc Duy· Jan 15, 2013, 12:53 UTC · lore

These variables are user parameters to control how to run the perf tests. Allow users to do so.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 t/test-lib.sh | 3 +++
 1 file changed, 3 insertions(+)

Show changes to t/test-lib.sh +3 −0

diff --git a/t/test-lib.sh b/t/test-lib.sh
index f50f834..b8d35d1 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -86,6 +86,9 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $("$PERL_PATH" -e '
 		PROVE
 		VALGRIND
 		PERF_AGGREGATING_LATER
+		PERF_LARGE_REPO
+		PERF_REPEAT_COUNT
+		PERF_REPO
 	));
 	my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);
 	print join("\n", @vars);

-- 
1.8.0.rc2.23.g1fb49df

Thomas Rast· Jan 15, 2013, 13:43 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH] test-lib.sh: unfilter GIT_PERF_*

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:

> These variables are user parameters to control how to run the perf
> tests. Allow users to do so.

[...]

Show 8 quoted lines

> @@ -86,6 +86,9 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $("$PERL_PATH" -e '
>  		PROVE
>  		VALGRIND
>  		PERF_AGGREGATING_LATER
> +		PERF_LARGE_REPO
> +		PERF_REPEAT_COUNT
> +		PERF_REPO
>  	));

Wouldn't it be more futureproof to put simply PERF as an entry, and rely on the leading-match logic

> 	my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);

to allow all GIT_PERF variables?

Other than that, Ack. I never noticed because I set mine through config.mak, which goes to GIT-BUILD-OPTIONS. Those options are not exported, which means perl does not pick them up. (That just took me far too long to realize.)

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Duy Nguyen· Jan 15, 2013, 13:47 UTC · re: Thomas Rast · lore

Re: [PATCH] test-lib.sh: unfilter GIT_PERF_*

On Tue, Jan 15, 2013 at 8:43 PM, Thomas Rast <trast@student.ethz.ch> wrote:

Show 20 quoted lines

> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> These variables are user parameters to control how to run the perf
>> tests. Allow users to do so.
> [...]
>> @@ -86,6 +86,9 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $("$PERL_PATH" -e '
>>               PROVE
>>               VALGRIND
>>               PERF_AGGREGATING_LATER
>> +             PERF_LARGE_REPO
>> +             PERF_REPEAT_COUNT
>> +             PERF_REPO
>>       ));
>
> Wouldn't it be more futureproof to put simply PERF as an entry, and rely
> on the leading-match logic
>
>>       my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);
>
> to allow all GIT_PERF variables?

Yeah.

> Other than that, Ack.  I never noticed because I set mine through
> config.mak, which goes to GIT-BUILD-OPTIONS.  Those options are not
> exported, which means perl does not pick them up.  (That just took me
> far too long to realize.)

By the way is there an option to skip the first few runs (too lazy to check out the source code, apparently)? I tried linux-2.6 as the large repo and I think the first (cold cache) run ruins the numbers.

-- 
Duy

Nguyễn Thái Ngọc Duy· Jan 15, 2013, 13:50 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH v2] test-lib.sh: unfilter GIT_PERF_*

These variables are user parameters to control how to run the perf tests. Allow users to do so.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 t/test-lib.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to t/test-lib.sh +1 −1

diff --git a/t/test-lib.sh b/t/test-lib.sh
index f50f834..e1c8c85 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -85,7 +85,7 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $("$PERL_PATH" -e '
 		.*_TEST
 		PROVE
 		VALGRIND
-		PERF_AGGREGATING_LATER
+		PERF
 	));
 	my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);
 	print join("\n", @vars);

-- 
1.8.0.rc2.23.g1fb49df

Junio C Hamano· Jan 15, 2013, 19:31 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH v2] test-lib.sh: unfilter GIT_PERF_*

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

Show 5 quoted lines

> These variables are user parameters to control how to run the perf
> tests. Allow users to do so.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---

I think the Subject makes more sense, so I'd suggest replacing the current one with "PERF_", not with "PERF".

Show 16 quoted lines

>  t/test-lib.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index f50f834..e1c8c85 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -85,7 +85,7 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $("$PERL_PATH" -e '
>  		.*_TEST
>  		PROVE
>  		VALGRIND
> -		PERF_AGGREGATING_LATER
> +		PERF
>  	));
>  	my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);
>  	print join("\n", @vars);

Nguyễn Thái Ngọc Duy· Jan 16, 2013, 10:23 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH v3] test-lib.sh: unfilter GIT_PERF_*

These variables are user parameters to control how to run the perf tests. Allow users to do so.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 PERF -> PERF_

 t/test-lib.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to t/test-lib.sh +1 −1

diff --git a/t/test-lib.sh b/t/test-lib.sh
index f50f834..bf4cf71 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -85,7 +85,7 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $("$PERL_PATH" -e '
 		.*_TEST
 		PROVE
 		VALGRIND
-		PERF_AGGREGATING_LATER
+		PERF_
 	));
 	my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);
 	print join("\n", @vars);

-- 
1.8.0.rc2.23.g1fb49df

Jonathan Nieder· Jan 16, 2013, 18:54 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH v3] test-lib.sh: unfilter GIT_PERF_*

Nguyễn Thái Ngọc Duy wrote:

> These variables are user parameters to control how to run the perf
> tests. Allow users to do so.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

FWIW, of the three versions proposed, this one makes the most sense to me. Looks good.