threads / patch / 59380

patch, 5 partsperf: fixes and cleanups

Subject: [PATCH 0/5] perf: fixes and cleanups

## tl;dr

6 messages between Mar 12, 2023 and Mar 12, 2023. Diffs are folded; open one to read it.

replies: 5people: 1as markdown or json

Felipe Contreras· Mar 12, 2023, 20:08 UTC · lore

Just for the sake of it I tried to `make perf` for the first time and it didn't work. What follows are some fixes and cleanups I found along the way.

One problem should have prevented this target to build since six months ago, I'm surprised nobody has seen it. Maybe more people should try `GIT_PERF_REPEAT_COUNT=1 make perf` just to make sure it builds.

Felipe Contreras (5):
  perf: sparse: remove grep --sparse option
  perf: grep: fix prereqs
  perf: grep: don't check PRCE twice
  perf: grep: avoid unused prereq
  perf: aggregate: check if subtest exists
 t/perf/aggregate.perl                  |  1 +
 t/perf/p2000-sparse-operations.sh      |  2 +-
 t/perf/p4220-log-grep-engines.sh       |  2 +-
 t/perf/p4221-log-grep-engines-fixed.sh |  2 +-
 t/perf/p7820-grep-engines.sh           | 11 +++--------
 t/perf/p7821-grep-engines-fixed.sh     | 15 +++++----------
 t/perf/p7822-grep-perl-character.sh    |  7 +------
 7 files changed, 13 insertions(+), 27 deletions(-)
-- 
2.39.2.13.g1fb56cf030
Felipe Contreras· Mar 12, 2023, 20:08 UTC · re: Felipe Contreras · lore

[PATCH 1/5] perf: sparse: remove grep --sparse option

It was never added.
Cc: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/perf/p2000-sparse-operations.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to t/perf/p2000-sparse-operations.sh +1 −1
diff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh
index 3242cfe91a..e4a132f593 100755
--- a/t/perf/p2000-sparse-operations.sh
+++ b/t/perf/p2000-sparse-operations.sh
@@ -124,6 +124,6 @@ test_perf_on_all git read-tree -mu HEAD
 test_perf_on_all git checkout-index -f --all
 test_perf_on_all git update-index --add --remove $SPARSE_CONE/a
 test_perf_on_all "git rm -f $SPARSE_CONE/a && git checkout HEAD -- $SPARSE_CONE/a"
-test_perf_on_all git grep --cached --sparse bogus -- "f2/f1/f1/*"
+test_perf_on_all git grep --cached bogus -- "f2/f1/f1/*"
 
 test_done

-- 
2.39.2.13.g1fb56cf030
Felipe Contreras· Mar 12, 2023, 20:08 UTC · re: Felipe Contreras · lore

[PATCH 2/5] perf: grep: fix prereqs

Since 5dccd9155f (t/perf: add iteration setup mechanism to perf-lib, 2022-04-04) they are in a --prereq option.

Cc: Neeraj Singh <neerajsi@microsoft.com>
Cc: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/perf/p7821-grep-engines-fixed.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to t/perf/p7821-grep-engines-fixed.sh +2 −2
diff --git a/t/perf/p7821-grep-engines-fixed.sh b/t/perf/p7821-grep-engines-fixed.sh
index 61e41b82cf..0d75240dae 100755
--- a/t/perf/p7821-grep-engines-fixed.sh
+++ b/t/perf/p7821-grep-engines-fixed.sh
@@ -33,13 +33,13 @@ do
 		fi
 		if ! test_have_prereq PERF_GREP_ENGINES_THREADS
 		then
-			test_perf $prereq "$engine grep$GIT_PERF_7821_GREP_OPTS $pattern" "
+			test_perf "$engine grep$GIT_PERF_7821_GREP_OPTS $pattern" --prereq "$prereq" "
 				git -c grep.patternType=$engine grep$GIT_PERF_7821_GREP_OPTS $pattern >'out.$engine' || :
 			"
 		else
 			for threads in $GIT_PERF_GREP_THREADS
 			do
-				test_perf PTHREADS,$prereq "$engine grep$GIT_PERF_7821_GREP_OPTS $pattern with $threads threads" "
+				test_perf "$engine grep$GIT_PERF_7821_GREP_OPTS $pattern with $threads threads" --prereq "$prereq" "
 					git -c grep.patternType=$engine -c grep.threads=$threads grep$GIT_PERF_7821_GREP_OPTS $pattern >'out.$engine.$threads' || :
 				"
 			done

-- 
2.39.2.13.g1fb56cf030
Felipe Contreras· Mar 12, 2023, 20:08 UTC · re: Felipe Contreras · lore

