threads / patch / 45954

patch, 5 partsp0004: support being called by t/perf/run

Subject: [PATCH 0/5] p0004: support being called by t/perf/run

## tl;dr

7 messages between May 13, 2017 and May 15, 2017. Diffs are folded; open one to read it.

replies: 6people: 2as markdown or json

René Scharfe· May 13, 2017, 15:55 UTC · lore

p0004-lazy-init-name-hash.sh errors out if the test repo is too small, and doesn't generate any perf test results even if it finishes successfully. That prevents t/perf/run from running the whole test suite. This series tries to address these issues.

  p0004: simplify calls of test-lazy-init-name-hash
  p0004: avoid using pipes
  p0004: use test_perf
  p0004: don't abort if multi-threaded is too slow
  p0004: don't error out if test repo is too small
 t/perf/p0004-lazy-init-name-hash.sh | 47 +++++++++++++++++++++++++++++++++----
 1 file changed, 42 insertions(+), 5 deletions(-)
-- 
2.12.2
René Scharfe· May 13, 2017, 15:59 UTC · re: René Scharfe · lore

[PATCH 1/5] p0004: simplify calls of test-lazy-init-name-hash

The test library puts helpers into $PATH, so we can simply call them without specifying their location.

The suffix $X is also not necessary because .exe files on Windows can be started without specifying their extension, and on other platforms it's empty anyway.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 t/perf/p0004-lazy-init-name-hash.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Show changes to t/perf/p0004-lazy-init-name-hash.sh +3 −3
diff --git a/t/perf/p0004-lazy-init-name-hash.sh b/t/perf/p0004-lazy-init-name-hash.sh
index 5afa8c8df3..716c951553 100755
--- a/t/perf/p0004-lazy-init-name-hash.sh
+++ b/t/perf/p0004-lazy-init-name-hash.sh
@@ -7,13 +7,13 @@ test_perf_large_repo
 test_checkout_worktree
 
 test_expect_success 'verify both methods build the same hashmaps' '
-	$GIT_BUILD_DIR/t/helper/test-lazy-init-name-hash$X --dump --single | sort >out.single &&
-	$GIT_BUILD_DIR/t/helper/test-lazy-init-name-hash$X --dump --multi  | sort >out.multi  &&
+	test-lazy-init-name-hash --dump --single | sort >out.single &&
+	test-lazy-init-name-hash --dump --multi  | sort >out.multi  &&
 	test_cmp out.single out.multi
 '
 
 test_expect_success 'multithreaded should be faster' '
-	$GIT_BUILD_DIR/t/helper/test-lazy-init-name-hash$X --perf >out.perf
+	test-lazy-init-name-hash --perf >out.perf
 '
 
 test_done

-- 
2.12.2
René Scharfe· May 13, 2017, 16:00 UTC · re: René Scharfe · lore

[PATCH 2/5] p0004: avoid using pipes

The return code of commands on the producing end of a pipe is ignored. Evaluate the outcome of test-lazy-init-name-hash by calling sort separately.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 t/perf/p0004-lazy-init-name-hash.sh | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
Show changes to t/perf/p0004-lazy-init-name-hash.sh +5 −3
diff --git a/t/perf/p0004-lazy-init-name-hash.sh b/t/perf/p0004-lazy-init-name-hash.sh
index 716c951553..576bdc3c4e 100755
--- a/t/perf/p0004-lazy-init-name-hash.sh
+++ b/t/perf/p0004-lazy-init-name-hash.sh
@@ -7,9 +7,11 @@ test_perf_large_repo
 test_checkout_worktree
 
 test_expect_success 'verify both methods build the same hashmaps' '
-	test-lazy-init-name-hash --dump --single | sort >out.single &&
-	test-lazy-init-name-hash --dump --multi  | sort >out.multi  &&
-	test_cmp out.single out.multi
+	test-lazy-init-name-hash --dump --single >out.single &&
+	test-lazy-init-name-hash --dump --multi >out.multi &&
+	sort <out.single >sorted.single &&
+	sort <out.multi >sorted.multi &&
+	test_cmp sorted.single sorted.multi
 '
 
 test_expect_success 'multithreaded should be faster' '

-- 
2.12.2
René Scharfe· May 13, 2017, 16:02 UTC · re: René Scharfe · lore

[PATCH 3/5] p0004: use test_perf

The perf test suite (more specifically: t/perf/aggregate.perl) requires each test script to write test results into a file, otherwise it aborts when aggregating. Add actual performance tests with test_perf to allow p0004 to be run together with other perf scripts.

Calibrate the value for the parameter --count based on the size of the test repository, in order to get meaningful results with smaller repos yet still be able to finish the script against huge ones without having to wait for hours.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
The numbers are just guesses; I didn't actually test all ranges.
 t/perf/p0004-lazy-init-name-hash.sh | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)
Show changes to t/perf/p0004-lazy-init-name-hash.sh +36 −0
diff --git a/t/perf/p0004-lazy-init-name-hash.sh b/t/perf/p0004-lazy-init-name-hash.sh
index 576bdc3c4e..d30c32f97b 100755
--- a/t/perf/p0004-lazy-init-name-hash.sh
+++ b/t/perf/p0004-lazy-init-name-hash.sh
@@ -18,4 +18,40 @@ test_expect_success 'multithreaded should be faster' '
 	test-lazy-init-name-hash --perf >out.perf
 '
 
