perf: amend the grep tests to test grep.threads

Ever since 5b594f457a ("Threaded grep", 2010-01-25) the number of threads git-grep uses under PTHREADS has been hardcoded to 8, but there's no performance test to check whether this is an optimal setting.

Amend the existing tests for the grep engines to support a mode where this can be tested, e.g.:

    GIT_PERF_GREP_THREADS='1 8 16' GIT_PERF_LARGE_REPO=~/g/linux ./run p782*

Signed-off-by Ævar Arnfjörð Bjarmason <[email protected]>

---

This looks less scary under diff -w.

 t/perf/p7820-grep-engines.sh       | 52 ++++++++++++++++++++++++++++-------
 t/perf/p7821-grep-engines-fixed.sh | 55 ++++++++++++++++++++++++++++++--------
 2 files changed, 86 insertions(+), 21 deletions(-)

t/perf/p7820-grep-engines.sh +0 -0

index 62aba19e76..8b09c5bf32 100755
@@ -12,6 +12,9 @@ e.g. GIT_PERF_7820_GREP_OPTS=' -i'. Some options to try:
...

t/perf/p7821-grep-engines-fixed.sh +1 -1

index c7ef1e198f..61e41b82cf 100755
@@ -6,6 +6,9 @@ Set GIT_PERF_7821_GREP_OPTS in the environment to pass options to
...
 for pattern in 'int' 'uncommon' 'æ'
 do
 	for engine in fixed basic extended perl

@@ -23,19 +31,44 @@ do
...
+		if ! test_have_prereq PERF_GREP_ENGINES_THREADS
 		then
-			test_cmp out.fixed out.perl

Quoted 25 lines · Ævar Arnfjörð Bjarmason

Ever since 5b594f457a ("Threaded grep", 2010-01-25) the number of threads git-grep uses under PTHREADS has been hardcoded to 8, but there's no performance test to check whether this is an optimal setting.

Amend the existing tests for the grep engines to support a mode where this can be tested, e.g.:

    GIT_PERF_GREP_THREADS='1 8 16' GIT_PERF_LARGE_REPO=~/g/linux ./run p782*

Signed-off-by Ævar Arnfjörð Bjarmason <[email protected]>

---

This looks less scary under diff -w.

 t/perf/p7820-grep-engines.sh       | 52 ++++++++++++++++++++++++++++-------
 t/perf/p7821-grep-engines-fixed.sh | 55 ++++++++++++++++++++++++++++++--------
 2 files changed, 86 insertions(+), 21 deletions(-)

t/perf/p7820-grep-engines.sh +0 -0

index 62aba19e76..8b09c5bf32 100755
@@ -12,6 +12,9 @@ e.g. GIT_PERF_7820_GREP_OPTS=' -i'. Some options to try:
...

t/perf/p7820-grep-engines.sh e.g. GIT_PERF_7820_GREP_OPTS=' -i'. Some options to try: · line 58

Quoted 10 lines

t/perf/p7820-grep-engines.sh +8 -1

+		if ! test_have_prereq PERF_GREP_ENGINES_THREADS
 		then
-			test_cmp out.basic out.perl
+			test_perf $prereq "$engine grep$GIT_PERF_7820_GREP_OPTS '$pattern'" "
+				git -c grep.patternType=$engine grep$GIT_PERF_7820_GREP_OPTS -- '$pattern' >'out.$engine' || :
+			"
+		else
+			for threads in $GIT_PERF_GREP_THREADS
+			do
+				test_perf PTHREADS,$prereq "$engine grep$GIT_PERF_7820_GREP_OPTS '$pattern' with $threads threads" "

Is it guaranteed that $prereq is not empty at this point?

Judging by the way the other side uses "test_perf $prereq ..." without quotes around it, I suspect you do expect it to be empty in some cases. It means you expect test_have_prereq is prepared to skip an empty prerequisite in a prereq list, but I do not recall writing that helper in such a way, so...

	PTHREADS${prereq:+,}$prereq

or something along that line, perhaps?

Quoted 20 lines

t/perf/p7821-grep-engines-fixed.sh +7 -1

index c7ef1e198f..61e41b82cf 100755
@@ -6,6 +6,9 @@ Set GIT_PERF_7821_GREP_OPTS in the environment to pass options to
...
 for pattern in 'int' 'uncommon' 'æ'
 do
 	for engine in fixed basic extended perl

@@ -23,19 +31,44 @@ do
...
+		if ! test_have_prereq PERF_GREP_ENGINES_THREADS
 		then
-			test_cmp out.fixed out.perl
+			test_perf $prereq "$engine grep$GIT_PERF_7821_GREP_OPTS $pattern" "
+				git -c grep.patternType=$engine grep$GIT_PERF_7821_GREP_OPTS $pattern >'out.$engine' || :
+			"
+		else
+			for threads in $GIT_PERF_GREP_THREADS
+			do

t/perf/p7821-grep-engines-fixed.sh do · line 45

Quoted 4 lines

t/perf/p7821-grep-engines-fixed.sh +4 -0

+				test_perf PTHREADS,$prereq "$engine grep$GIT_PERF_7821_GREP_OPTS $pattern with $threads threads" "
+					git -c grep.patternType=$engine -c grep.threads=$threads grep$GIT_PERF_7821_GREP_OPTS $pattern >'out.$engine.$threads' || :
+				"
+			done

Same here, which means these two scripts share somewhat large body of text and makes me wonder if it is worth refactoring it to ease future updates to them.

Quoted 8 lines

Judging by the way the other side uses "test_perf $prereq ..." without quotes around it, I suspect you do expect it to be empty in some cases. It means you expect test_have_prereq is prepared to skip an empty prerequisite in a prereq list, but I do not recall writing that helper in such a way, so...

	PTHREADS${prereq:+,}$prereq

Quoted 1 line

or something along that line, perhaps?

It's not, but a trailing comma at the end of the prereq list works since test_have_prereq() relies on setting the IFS to ",", so as long as this is portable:
...
We could use the ${...} pattern above if you prefer, but since it doesn't appear to be needed...