Re: [PATCH v2 2/3] p7300: add performance tests for clean

On Sat, Apr 11, 2015 at 7:59 PM, Thomas Gummerer [off-list ref] wrote:

On 04/11, Erik Elfström wrote:

quoted

Signed-off-by: Erik Elfström <redacted>
---
 t/perf/p7300-clean.sh | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100755 t/perf/p7300-clean.sh

diff --git a/t/perf/p7300-clean.sh b/t/perf/p7300-clean.sh
new file mode 100755
index 0000000..af50d5d
--- /dev/null
+++ b/t/perf/p7300-clean.sh

@@ -0,0 +1,37 @@
+#!/bin/sh
+
+test_description="Test git-clean performance"
+
+. ./perf-lib.sh
+
+test_perf_large_repo
+test_checkout_worktree
+
+test_expect_success 'setup untracked directory with many sub dirs' '
+     rm -rf 500_sub_dirs 50000_sub_dirs clean_test_dir &&
+     mkdir 500_sub_dirs 50000_sub_dirs clean_test_dir &&
+     for i in $(test_seq 1 500)
+     do
+             mkdir 500_sub_dirs/dir$i || return $?
+     done &&
+     for i in $(test_seq 1 100)
+     do
+             cp -r 500_sub_dirs 50000_sub_dirs/dir$i || return $?
+     done
+'
+
+test_perf 'clean many untracked sub dirs, check for nested git' '
+     rm -rf clean_test_dir/50000_sub_dirs_cpy &&
+     cp -r 50000_sub_dirs clean_test_dir/50000_sub_dirs_cpy &&

Maybe this would be a good place to use test_perf_cleanup, which I
introduced a while ago and you can find in the
tg/perf-lib-test-perf-cleanup branch?  It probably won't influence the
performance a lot, but still better separate the code that actually
needs to be tested from the cleanup/preparation code.  Ditto in the
other test.

Yes, that would be a clear improvement. I was looking for something like
this, the copy takes more time than the clean currently.

The cleanup hook is maybe not exactly the right fit here though. I would
need to do one initial copy in the setup test and then a copy in the
cleanup, something like this:

test_expect_success 'setup untracked directory with many sub dirs' '
    ...
    cp -r 50000_sub_dirs clean_test_dir/50000_sub_dirs_cpy
'

test_perf_cleanup 'clean many untracked sub dirs, check for nested git' '
    git clean -q -f -d  clean_test_dir/
' '
    test_dir_is_empty clean_test_dir &&
    rm -rf clean_test_dir/50000_sub_dirs_cpy &&
    cp -r 50000_sub_dirs clean_test_dir/50000_sub_dirs_cpy
'

This works better than my original code but maybe we can do even better
with something like:

test_setup_perf_cleanup 'clean many untracked sub dirs, check for nested git' '
    rm -rf clean_test_dir/50000_sub_dirs_cpy &&
    cp -r 50000_sub_dirs clean_test_dir/50000_sub_dirs_cpy
' '
    git clean -q -f -d  clean_test_dir/
' '
    test_dir_is_empty clean_test_dir
'

Having a setup phase avoids the initial copy in the setup test making
things a little easier to follow. I'm not sure its worth the extra complexity
in perf-lib though (and I'm not sure I would be able to implement it either).

Also, what would the implications be if I were to use your new cleanup
function that is not yet on master? Should I rebase on top of your topic
or make a follow up patch to switch over?

quoted

+     git clean -q -f -d  clean_test_dir/ &&
+     test_dir_is_empty clean_test_dir
+'
+
+test_perf 'clean many untracked sub dirs, ignore nested git' '
+     rm -rf clean_test_dir/50000_sub_dirs_cpy &&
+     cp -r 50000_sub_dirs clean_test_dir/50000_sub_dirs_cpy &&
+     git clean -q -f -f -d  clean_test_dir/ &&
+     test_dir_is_empty clean_test_dir
+'
+
+test_done
--
2.4.0.rc0.37.ga3b75b3

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html