On Sat, Apr 11, 2015 at 7:59 PM, Thomas Gummerer <t.gummerer@xxxxxxxxx> wrote:
> On 04/11, Erik Elfström wrote:
>> Signed-off-by: Erik Elfström <erik.elfstrom@xxxxxxxxx>
>> ---
>> 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?
>> + 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@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html