[PATCH 0/2] t/perf tests against older versions

[PATCH 0/2] t/perf tests against older versions

[Jeff King]

One of the points of the t/perf suite is to be able to detect
performance regressions between versions. But I don't think anybody
really runs it systematically; we mostly just use it to show off our
shiny new improvements. :)

So I decided to run the suite against v2.0.0 and v2.9.0, to catch any
regressions that have crept in the past few years. The good news is that
there aren't any. But I did need a few patches to show that:

  [1/2]: t/perf: fix regression in testing older versions of git
  [2/2]: p4211: explicitly disable renames in no-rename test

The first one fixes the issue I reported in [1], which let me run the
suite against v2.0.0 at all. And the second fixes something that looks
like a regression in the results, but really isn't.

-Peff

[1] http://article.gmane.org/gmane.comp.version-control.git/297875

[PATCH 1/2] t/perf: fix regression in testing older versions of git

[Jeff King]

Commit 7501b59 (perf: make the tests work in worktrees,
2016-05-13) introduced the use of "git rev-parse --git-path"
in the perf-lib setup code. Because the to-be-tested version
of git is at the front of the $PATH when this code runs,
this means we cannot use modern versions of t/perf to test
versions of git older than v2.5.0 (when that option was
introduced).

This is a symptom of a more general problem. The t/perf
suite is essentially independent of git versions, and
ideally we would be able to run the most modern and complete
set of tests across many historical versions (to see how
they compare). But any setup code they run is therefore
required to use the lowest common denominator we expect to
test.

So let's introduce a new variable, $MODERN_GIT, that we can
use both in perf-lib and in the test setup to get a reliable
set of git features (we might change git and break some
tests, of course, but $MODERN_GIT is tied to the same
version of git as the t/perf scripts, so they can be fixed
or adjusted together).

This commit fixes the "--git-path" case, but does not
mass-convert existing setup code to use $MODERN_GIT. Most
setup code is fairly vanilla and will work with effectively
all versions. But now the tool is there to fix any other
issues we find going forward.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/perf/README      | 12 ++++++++++--
 t/perf/perf-lib.sh |  5 ++++-
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/t/perf/README b/t/perf/README
index 8848c14..15986ca 100644
--- a/t/perf/README
+++ b/t/perf/README
@@ -115,8 +115,16 @@ After that you will want to use some of the following:
 
 At least one of the first two is required!
 
-You can use test_expect_success as usual.  For actual performance
-tests, use
+You can use test_expect_success as usual. In both test_expect_success
+and in test_perf, running "git" points to the version that is being
+peft-tested. The $MODERN_GIT variable points to the git wrapper for the
+currently checked-out version (i.e., the one that matches the t/perf
+scripts you are running).  This is useful if your setup uses commands
+that only work with newer versions of git than what you might want to
+test (but obviously your new commands must still create a state that can
+be used by the older version of git you are testing).
+
+For actual performance tests, use
 
 	test_perf 'descriptive string' '
 		command1 &&
diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index 18c363e..4eb2536 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -52,6 +52,9 @@ TEST_NO_MALLOC_CHECK=t
 # need to export them for test_perf subshells
 export TEST_DIRECTORY TRASH_DIRECTORY GIT_BUILD_DIR GIT_TEST_CMP
 
+MODERN_GIT=$GIT_BUILD_DIR/bin-wrappers/git
+export MODERN_GIT
+
 perf_results_dir=$TEST_OUTPUT_DIRECTORY/test-results
 mkdir -p "$perf_results_dir"
 rm -f "$perf_results_dir"/$(basename "$0" .sh).subtests
