{"thread":{"id":"42692","subject":"[PATCH 0/2] t/perf tests against older versions","startedAt":"2016-06-22T19:39:11Z","lastAt":"2016-06-22T20:52:03Z","messageCount":8,"participants":["Jeff King","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"289892","messageId":"20160622193904.GA1509@sigill.intra.peff.net","threadId":"42692","inReplyTo":null,"subject":"[PATCH 0/2] t/perf tests against older versions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-22T19:39:04Z","receivedAt":"2016-06-22T19:39:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"One of the points of the t/perf suite is to be able to detect\nperformance regressions between versions. But I don't think anybody\nreally runs it systematically; we mostly just use it to show off our\nshiny new improvements. :)\n\nSo I decided to run the suite against v2.0.0 and v2.9.0, to catch any\nregressions that have crept in the past few years. The good news is that\nthere aren't any. But I did need a few patches to show that:\n\n  [1/2]: t/perf: fix regression in testing older versions of git\n  [2/2]: p4211: explicitly disable renames in no-rename test\n\nThe first one fixes the issue I reported in [1], which let me run the\nsuite against v2.0.0 at all. And the second fixes something that looks\nlike a regression in the results, but really isn't.\n\n-Peff\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/297875\n\n\n"},{"id":"289893","messageId":"20160622194013.GA1697@sigill.intra.peff.net","threadId":"42692","inReplyTo":"20160622193904.GA1509@sigill.intra.peff.net","subject":"[PATCH 1/2] t/perf: fix regression in testing older versions of git","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-22T19:40:13Z","receivedAt":"2016-06-22T19:40:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Commit 7501b59 (perf: make the tests work in worktrees,\n2016-05-13) introduced the use of \"git rev-parse --git-path\"\nin the perf-lib setup code. Because the to-be-tested version\nof git is at the front of the $PATH when this code runs,\nthis means we cannot use modern versions of t/perf to test\nversions of git older than v2.5.0 (when that option was\nintroduced).\n\nThis is a symptom of a more general problem. The t/perf\nsuite is essentially independent of git versions, and\nideally we would be able to run the most modern and complete\nset of tests across many historical versions (to see how\nthey compare). But any setup code they run is therefore\nrequired to use the lowest common denominator we expect to\ntest.\n\nSo let's introduce a new variable, $MODERN_GIT, that we can\nuse both in perf-lib and in the test setup to get a reliable\nset of git features (we might change git and break some\ntests, of course, but $MODERN_GIT is tied to the same\nversion of git as the t/perf scripts, so they can be fixed\nor adjusted together).\n\nThis commit fixes the \"--git-path\" case, but does not\nmass-convert existing setup code to use $MODERN_GIT. Most\nsetup code is fairly vanilla and will work with effectively\nall versions. But now the tool is there to fix any other\nissues we find going forward.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/perf/README      | 12 ++++++++++--\n t/perf/perf-lib.sh |  5 ++++-\n 2 files changed, 14 insertions(+), 3 deletions(-)\n\ndiff --git a/t/perf/README b/t/perf/README\nindex 8848c14..15986ca 100644\n--- a/t/perf/README\n+++ b/t/perf/README\n@@ -115,8 +115,16 @@ After that you will want to use some of the following:\n \n At least one of the first two is required!\n \n-You can use test_expect_success as usual.  For actual performance\n-tests, use\n+You can use test_expect_success as usual. In both test_expect_success\n+and in test_perf, running \"git\" points to the version that is being\n+peft-tested. The $MODERN_GIT variable points to the git wrapper for the\n+currently checked-out version (i.e., the one that matches the t/perf\n+scripts you are running).  This is useful if your setup uses commands\n+that only work with newer versions of git than what you might want to\n+test (but obviously your new commands must still create a state that can\n+be used by the older version of git you are testing).\n+\n+For actual performance tests, use\n \n \ttest_perf 'descriptive string' '\n \t\tcommand1 &&\ndiff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh\nindex 18c363e..4eb2536 100644\n--- a/t/perf/perf-lib.sh\n+++ b/t/perf/perf-lib.sh\n@@ -52,6 +52,9 @@ TEST_NO_MALLOC_CHECK=t\n # need to export them for test_perf subshells\n export TEST_DIRECTORY TRASH_DIRECTORY GIT_BUILD_DIR GIT_TEST_CMP\n \n+MODERN_GIT=$GIT_BUILD_DIR/bin-wrappers/git\n+export MODERN_GIT\n+\n perf_results_dir=$TEST_OUTPUT_DIRECTORY/test-results\n mkdir -p \"$perf_results_dir\"\n rm -f \"$perf_results_dir\"/$(basename \"$0\" .sh).subtests\n@@ -81,7 +84,7 @@ test_perf_create_repo_from () {\n \trepo=\"$1\"\n \tsource=\"$2\"\n \tsource_git=\"$(git -C \"$source\" rev-parse --git-dir)\"\n-\tobjects_dir=\"$(git -C \"$source\" rev-parse --git-path objects)\"\n+\tobjects_dir=\"$(\"$MODERN_GIT\" -C \"$source\" rev-parse --git-path objects)\"\n \tmkdir -p \"$repo/.git\"\n \t(\n \t\tcd \"$source\" &&\n-- \n2.9.0.204.g1499a7b\n\n"},{"id":"289894","messageId":"20160622194051.GB1697@sigill.intra.peff.net","threadId":"42692","inReplyTo":"20160622193904.GA1509@sigill.intra.peff.net","subject":"[PATCH 2/2] p4211: explicitly disable renames in no-rename test","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-22T19:40:51Z","receivedAt":"2016-06-22T19:41:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"p4211 tests line-log performance both with and without \"-M\".\nIn v2.9.0, the case without \"-M\" appears to have regressed\nbadly, but that is only because we flipped on renames by\ndefault.\n\nLet's have the test explicitly disable renames to get\nconsistent timings (and to match the presumed intent of the\ntest, which is to see the effects with and without renames).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/perf/p4211-line-log.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/perf/p4211-line-log.sh b/t/perf/p4211-line-log.sh\nindex 3d074b0..b7ff68d 100755\n--- a/t/perf/p4211-line-log.sh\n+++ b/t/perf/p4211-line-log.sh\n@@ -23,11 +23,11 @@ test_perf 'git log --follow (baseline for -M)' '\n \tgit log --oneline --follow -- \"$file\" >/dev/null\n '\n \n-test_perf 'git log -L' '\n-\tgit log -L 1:\"$file\" >/dev/null\n+test_perf 'git log -L (renames off)' '\n+\tgit log --no-renames -L 1:\"$file\" >/dev/null\n '\n \n-test_perf 'git log -M -L' '\n+test_perf 'git log -L (renames on)' '\n \tgit log -M -L 1:\"$file\" >/dev/null\n '\n \n-- \n2.9.0.204.g1499a7b\n"},{"id":"289912","messageId":"alpine.DEB.2.20.1606222223100.10382@virtualbox","threadId":"42692","inReplyTo":"20160622194013.GA1697@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] t/perf: fix regression in testing older versions of git","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-06-22T20:23:24Z","receivedAt":"2016-06-22T20:23:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Wed, 22 Jun 2016, Jeff King wrote:\n\n> Commit 7501b59 (perf: make the tests work in worktrees,\n> 2016-05-13) introduced the use of \"git rev-parse --git-path\"\n> in the perf-lib setup code. Because the to-be-tested version\n> of git is at the front of the $PATH when this code runs,\n> this means we cannot use modern versions of t/perf to test\n> versions of git older than v2.5.0 (when that option was\n> introduced).\n> \n> This is a symptom of a more general problem. The t/perf\n> suite is essentially independent of git versions, and\n> ideally we would be able to run the most modern and complete\n> set of tests across many historical versions (to see how\n> they compare). But any setup code they run is therefore\n> required to use the lowest common denominator we expect to\n> test.\n> \n> So let's introduce a new variable, $MODERN_GIT, that we can\n> use both in perf-lib and in the test setup to get a reliable\n> set of git features (we might change git and break some\n> tests, of course, but $MODERN_GIT is tied to the same\n> version of git as the t/perf scripts, so they can be fixed\n> or adjusted together).\n> \n> This commit fixes the \"--git-path\" case, but does not\n> mass-convert existing setup code to use $MODERN_GIT. Most\n> setup code is fairly vanilla and will work with effectively\n> all versions. But now the tool is there to fix any other\n> issues we find going forward.\n\nThanks for beating me to it!\n\nCiao,\nDscho\n"},{"id":"289913","messageId":"alpine.DEB.2.20.1606222223530.10382@virtualbox","threadId":"42692","inReplyTo":"20160622194013.GA1697@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] t/perf: fix regression in testing older versions of git","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-06-22T20:25:53Z","receivedAt":"2016-06-22T20:27:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Wed, 22 Jun 2016, Jeff King wrote:\n\n> diff --git a/t/perf/README b/t/perf/README\n> index 8848c14..15986ca 100644\n> --- a/t/perf/README\n> +++ b/t/perf/README\n> @@ -115,8 +115,16 @@ After that you will want to use some of the following:\n>  \n>  At least one of the first two is required!\n>  \n> -You can use test_expect_success as usual.  For actual performance\n> -tests, use\n> +You can use test_expect_success as usual. In both test_expect_success\n> +and in test_perf, running \"git\" points to the version that is being\n> +peft-tested. The $MODERN_GIT variable points to the git wrapper for the\n\ns/peft/perf/\n\nOr s/peft/peff/. :-)\n\nThe rest looks fine!\nDscho\n"},{"id":"289915","messageId":"xmqqmvmdkl3y.fsf@gitster.mtv.corp.google.com","threadId":"42692","inReplyTo":"20160622194013.GA1697@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] t/perf: fix regression in testing older versions of git","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-22T20:46:25Z","receivedAt":"2016-06-22T20:46:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> So let's introduce a new variable, $MODERN_GIT, that we can\n> use both in perf-lib and in the test setup to get a reliable\n> set of git features (we might change git and break some\n> tests, of course, but $MODERN_GIT is tied to the same\n> version of git as the t/perf scripts, so they can be fixed\n> or adjusted together).\n\nI can see how this works for \"git -C ... rev-parse ...\" or any other\nbuilt-in commands, but I am not sure if this is sufficient when any\nnon-built-in command is used in the perf framework.  How does it\ninteract with GIT_EXEC_PATH we set in ../test-lib.sh that is\ndot-sourced by ./perf-lib.sh that everybody dot-sources?\n\n"},{"id":"289916","messageId":"20160622204836.GA4628@sigill.intra.peff.net","threadId":"42692","inReplyTo":"xmqqmvmdkl3y.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/2] t/perf: fix regression in testing older versions of git","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-22T20:48:36Z","receivedAt":"2016-06-22T20:48:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 22, 2016 at 01:46:25PM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > So let's introduce a new variable, $MODERN_GIT, that we can\n> > use both in perf-lib and in the test setup to get a reliable\n> > set of git features (we might change git and break some\n> > tests, of course, but $MODERN_GIT is tied to the same\n> > version of git as the t/perf scripts, so they can be fixed\n> > or adjusted together).\n> \n> I can see how this works for \"git -C ... rev-parse ...\" or any other\n> built-in commands, but I am not sure if this is sufficient when any\n> non-built-in command is used in the perf framework.  How does it\n> interact with GIT_EXEC_PATH we set in ../test-lib.sh that is\n> dot-sourced by ./perf-lib.sh that everybody dot-sources?\n\nI didn't test it but it should work because we are pointing to\nbin-wrappers/git, which will override GIT_EXEC_PATH, and stick itself at\nthe front of the PATH.\n\n-Peff\n"},{"id":"289917","messageId":"xmqqinx1kkus.fsf@gitster.mtv.corp.google.com","threadId":"42692","inReplyTo":"20160622204836.GA4628@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] t/perf: fix regression in testing older versions of git","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-22T20:51:55Z","receivedAt":"2016-06-22T20:52:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>> I can see how this works for \"git -C ... rev-parse ...\" or any other\n>> built-in commands, but I am not sure if this is sufficient when any\n>> non-built-in command is used in the perf framework.  How does it\n>> interact with GIT_EXEC_PATH we set in ../test-lib.sh that is\n>> dot-sourced by ./perf-lib.sh that everybody dot-sources?\n>\n> I didn't test it but it should work because we are pointing to\n> bin-wrappers/git, which will override GIT_EXEC_PATH, and stick itself at\n> the front of the PATH.\n\nAh, yes, bin-wrappers/git is not the real binary we just have built\nbut overrides GIT_EXEC_PATH to point at the matching version.  I\nforgot about that.\n\nThanks.\n"}]}