# [PATCH 0/5] p0004: support being called by t/perf/run 7 messages from 2017-05-13 to 2017-05-15. Participants: René Scharfe, Jeff Hostetler. Thread: https://gitlist.dev/t/45954 ## René Scharfe, 2017-05-13 15:55 Subject: [PATCH 0/5] p0004: support being called by t/perf/run Message-ID: <9b574d73-8dfb-f0b3-dc3e-d593726a8f0e@web.de> URL: https://gitlist.dev/e/9b574d73-8dfb-f0b3-dc3e-d593726a8f0e%40web.de ``` 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, 2017-05-13 15:59 Subject: [PATCH 1/5] p0004: simplify calls of test-lazy-init-name-hash Message-ID: <02f4218d-857a-3615-cc3a-d03ecbf35e67@web.de> URL: https://gitlist.dev/e/02f4218d-857a-3615-cc3a-d03ecbf35e67%40web.de In-Reply-To: <9b574d73-8dfb-f0b3-dc3e-d593726a8f0e@web.de> ``` 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 --- t/perf/p0004-lazy-init-name-hash.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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, 2017-05-13 16:00 Subject: [PATCH 2/5] p0004: avoid using pipes Message-ID: URL: https://gitlist.dev/e/f6d4d434-1f55-ef7c-3c7e-55b56fe96361%40web.de In-Reply-To: <9b574d73-8dfb-f0b3-dc3e-d593726a8f0e@web.de> ``` 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 --- t/perf/p0004-lazy-init-name-hash.sh | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) 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 sorted.single && + sort sorted.multi && + test_cmp sorted.single sorted.multi ' test_expect_success 'multithreaded should be faster' ' -- 2.12.2 ``` ## René Scharfe, 2017-05-13 16:02 Subject: [PATCH 3/5] p0004: use test_perf Message-ID: <5d8cff82-c695-ea6a-4d82-448dc10b8b63@web.de> URL: https://gitlist.dev/e/5d8cff82-c695-ea6a-4d82-448dc10b8b63%40web.de In-Reply-To: <9b574d73-8dfb-f0b3-dc3e-d593726a8f0e@web.de> ``` 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 --- 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(+) 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 URL: https://gitlist.dev/e/871f5f29-feb3-5c69-7ddd-f92819a76cce%40web.de In-Reply-To: <9b574d73-8dfb-f0b3-dc3e-d593726a8f0e@web.de> ``` 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 --- t/perf/p0004-lazy-init-name-hash.sh | 4 ---- 1 file changed, 4 deletions(-) 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 URL: https://gitlist.dev/e/ca844223-bb04-8885-2f33-2c3e9c5a5e3c%40web.de In-Reply-To: <9b574d73-8dfb-f0b3-dc3e-d593726a8f0e@web.de> ``` 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 --- t/perf/p0004-lazy-init-name-hash.sh | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) 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 sorted.single && - sort 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 sorted.single && + sort 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, 2017-05-15 13:14 Subject: Re: [PATCH 0/5] p0004: support being called by t/perf/run Message-ID: <543cd794-273f-9d11-b4cd-4d8dc8b6df96@jeffhostetler.com> URL: https://gitlist.dev/e/543cd794-273f-9d11-b4cd-4d8dc8b6df96%40jeffhostetler.com In-Reply-To: <9b574d73-8dfb-f0b3-dc3e-d593726a8f0e@web.de> ``` On 5/13/2017 11:55 AM, René Scharfe wrote: > 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 ```