{"thread":{"id":"63213","subject":"[PATCH 0/2] Two perf test fixes","startedAt":"2025-03-28T17:07:52Z","lastAt":"2025-04-14T21:48:32Z","messageCount":13,"participants":["Philippe Blain via GitGitGadget","Patrick Steinhardt","Philippe Blain","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"515246","messageId":"pull.1936.git.git.1743181669.gitgitgadget@gmail.com","threadId":"63213","inReplyTo":null,"subject":"[PATCH 0/2] Two perf test fixes","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-03-28T17:07:47Z","receivedAt":"2025-03-28T17:07:52Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Here a two fixes for failures I noticed while running the perf tests.\n\nPhilippe Blain (2):\n p7821: fix test_perf invocation for prereqs\n p9210: fix 'scalar clone' when running from a detached HEAD\n\n t/perf/p7821-grep-engines-fixed.sh | 4 ++--\n t/perf/p9210-scalar.sh | 3 ++-\n 2 files changed, 4 insertions(+), 3 deletions(-)\n\n\nbase-commit: 683c54c999c301c2cd6f715c411407c413b1d84e\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1936%2Fphil-blain%2Fperf-test-fixes-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1936/phil-blain/perf-test-fixes-v1\nPull-Request: https://github.com/git/git/pull/1936\n-- \ngitgitgadget\n"},{"id":"515247","messageId":"41a093d570a5756f730b069980edafbcedf5c8bc.1743181669.git.gitgitgadget@gmail.com","threadId":"63213","inReplyTo":"pull.1936.git.git.1743181669.gitgitgadget@gmail.com","subject":"[PATCH 1/2] p7821: fix test_perf invocation for prereqs","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-03-28T17:07:48Z","receivedAt":"2025-03-28T17:07:53Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\nSince 5dccd9155f (t/perf: add iteration setup mechanism to perf-lib,\n2022-04-04), perf tests need to declare their prerequisites with\n'--prereq', after the test title. p7821 was forgotten in that commit,\nsuch that running that test on a machine where the PCRE prereq is not\nsatisfied aborts the test with:\n\n error: bug in the test script: test_wrapper_ needs 2 positional parameters\n\nFix this by correcting the two 'test_perf' invocations in that test\nsuite.\n\nSigned-off-by: Philippe Blain \n---\n t/perf/p7821-grep-engines-fixed.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/perf/p7821-grep-engines-fixed.sh b/t/perf/p7821-grep-engines-fixed.sh\nindex 61e41b82cff..1d126c7b039 100755\n--- a/t/perf/p7821-grep-engines-fixed.sh\n+++ b/t/perf/p7821-grep-engines-fixed.sh\n@@ -33,13 +33,13 @@ do\n \t\tfi\n \t\tif ! test_have_prereq PERF_GREP_ENGINES_THREADS\n \t\tthen\n-\t\t\ttest_perf $prereq \"$engine grep$GIT_PERF_7821_GREP_OPTS $pattern\" \"\n+\t\t\ttest_perf \"$engine grep$GIT_PERF_7821_GREP_OPTS $pattern\" --prereq \"$prereq\" \"\n \t\t\t\tgit -c grep.patternType=$engine grep$GIT_PERF_7821_GREP_OPTS $pattern >'out.$engine' || :\n \t\t\t\"\n \t\telse\n \t\t\tfor threads in $GIT_PERF_GREP_THREADS\n \t\t\tdo\n-\t\t\t\ttest_perf PTHREADS,$prereq \"$engine grep$GIT_PERF_7821_GREP_OPTS $pattern with $threads threads\" \"\n+\t\t\t\ttest_perf \"$engine grep$GIT_PERF_7821_GREP_OPTS $pattern with $threads threads\" --prereq \"PTHREADS,$prereq\" \"\n \t\t\t\t\tgit -c grep.patternType=$engine -c grep.threads=$threads grep$GIT_PERF_7821_GREP_OPTS $pattern >'out.$engine.$threads' || :\n \t\t\t\t\"\n \t\t\tdone\n-- \ngitgitgadget\n\n"},{"id":"515248","messageId":"1092c32609f249839453052ca802cb10256cb48f.1743181669.git.gitgitgadget@gmail.com","threadId":"63213","inReplyTo":"pull.1936.git.git.1743181669.gitgitgadget@gmail.com","subject":"[PATCH 2/2] p9210: fix 'scalar clone' when running from a detached HEAD","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-03-28T17:07:49Z","receivedAt":"2025-03-28T17:07:54Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\nIn p9210-scalar-clone.sh, we test using 'scalar clone' to clone\n$GIT_PERF_LARGE_REPO (copied locally as 'to-clone'), which defaults to\nthe git.git checkout we are running the test from.\n\nWhen --branch is not specified (as in this test), 'scalar clone' tries\nto get the default branch of the remote repository by parsing the output\nof 'git ls-remote --symref $URL HEAD', as implemented in\nscalar.c:remote_default_branch. When the git.git checkout we are running\nthe test from is in detached HEAD, this fails and we fall back to using\nthe name of the currently checked out branch in the newly initialized\nrepository, which in this case is the value returned earlier in\ncmd_clone by repo_default_branch_name.\n\nWe then invoke 'git checkout -t origin/$branch', with $branch being the\nname we got from remote_default_branch. This invocation fails if\n'$branch' does not exist as a branch in the current git.git checkout.\n\nFix this by creating a local branch in 'to-clone' in the setup test\n\"enable server-side partial clone\", making sure to use '-B' in case a\nbranch named 'test-branch' already exists.\n\nSigned-off-by: Philippe Blain \n---\n t/perf/p9210-scalar.sh | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/t/perf/p9210-scalar.sh b/t/perf/p9210-scalar.sh\nindex 265f7cd1fe2..56b075e906e 100755\n--- a/t/perf/p9210-scalar.sh\n+++ b/t/perf/p9210-scalar.sh\n@@ -7,7 +7,8 @@ test_perf_large_repo \"$TRASH_DIRECTORY/to-clone\"\n \n test_expect_success 'enable server-side partial clone' '\n \tgit -C to-clone config uploadpack.allowFilter true &&\n-\tgit -C to-clone config uploadpack.allowAnySHA1InWant true\n+\tgit -C to-clone config uploadpack.allowAnySHA1InWant true &&\n+\tgit -C to-clone checkout -B test-branch\n '\n \n test_perf 'scalar clone' '\n-- \ngitgitgadget\n"},{"id":"515327","messageId":"Z-pD1puYT87YKAd4@pks.im","threadId":"63213","inReplyTo":"41a093d570a5756f730b069980edafbcedf5c8bc.1743181669.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] p7821: fix test_perf invocation for prereqs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-31T07:27:18Z","receivedAt":"2025-03-31T07:27:22Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Mar 28, 2025 at 05:07:48PM +0000, Philippe Blain via GitGitGadget wrote:\n> diff --git a/t/perf/p7821-grep-engines-fixed.sh b/t/perf/p7821-grep-engines-fixed.sh\n> index 61e41b82cff..1d126c7b039 100755\n> --- a/t/perf/p7821-grep-engines-fixed.sh\n> +++ b/t/perf/p7821-grep-engines-fixed.sh\n> @@ -33,13 +33,13 @@ do\n> \t\tfi\n> \t\tif ! test_have_prereq PERF_GREP_ENGINES_THREADS\n> \t\tthen\n> -\t\t\ttest_perf $prereq \"$engine grep$GIT_PERF_7821_GREP_OPTS $pattern\" \"\n> +\t\t\ttest_perf \"$engine grep$GIT_PERF_7821_GREP_OPTS $pattern\" --prereq \"$prereq\" \"\n> \t\t\t\tgit -c grep.patternType=$engine grep$GIT_PERF_7821_GREP_OPTS $pattern >'out.$engine' || :\n> \t\t\t\"\n> \t\telse\n> \t\t\tfor threads in $GIT_PERF_GREP_THREADS\n> \t\t\tdo\n> -\t\t\t\ttest_perf PTHREADS,$prereq \"$engine grep$GIT_PERF_7821_GREP_OPTS $pattern with $threads threads\" \"\n> +\t\t\t\ttest_perf \"$engine grep$GIT_PERF_7821_GREP_OPTS $pattern with $threads threads\" --prereq \"PTHREADS,$prereq\" \"\n> \t\t\t\t\tgit -c grep.patternType=$engine -c grep.threads=$threads grep$GIT_PERF_7821_GREP_OPTS $pattern >'out.$engine.$threads' || :\n> \t\t\t\t\"\n> \t\t\tdone\n\n\"$prereq\" can be empty here as it depends on which regexp engine we're\nusing. The second case you adapt already looked weird before because we\npotentially checked for \"PTHREADS,\", but the first case was correct\nbefore but is now potentially checking for the empty prerequisite. Does\nthat actually work as expected?\n\nPatrick\n"},{"id":"515328","messageId":"Z-pD2aeCJ6yp9XBN@pks.im","threadId":"63213","inReplyTo":"1092c32609f249839453052ca802cb10256cb48f.1743181669.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] p9210: fix 'scalar clone' when running from a detached HEAD","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-31T07:27:21Z","receivedAt":"2025-03-31T07:27:24Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Mar 28, 2025 at 05:07:49PM +0000, Philippe Blain via GitGitGadget wrote:\n> From: Philippe Blain \n> \n> In p9210-scalar-clone.sh, we test using 'scalar clone' to clone\n> $GIT_PERF_LARGE_REPO (copied locally as 'to-clone'), which defaults to\n> the git.git checkout we are running the test from.\n> \n> When --branch is not specified (as in this test), 'scalar clone' tries\n> to get the default branch of the remote repository by parsing the output\n> of 'git ls-remote --symref $URL HEAD', as implemented in\n> scalar.c:remote_default_branch. When the git.git checkout we are running\n> the test from is in detached HEAD, this fails and we fall back to using\n> the name of the currently checked out branch in the newly initialized\n> repository, which in this case is the value returned earlier in\n> cmd_clone by repo_default_branch_name.\n> \n> We then invoke 'git checkout -t origin/$branch', with $branch being the\n> name we got from remote_default_branch. This invocation fails if\n> '$branch' does not exist as a branch in the current git.git checkout.\n> \n> Fix this by creating a local branch in 'to-clone' in the setup test\n> \"enable server-side partial clone\", making sure to use '-B' in case a\n> branch named 'test-branch' already exists.\n> \n> Signed-off-by: Philippe Blain \n> ---\n> t/perf/p9210-scalar.sh | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n> \n> diff --git a/t/perf/p9210-scalar.sh b/t/perf/p9210-scalar.sh\n> index 265f7cd1fe2..56b075e906e 100755\n> --- a/t/perf/p9210-scalar.sh\n> +++ b/t/perf/p9210-scalar.sh\n> @@ -7,7 +7,8 @@ test_perf_large_repo \"$TRASH_DIRECTORY/to-clone\"\n> \n> test_expect_success 'enable server-side partial clone' '\n> \tgit -C to-clone config uploadpack.allowFilter true &&\n> -\tgit -C to-clone config uploadpack.allowAnySHA1InWant true\n> +\tgit -C to-clone config uploadpack.allowAnySHA1InWant true &&\n> +\tgit -C to-clone checkout -B test-branch\n> '\n\nThis feels like an easy and pragmatic fix. Thanks!\n\nPatrick\n"},{"id":"516048","messageId":"pull.1936.v2.git.git.1744481732.gitgitgadget@gmail.com","threadId":"63213","inReplyTo":"pull.1936.git.git.1743181669.gitgitgadget@gmail.com","subject":"[PATCH v2 0/3] Two perf test fixes","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-04-12T18:15:29Z","receivedAt":"2025-04-12T18:15:35Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Here a two fixes for failures I noticed while running the perf tests.\n\nPhilippe Blain (3):\n p7821: fix test_perf invocation for prereqs\n p9210: fix 'scalar clone' when running from a detached HEAD\n p7821: fix instructions for testing with threads\n\n t/perf/p7821-grep-engines-fixed.sh | 6 +++---\n t/perf/p9210-scalar.sh | 3 ++-\n 2 files changed, 5 insertions(+), 4 deletions(-)\n\n\nbase-commit: 683c54c999c301c2cd6f715c411407c413b1d84e\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1936%2Fphil-blain%2Fperf-test-fixes-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1936/phil-blain/perf-test-fixes-v2\nPull-Request: https://github.com/git/git/pull/1936\n\nRange-diff vs v1:\n\n 1: 41a093d570a = 1: 41a093d570a p7821: fix test_perf invocation for prereqs\n 2: 1092c32609f = 2: 1092c32609f p9210: fix 'scalar clone' when running from a detached HEAD\n -: ----------- > 3: abd146b7c2a p7821: fix instructions for testing with threads\n\n-- \ngitgitgadget\n"},{"id":"516049","messageId":"41a093d570a5756f730b069980edafbcedf5c8bc.1744481732.git.gitgitgadget@gmail.com","threadId":"63213","inReplyTo":"pull.1936.v2.git.git.1744481732.gitgitgadget@gmail.com","subject":"[PATCH v2 1/3] p7821: fix test_perf invocation for prereqs","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-04-12T18:15:30Z","receivedAt":"2025-04-12T18:15:38Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\nSince 5dccd9155f (t/perf: add iteration setup mechanism to perf-lib,\n2022-04-04), perf tests need to declare their prerequisites with\n'--prereq', after the test title. p7821 was forgotten in that commit,\nsuch that running that test on a machine where the PCRE prereq is not\nsatisfied aborts the test with:\n\n error: bug in the test script: test_wrapper_ needs 2 positional parameters\n\nFix this by correcting the two 'test_perf' invocations in that test\nsuite.\n\nSigned-off-by: Philippe Blain \n---\n t/perf/p7821-grep-engines-fixed.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/perf/p7821-grep-engines-fixed.sh b/t/perf/p7821-grep-engines-fixed.sh\nindex 61e41b82cff..1d126c7b039 100755\n--- a/t/perf/p7821-grep-engines-fixed.sh\n+++ b/t/perf/p7821-grep-engines-fixed.sh\n@@ -33,13 +33,13 @@ do\n \t\tfi\n \t\tif ! test_have_prereq PERF_GREP_ENGINES_THREADS\n \t\tthen\n-\t\t\ttest_perf $prereq \"$engine grep$GIT_PERF_7821_GREP_OPTS $pattern\" \"\n+\t\t\ttest_perf \"$engine grep$GIT_PERF_7821_GREP_OPTS $pattern\" --prereq \"$prereq\" \"\n \t\t\t\tgit -c grep.patternType=$engine grep$GIT_PERF_7821_GREP_OPTS $pattern >'out.$engine' || :\n \t\t\t\"\n \t\telse\n \t\t\tfor threads in $GIT_PERF_GREP_THREADS\n \t\t\tdo\n-\t\t\t\ttest_perf PTHREADS,$prereq \"$engine grep$GIT_PERF_7821_GREP_OPTS $pattern with $threads threads\" \"\n+\t\t\t\ttest_perf \"$engine grep$GIT_PERF_7821_GREP_OPTS $pattern with $threads threads\" --prereq \"PTHREADS,$prereq\" \"\n \t\t\t\t\tgit -c grep.patternType=$engine -c grep.threads=$threads grep$GIT_PERF_7821_GREP_OPTS $pattern >'out.$engine.$threads' || :\n \t\t\t\t\"\n \t\t\tdone\n-- \ngitgitgadget\n\n"},{"id":"516050","messageId":"1092c32609f249839453052ca802cb10256cb48f.1744481732.git.gitgitgadget@gmail.com","threadId":"63213","inReplyTo":"pull.1936.v2.git.git.1744481732.gitgitgadget@gmail.com","subject":"[PATCH v2 2/3] p9210: fix 'scalar clone' when running from a detached HEAD","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-04-12T18:15:31Z","receivedAt":"2025-04-12T18:15:39Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\nIn p9210-scalar-clone.sh, we test using 'scalar clone' to clone\n$GIT_PERF_LARGE_REPO (copied locally as 'to-clone'), which defaults to\nthe git.git checkout we are running the test from.\n\nWhen --branch is not specified (as in this test), 'scalar clone' tries\nto get the default branch of the remote repository by parsing the output\nof 'git ls-remote --symref $URL HEAD', as implemented in\nscalar.c:remote_default_branch. When the git.git checkout we are running\nthe test from is in detached HEAD, this fails and we fall back to using\nthe name of the currently checked out branch in the newly initialized\nrepository, which in this case is the value returned earlier in\ncmd_clone by repo_default_branch_name.\n\nWe then invoke 'git checkout -t origin/$branch', with $branch being the\nname we got from remote_default_branch. This invocation fails if\n'$branch' does not exist as a branch in the current git.git checkout.\n\nFix this by creating a local branch in 'to-clone' in the setup test\n\"enable server-side partial clone\", making sure to use '-B' in case a\nbranch named 'test-branch' already exists.\n\nSigned-off-by: Philippe Blain \n---\n t/perf/p9210-scalar.sh | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/t/perf/p9210-scalar.sh b/t/perf/p9210-scalar.sh\nindex 265f7cd1fe2..56b075e906e 100755\n--- a/t/perf/p9210-scalar.sh\n+++ b/t/perf/p9210-scalar.sh\n@@ -7,7 +7,8 @@ test_perf_large_repo \"$TRASH_DIRECTORY/to-clone\"\n \n test_expect_success 'enable server-side partial clone' '\n \tgit -C to-clone config uploadpack.allowFilter true &&\n-\tgit -C to-clone config uploadpack.allowAnySHA1InWant true\n+\tgit -C to-clone config uploadpack.allowAnySHA1InWant true &&\n+\tgit -C to-clone checkout -B test-branch\n '\n \n test_perf 'scalar clone' '\n-- \ngitgitgadget\n\n"},{"id":"516051","messageId":"abd146b7c2a62aaef5c22269cff155387f33fe32.1744481732.git.gitgitgadget@gmail.com","threadId":"63213","inReplyTo":"pull.1936.v2.git.git.1744481732.gitgitgadget@gmail.com","subject":"[PATCH v2 3/3] p7821: fix instructions for testing with threads","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-04-12T18:15:32Z","receivedAt":"2025-04-12T18:15:40Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\nIn 7b31b55db1 (perf: amend the grep tests to test grep.threads,\n2017-12-29), p7821 was tweaked to test the performance of 'git grep'\nunder different number of threads. These tests are run if\nGIT_PERF_GREP_THREADS is set to a list of thread numbers, but the\ncomment at the top of the file instead mentions GIT_PERF_7821_THREADS.\nFix the comment.\n\nSigned-off-by: Philippe Blain \n---\n t/perf/p7821-grep-engines-fixed.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/perf/p7821-grep-engines-fixed.sh b/t/perf/p7821-grep-engines-fixed.sh\nindex 1d126c7b039..66bec284e3b 100755\n--- a/t/perf/p7821-grep-engines-fixed.sh\n+++ b/t/perf/p7821-grep-engines-fixed.sh\n@@ -7,7 +7,7 @@ git-grep. Make sure to include a leading space,\n e.g. GIT_PERF_7821_GREP_OPTS=' -w'. See p7820-grep-engines.sh for more\n options to try.\n \n-If GIT_PERF_7821_THREADS is set to a list of threads (e.g. '1 4 8'\n+If GIT_PERF_GREP_THREADS is set to a list of threads (e.g. '1 4 8'\n etc.) we will test the patterns under those numbers of threads.\n \"\n \n-- \ngitgitgadget\n"},{"id":"516054","messageId":"54864a66-c399-ac2e-e223-affd6a493989@gmail.com","threadId":"63213","inReplyTo":"pull.1936.v2.git.git.1744481732.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/3] Two perf test fixes","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2025-04-13T02:50:33Z","receivedAt":"2025-04-13T02:50:43Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Sorry, I forgot to mention that this v2 only adds a third commit\nwith a small comment fix.\n\nPhilippe.\n\nLe 2025-04-12 à 14:15, Philippe Blain via GitGitGadget a écrit :\n> Here a two fixes for failures I noticed while running the perf tests.\n> \n> Philippe Blain (3):\n> p7821: fix test_perf invocation for prereqs\n> p9210: fix 'scalar clone' when running from a detached HEAD\n> p7821: fix instructions for testing with threads\n> \n> t/perf/p7821-grep-engines-fixed.sh | 6 +++---\n> t/perf/p9210-scalar.sh | 3 ++-\n> 2 files changed, 5 insertions(+), 4 deletions(-)\n> \n> \n> base-commit: 683c54c999c301c2cd6f715c411407c413b1d84e\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1936%2Fphil-blain%2Fperf-test-fixes-v2\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1936/phil-blain/perf-test-fixes-v2\n> Pull-Request: https://github.com/git/git/pull/1936\n> \n> Range-diff vs v1:\n> \n> 1: 41a093d570a = 1: 41a093d570a p7821: fix test_perf invocation for prereqs\n> 2: 1092c32609f = 2: 1092c32609f p9210: fix 'scalar clone' when running from a detached HEAD\n> -: ----------- > 3: abd146b7c2a p7821: fix instructions for testing with threads\n> \n"},{"id":"516059","messageId":"90b3f122-8b94-3b45-08e0-32af95f9cea5@gmail.com","threadId":"63213","inReplyTo":"Z-pD1puYT87YKAd4@pks.im","subject":"Re: [PATCH 1/2] p7821: fix test_perf invocation for prereqs","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2025-04-13T19:00:41Z","receivedAt":"2025-04-13T19:00:43Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Patrick,\n\nLe 2025-03-31 à 03:27, Patrick Steinhardt a écrit :\n> On Fri, Mar 28, 2025 at 05:07:48PM +0000, Philippe Blain via GitGitGadget wrote:\n>> diff --git a/t/perf/p7821-grep-engines-fixed.sh b/t/perf/p7821-grep-engines-fixed.sh\n>> index 61e41b82cff..1d126c7b039 100755\n>> --- a/t/perf/p7821-grep-engines-fixed.sh\n>> +++ b/t/perf/p7821-grep-engines-fixed.sh\n>> @@ -33,13 +33,13 @@ do\n>> \t\tfi\n>> \t\tif ! test_have_prereq PERF_GREP_ENGINES_THREADS\n>> \t\tthen\n>> -\t\t\ttest_perf $prereq \"$engine grep$GIT_PERF_7821_GREP_OPTS $pattern\" \"\n>> +\t\t\ttest_perf \"$engine grep$GIT_PERF_7821_GREP_OPTS $pattern\" --prereq \"$prereq\" \"\n>> \t\t\t\tgit -c grep.patternType=$engine grep$GIT_PERF_7821_GREP_OPTS $pattern >'out.$engine' || :\n>> \t\t\t\"\n>> \t\telse\n>> \t\t\tfor threads in $GIT_PERF_GREP_THREADS\n>> \t\t\tdo\n>> -\t\t\t\ttest_perf PTHREADS,$prereq \"$engine grep$GIT_PERF_7821_GREP_OPTS $pattern with $threads threads\" \"\n>> +\t\t\t\ttest_perf \"$engine grep$GIT_PERF_7821_GREP_OPTS $pattern with $threads threads\" --prereq \"PTHREADS,$prereq\" \"\n>> \t\t\t\t\tgit -c grep.patternType=$engine -c grep.threads=$threads grep$GIT_PERF_7821_GREP_OPTS $pattern >'out.$engine.$threads' || :\n>> \t\t\t\t\"\n>> \t\t\tdone\n> \n> \"$prereq\" can be empty here as it depends on which regexp engine we're\n> using. The second case you adapt already looked weird before because we\n> potentially checked for \"PTHREADS,\", \n\nIndeed, the reason why the second case did not fail even when built with PCRE (which\nwould not fail the first 'test_perf' since '$prereq' would be empty) is \nthat this second test (in fact the whole loop) is only reached if\nGIT_PERF_GREP_THREADS is set in the environment, which sets the \nPERF_GREP_ENGINES_THREADS prereq. So just running 'make perf' or \n'./p7821-*' would not enter this part of the test.\n\n> but the first case was correct\n> before but is now potentially checking for the empty prerequisite. Does\n> that actually work as expected?\n\nYes, it was correct when built with PCRE, but not without, as then \n$prereq would not be empty. I did check that it works correctly \nbefore sending the patch, both when built with and without PCRE.\n\nThank you for the review and also checking that the patch works correctly.\nI just checked 'test_skip' which is the function that checks the prereq and\nindeed and empty 'test_prereq' is treated as no prereq.\n\nCheers,\n\nPhilippe.\n"},{"id":"516111","messageId":"xmqqlds2rapw.fsf@gitster.g","threadId":"63213","inReplyTo":"54864a66-c399-ac2e-e223-affd6a493989@gmail.com","subject":"Re: [PATCH v2 0/3] Two perf test fixes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-04-14T16:04:11Z","receivedAt":"2025-04-14T16:04:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philippe Blain writes:\n\n> Sorry, I forgot to mention that this v2 only adds a third commit\n> with a small comment fix.\n\nThanks!\n"},{"id":"516150","messageId":"xmqq7c3mpg7m.fsf@gitster.g","threadId":"63213","inReplyTo":"abd146b7c2a62aaef5c22269cff155387f33fe32.1744481732.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 3/3] p7821: fix instructions for testing with threads","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-04-14T21:48:29Z","receivedAt":"2025-04-14T21:48:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philippe Blain via GitGitGadget\" writes:\n\n> From: Philippe Blain \n>\n> In 7b31b55db1 (perf: amend the grep tests to test grep.threads,\n> 2017-12-29), p7821 was tweaked to test the performance of 'git grep'\n> under different number of threads. These tests are run if\n> GIT_PERF_GREP_THREADS is set to a list of thread numbers, but the\n> comment at the top of the file instead mentions GIT_PERF_7821_THREADS.\n> Fix the comment.\n>\n> Signed-off-by: Philippe Blain \n> ---\n> t/perf/p7821-grep-engines-fixed.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n\nThanks.\n"}]}