[PATCH 3/5] perf: grep: don't check PRCE twice

Just set the prereq and let the testing framework check it once.
Cc: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/perf/p4220-log-grep-engines.sh       | 2 +-
 t/perf/p4221-log-grep-engines-fixed.sh | 2 +-
 t/perf/p7820-grep-engines.sh           | 2 +-
 t/perf/p7821-grep-engines-fixed.sh     | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)
Show changes to 4 files +4 −4

t/perf/p4220-log-grep-engines.sh, t/perf/p4221-log-grep-engines-fixed.sh, t/perf/p7820-grep-engines.sh, t/perf/p7821-grep-engines-fixed.sh

diff --git a/t/perf/p4220-log-grep-engines.sh b/t/perf/p4220-log-grep-engines.sh
index 03fbfbb85d..2c619a626f 100755
--- a/t/perf/p4220-log-grep-engines.sh
+++ b/t/perf/p4220-log-grep-engines.sh
@@ -30,7 +30,7 @@ do
 			# Poor man's basic -> extended converter.
 			pattern=$(echo $pattern | sed 's/\\//g')
 		fi
-		if test $engine = "perl" && ! test_have_prereq PCRE
+		if test $engine = "perl"
 		then
 			prereq="PCRE"
 		else
diff --git a/t/perf/p4221-log-grep-engines-fixed.sh b/t/perf/p4221-log-grep-engines-fixed.sh
index 0a6d6dfc21..095c5d1765 100755
--- a/t/perf/p4221-log-grep-engines-fixed.sh
+++ b/t/perf/p4221-log-grep-engines-fixed.sh
@@ -20,7 +20,7 @@ for pattern in 'int' 'uncommon' 'æ'
 do
 	for engine in fixed basic extended perl
 	do
-		if test $engine = "perl" && ! test_have_prereq PCRE
+		if test $engine = "perl"
 		then
 			prereq="PCRE"
 		else
diff --git a/t/perf/p7820-grep-engines.sh b/t/perf/p7820-grep-engines.sh
index 9bfb86842a..336424cb00 100755
--- a/t/perf/p7820-grep-engines.sh
+++ b/t/perf/p7820-grep-engines.sh
@@ -41,7 +41,7 @@ do
 			# Poor man's basic -> extended converter.
 			pattern=$(echo "$pattern" | sed 's/\\//g')
 		fi
-		if test $engine = "perl" && ! test_have_prereq PCRE
+		if test $engine = "perl"
 		then
 			prereq="PCRE"
 		else
diff --git a/t/perf/p7821-grep-engines-fixed.sh b/t/perf/p7821-grep-engines-fixed.sh
index 0d75240dae..79b1b9f8b2 100755
--- a/t/perf/p7821-grep-engines-fixed.sh
+++ b/t/perf/p7821-grep-engines-fixed.sh
@@ -25,7 +25,7 @@ for pattern in 'int' 'uncommon' 'æ'
 do
 	for engine in fixed basic extended perl
 	do
-		if test $engine = "perl" && ! test_have_prereq PCRE
+		if test $engine = "perl"
 		then
 			prereq="PCRE"
 		else

-- 
2.39.2.13.g1fb56cf030
Felipe Contreras· Mar 12, 2023, 20:08 UTC · re: Felipe Contreras · lore

[PATCH 5/5] perf: aggregate: check if subtest exists

If no tests were run, for example in p7822-grep-perl-character.sh, that's not a fatal error, just skip to the next one.

Cc: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/perf/aggregate.perl | 1 +
 1 file changed, 1 insertion(+)