+test_expect_success 'calibrate' '
+	entries=$(wc -l <out.single) &&
+
+	case $entries in
+	?) count=1000000 ;;
+	??) count=100000 ;;
+	???) count=10000 ;;
+	????) count=1000 ;;
+	?????) count=100 ;;
+	??????) count=10 ;;
+	*) count=1 ;;
+	esac &&
+	export count &&
+
+	case $entries in
+	1) entries_desc="1 entry" ;;
+	*) entries_desc="$entries entries" ;;
+	esac &&
+
+	case $count in
+	1) count_desc="1 round" ;;
+	*) count_desc="$count rounds" ;;
+	esac &&
+
+	desc="$entries_desc, $count_desc" &&
+	export desc
+'
+
+test_perf "single-threaded, $desc" "
+	test-lazy-init-name-hash --single --count=$count
+"
+
+test_perf "multi-threaded, $desc" "
+	test-lazy-init-name-hash --multi --count=$count
+"
+
 test_done

-- 
2.12.2
René Scharfe· May 13, 2017, 16:03 UTC · re: René Scharfe · lore

[PATCH 4/5] p0004: don't abort if multi-threaded is too slow

If the single-threaded variant beats the multi-threaded one then we may have a performance bug, but that doesn't justify aborting the test. Drop that check; we can compare the results for --single and --multi using the actual performance tests.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 t/perf/p0004-lazy-init-name-hash.sh | 4 ----
 1 file changed, 4 deletions(-)
Show changes to t/perf/p0004-lazy-init-name-hash.sh +0 −4
diff --git a/t/perf/p0004-lazy-init-name-hash.sh b/t/perf/p0004-lazy-init-name-hash.sh
index d30c32f97b..3c2135a185 100755
--- a/t/perf/p0004-lazy-init-name-hash.sh
+++ b/t/perf/p0004-lazy-init-name-hash.sh
@@ -14,10 +14,6 @@ test_expect_success 'verify both methods build the same hashmaps' '
 	test_cmp sorted.single sorted.multi
 '
 
-test_expect_success 'multithreaded should be faster' '
-	test-lazy-init-name-hash --perf >out.perf
-'
-
 test_expect_success 'calibrate' '
 	entries=$(wc -l <out.single) &&
 

-- 
2.12.2
René Scharfe· May 13, 2017, 16:03 UTC · re: René Scharfe · lore

[PATCH 5/5] p0004: don't error out if test repo is too small

Repositories with less than 4000 entries are always handled using a single thread, causing test-lazy-init-name-hash --multi to error out. Don't abort the whole test script in that case, but simply skip the multi-threaded performance check. We can still use it to compare the single-threaded speed of different versions in that case.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 t/perf/p0004-lazy-init-name-hash.sh | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)
Show changes to t/perf/p0004-lazy-init-name-hash.sh +8 −5
diff --git a/t/perf/p0004-lazy-init-name-hash.sh b/t/perf/p0004-lazy-init-name-hash.sh
index 3c2135a185..8de5a98cfc 100755
--- a/t/perf/p0004-lazy-init-name-hash.sh
+++ b/t/perf/p0004-lazy-init-name-hash.sh
@@ -8,10 +8,13 @@ test_checkout_worktree
 
 test_expect_success 'verify both methods build the same hashmaps' '
 	test-lazy-init-name-hash --dump --single >out.single &&
-	test-lazy-init-name-hash --dump --multi >out.multi &&
-	sort <out.single >sorted.single &&
-	sort <out.multi >sorted.multi &&
-	test_cmp sorted.single sorted.multi
+	if test-lazy-init-name-hash --dump --multi >out.multi
+	then
+		test_set_prereq REPO_BIG_ENOUGH_FOR_MULTI &&
+		sort <out.single >sorted.single &&
+		sort <out.multi >sorted.multi &&
+		test_cmp sorted.single sorted.multi
+	fi
 '
 
 test_expect_success 'calibrate' '
@@ -46,7 +49,7 @@ test_perf "single-threaded, $desc" "
 	test-lazy-init-name-hash --single --count=$count
 "
 
-test_perf "multi-threaded, $desc" "
+test_perf REPO_BIG_ENOUGH_FOR_MULTI "multi-threaded, $desc" "
 	test-lazy-init-name-hash --multi --count=$count
 "
 

-- 
2.12.2
Jeff Hostetler· May 15, 2017, 13:14 UTC · re: René Scharfe · lore

Re: [PATCH 0/5] p0004: support being called by t/perf/run

On 5/13/2017 11:55 AM, René Scharfe wrote:
Show 14 quoted lines
> p0004-lazy-init-name-hash.sh errors out if the test repo is too small,
> and doesn't generate any perf test results even if it finishes
> successfully.  That prevents t/perf/run from running the whole test
> suite.  This series tries to address these issues.
>
>   p0004: simplify calls of test-lazy-init-name-hash
>   p0004: avoid using pipes
>   p0004: use test_perf
>   p0004: don't abort if multi-threaded is too slow
>   p0004: don't error out if test repo is too small
>
>  t/perf/p0004-lazy-init-name-hash.sh | 47 +++++++++++++++++++++++++++++++++----
>  1 file changed, 42 insertions(+), 5 deletions(-)
>

Nicely done! Thanks for cleaning this up. Jeff

← back to recent threads