@@ -81,7 +84,7 @@ test_perf_create_repo_from () {
 	repo="$1"
 	source="$2"
 	source_git="$(git -C "$source" rev-parse --git-dir)"
-	objects_dir="$(git -C "$source" rev-parse --git-path objects)"
+	objects_dir="$("$MODERN_GIT" -C "$source" rev-parse --git-path objects)"
 	mkdir -p "$repo/.git"
 	(
 		cd "$source" &&
-- 
2.9.0.204.g1499a7b

[PATCH 2/2] p4211: explicitly disable renames in no-rename test

[Jeff King]

p4211 tests line-log performance both with and without "-M".
In v2.9.0, the case without "-M" appears to have regressed
badly, but that is only because we flipped on renames by
default.

Let's have the test explicitly disable renames to get
consistent timings (and to match the presumed intent of the
test, which is to see the effects with and without renames).

Signed-off-by: Jeff King <peff@peff.net>
---
 t/perf/p4211-line-log.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/perf/p4211-line-log.sh b/t/perf/p4211-line-log.sh
index 3d074b0..b7ff68d 100755
--- a/t/perf/p4211-line-log.sh
+++ b/t/perf/p4211-line-log.sh
@@ -23,11 +23,11 @@ test_perf 'git log --follow (baseline for -M)' '
 	git log --oneline --follow -- "$file" >/dev/null
 '
 
-test_perf 'git log -L' '
-	git log -L 1:"$file" >/dev/null
+test_perf 'git log -L (renames off)' '
+	git log --no-renames -L 1:"$file" >/dev/null
 '
 
-test_perf 'git log -M -L' '
+test_perf 'git log -L (renames on)' '
 	git log -M -L 1:"$file" >/dev/null
 '
 
-- 
2.9.0.204.g1499a7b

Re: [PATCH 1/2] t/perf: fix regression in testing older versions of git

[Johannes Schindelin]

Hi Peff,

On Wed, 22 Jun 2016, Jeff King wrote:

> Commit 7501b59 (perf: make the tests work in worktrees,
> 2016-05-13) introduced the use of "git rev-parse --git-path"
> in the perf-lib setup code. Because the to-be-tested version
> of git is at the front of the $PATH when this code runs,
> this means we cannot use modern versions of t/perf to test
> versions of git older than v2.5.0 (when that option was
> introduced).
> 
> This is a symptom of a more general problem. The t/perf
> suite is essentially independent of git versions, and
> ideally we would be able to run the most modern and complete
> set of tests across many historical versions (to see how
> they compare). But any setup code they run is therefore
> required to use the lowest common denominator we expect to
> test.
> 
> So let's introduce a new variable, $MODERN_GIT, that we can
> use both in perf-lib and in the test setup to get a reliable
> set of git features (we might change git and break some
> tests, of course, but $MODERN_GIT is tied to the same
> version of git as the t/perf scripts, so they can be fixed
> or adjusted together).
> 
> This commit fixes the "--git-path" case, but does not
> mass-convert existing setup code to use $MODERN_GIT. Most
> setup code is fairly vanilla and will work with effectively
> all versions. But now the tool is there to fix any other
> issues we find going forward.

Thanks for beating me to it!

Ciao,
Dscho

Re: [PATCH 1/2] t/perf: fix regression in testing older versions of git

[Johannes Schindelin]

Hi Peff,

On Wed, 22 Jun 2016, Jeff King wrote:

> diff --git a/t/perf/README b/t/perf/README
> index 8848c14..15986ca 100644
> --- a/t/perf/README
> +++ b/t/perf/README
> @@ -115,8 +115,16 @@ After that you will want to use some of the following:
>  
>  At least one of the first two is required!
>  
> -You can use test_expect_success as usual.  For actual performance
> -tests, use
> +You can use test_expect_success as usual. In both test_expect_success
> +and in test_perf, running "git" points to the version that is being
> +peft-tested. The $MODERN_GIT variable points to the git wrapper for the

s/peft/perf/

Or s/peft/peff/. :-)

The rest looks fine!
Dscho

Re: [PATCH 1/2] t/perf: fix regression in testing older versions of git

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> So let's introduce a new variable, $MODERN_GIT, that we can
> use both in perf-lib and in the test setup to get a reliable
> set of git features (we might change git and break some
> tests, of course, but $MODERN_GIT is tied to the same
> version of git as the t/perf scripts, so they can be fixed
> or adjusted together).

I can see how this works for "git -C ... rev-parse ..." or any other
built-in commands, but I am not sure if this is sufficient when any
non-built-in command is used in the perf framework.  How does it
interact with GIT_EXEC_PATH we set in ../test-lib.sh that is
dot-sourced by ./perf-lib.sh that everybody dot-sources?

Re: [PATCH 1/2] t/perf: fix regression in testing older versions of git

[Jeff King]

On Wed, Jun 22, 2016 at 01:46:25PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > So let's introduce a new variable, $MODERN_GIT, that we can
> > use both in perf-lib and in the test setup to get a reliable
> > set of git features (we might change git and break some
> > tests, of course, but $MODERN_GIT is tied to the same
> > version of git as the t/perf scripts, so they can be fixed
> > or adjusted together).
> 
> I can see how this works for "git -C ... rev-parse ..." or any other
> built-in commands, but I am not sure if this is sufficient when any
> non-built-in command is used in the perf framework.  How does it
> interact with GIT_EXEC_PATH we set in ../test-lib.sh that is
> dot-sourced by ./perf-lib.sh that everybody dot-sources?

I didn't test it but it should work because we are pointing to
bin-wrappers/git, which will override GIT_EXEC_PATH, and stick itself at
the front of the PATH.

-Peff

Re: [PATCH 1/2] t/perf: fix regression in testing older versions of git

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> I can see how this works for "git -C ... rev-parse ..." or any other
>> built-in commands, but I am not sure if this is sufficient when any
>> non-built-in command is used in the perf framework.  How does it
>> interact with GIT_EXEC_PATH we set in ../test-lib.sh that is
>> dot-sourced by ./perf-lib.sh that everybody dot-sources?
>
> I didn't test it but it should work because we are pointing to
> bin-wrappers/git, which will override GIT_EXEC_PATH, and stick itself at
> the front of the PATH.

Ah, yes, bin-wrappers/git is not the real binary we just have built
but overrides GIT_EXEC_PATH to point at the matching version.  I
forgot about that.

Thanks.