Show changes to t/perf/aggregate.perl +1 −0
diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
index 575d2000cc..a79451a0af 100755
--- a/t/perf/aggregate.perl
+++ b/t/perf/aggregate.perl
@@ -156,6 +156,7 @@ sub sane_backticks {
 	$t =~ s{(?:.*/)?(p(\d+)-[^/]+)\.sh$}{$1} or die "bad test name: $t";
 	my $n = $2;
 	my $fname = "$resultsdir/$t.subtests";
+	next unless (-e $fname);
 	open my $fp, "<", $fname or die "cannot open $fname: $!";
 	for (<$fp>) {
 		chomp;

-- 
2.39.2.13.g1fb56cf030
Felipe Contreras· Mar 12, 2023, 20:08 UTC · re: Felipe Contreras · lore

[PATCH 4/5] perf: grep: avoid unused prereq

There is no point in setting PERF_GREP_ENGINES_THREADS if we are going to check it ourselves.

Cc: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/perf/p7820-grep-engines.sh        | 9 ++-------
 t/perf/p7821-grep-engines-fixed.sh  | 9 ++-------
 t/perf/p7822-grep-perl-character.sh | 7 +------
 3 files changed, 5 insertions(+), 20 deletions(-)
Show changes to 3 files +5 −20

t/perf/p7820-grep-engines.sh, t/perf/p7821-grep-engines-fixed.sh, t/perf/p7822-grep-perl-character.sh

diff --git a/t/perf/p7820-grep-engines.sh b/t/perf/p7820-grep-engines.sh
index 336424cb00..f80fdcb5e2 100755
--- a/t/perf/p7820-grep-engines.sh
+++ b/t/perf/p7820-grep-engines.sh
@@ -22,11 +22,6 @@ etc.) we will test the patterns under those numbers of threads.
 test_perf_large_repo
 test_checkout_worktree
 
-if test -n "$GIT_PERF_GREP_THREADS"
-then
-	test_set_prereq PERF_GREP_ENGINES_THREADS
-fi
-
 for pattern in \
 	'how.to' \
 	'^how to' \
@@ -47,7 +42,7 @@ do
 		else
 			prereq=""
 		fi
-		if ! test_have_prereq PERF_GREP_ENGINES_THREADS
+		if test -z "$GIT_PERF_GREP_THREADS"
 		then
 			test_perf "$engine grep$GIT_PERF_7820_GREP_OPTS '$pattern'" \
 				--prereq "$prereq" "
@@ -64,7 +59,7 @@ do
 		fi
 	done
 
-	if ! test_have_prereq PERF_GREP_ENGINES_THREADS
+	if test -z "$GIT_PERF_GREP_THREADS"
 	then
 		test_expect_success "assert that all engines found the same for$GIT_PERF_7820_GREP_OPTS '$pattern'" '
 			test_cmp out.basic out.extended &&
diff --git a/t/perf/p7821-grep-engines-fixed.sh b/t/perf/p7821-grep-engines-fixed.sh
index 79b1b9f8b2..f25b4976ea 100755
--- a/t/perf/p7821-grep-engines-fixed.sh
+++ b/t/perf/p7821-grep-engines-fixed.sh
@@ -16,11 +16,6 @@ etc.) we will test the patterns under those numbers of threads.
 test_perf_large_repo
 test_checkout_worktree
 
-if test -n "$GIT_PERF_GREP_THREADS"
-then
-	test_set_prereq PERF_GREP_ENGINES_THREADS
-fi
-
 for pattern in 'int' 'uncommon' 'æ'
 do
 	for engine in fixed basic extended perl
@@ -31,7 +26,7 @@ do
 		else
 			prereq=""
 		fi
-		if ! test_have_prereq PERF_GREP_ENGINES_THREADS
+		if test -z "$GIT_PERF_GREP_THREADS"
 		then
 			test_perf "$engine grep$GIT_PERF_7821_GREP_OPTS $pattern" --prereq "$prereq" "
 				git -c grep.patternType=$engine grep$GIT_PERF_7821_GREP_OPTS $pattern >'out.$engine' || :
@@ -46,7 +41,7 @@ do
 		fi
 	done
 
-	if ! test_have_prereq PERF_GREP_ENGINES_THREADS
+	if test -z "$GIT_PERF_GREP_THREADS"
 	then
 		test_expect_success "assert that all engines found the same for$GIT_PERF_7821_GREP_OPTS $pattern" '
 			test_cmp out.fixed out.basic &&
diff --git a/t/perf/p7822-grep-perl-character.sh b/t/perf/p7822-grep-perl-character.sh
index 87009c60df..2b73d41e2b 100755
--- a/t/perf/p7822-grep-perl-character.sh
+++ b/t/perf/p7822-grep-perl-character.sh
@@ -11,11 +11,6 @@ etc.) we will test the patterns under those numbers of threads.
 test_perf_large_repo
 test_checkout_worktree
 
-if test -n "$GIT_PERF_GREP_THREADS"
-then
-	test_set_prereq PERF_GREP_ENGINES_THREADS
-fi
-
 for pattern in \
 	'\\bhow' \
 	'\\bÆvar' \
@@ -24,7 +19,7 @@ for pattern in \
 	'\\w{12}\\b'
 do
 	echo '$pattern' >pat
-	if ! test_have_prereq PERF_GREP_ENGINES_THREADS
+	if test -z "$GIT_PERF_GREP_THREADS"
 	then
 		test_perf "grep -P '$pattern'" --prereq PCRE "
 			git -P grep -f pat || :

-- 
2.39.2.13.g1fb56cf030

← back to recent threads