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

2015-04-07 Thread erik elfström
Will fix!

Also I forgot to ask, does anyone have a good way of moving the copy
out of the performance timing?

After the fix this test spends more time copying than cleaning and
that is not so good. I'm not very good at shell scripting and the only
way I could think of was to make multiple copies at the start and then
in the test check and clean the first non empty directory. That feels
kind of ugly and will fail if a different number of iterations per
test is used. Any suggestions?

I looked a bit at the framework code but I couldn't figure out if it
was easy to add a setup option to be called be before each
iteration.

On Tue, Apr 7, 2015 at 12:09 AM, Eric Sunshine sunsh...@sunshineco.com wrote:
 On Mon, Apr 6, 2015 at 4:40 PM, Torsten Bögershausen tbo...@web.de wrote:
 On 2015-04-06 13.48, Erik Elfström wrote:
 Signed-off-by: Erik Elfström erik.elfst...@gmail.com
 ---
 diff --git a/t/perf/p7300-clean.sh b/t/perf/p7300-clean.sh
 new file mode 100755
 index 000..3f56fb2
 --- /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 5_sub_dirs clean_test_dir 
 + mkdir 500_sub_dirs 5_sub_dirs clean_test_dir 
 + for i in $(seq 1 500)
 I think seq is bash-only, and can be easily replaced by test_seq

 test_seq is definitely desirable. 'seq' is not present on some systems I use.


 + do
 + mkdir 500_sub_dirs/dir$i

 You may want:

 mkdir 500_sub_dirs/dir$i || return $?

 to catch failure of 'mkdir'.

 + done 
 + for i in $(seq 1 100)
 + do
 + cp -r 500_sub_dirs 5_sub_dirs/dir$i

 Ditto:

 cp -r 500_sub_dirs 5_sub_dirs/dir$i || return $?

 + done
 +'
 +
 +test_perf 'clean many untracked sub dirs, check for nested git' '
 + rm -rf clean_test_dir/5_sub_dirs_cpy 
 + cp -r 5_sub_dirs clean_test_dir/5_sub_dirs_cpy 
 + git clean -q -f -d  clean_test_dir/ 
 + test 0 = $(ls -A clean_test_dir/ | wc -l)

 Is the ls -A portable on all systems:
 http://pubs.opengroup.org/onlinepubs/009695399/utilities/ls.html

 My feeling is that the emptiness of a directory can by tested by simply 
 removing it:
 + git clean -q -f -d  clean_test_dir/ 
 + rmdir clean_test_dir
 (And similar below)

 There's also the test_dir_is_empty() function which makes the intent
 even clearer than 'rmdir'.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-04-06 Thread Torsten Bögershausen
On 2015-04-06 13.48, Erik Elfström wrote:
 Signed-off-by: Erik Elfström erik.elfst...@gmail.com
 ---
  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 000..3f56fb2
 --- /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 5_sub_dirs clean_test_dir 
 + mkdir 500_sub_dirs 5_sub_dirs clean_test_dir 
 + for i in $(seq 1 500)
I think seq is bash-only, and can be easily replaced by test_seq 

 + do
 + mkdir 500_sub_dirs/dir$i
 + done 
 + for i in $(seq 1 100)
 + do
 + cp -r 500_sub_dirs 5_sub_dirs/dir$i
 + done
 +'
 +
 +test_perf 'clean many untracked sub dirs, check for nested git' '
 + rm -rf clean_test_dir/5_sub_dirs_cpy 
 + cp -r 5_sub_dirs clean_test_dir/5_sub_dirs_cpy 
 + git clean -q -f -d  clean_test_dir/ 
 + test 0 = $(ls -A clean_test_dir/ | wc -l)

Is the ls -A portable on all systems:
http://pubs.opengroup.org/onlinepubs/009695399/utilities/ls.html

My feeling is that the emptiness of a directory can by tested by simply 
removing it:
 + git clean -q -f -d  clean_test_dir/ 
 + rmdir clean_test_dir
(And similar below)

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


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

2015-04-06 Thread Eric Sunshine
On Mon, Apr 6, 2015 at 4:40 PM, Torsten Bögershausen tbo...@web.de wrote:
 On 2015-04-06 13.48, Erik Elfström wrote:
 Signed-off-by: Erik Elfström erik.elfst...@gmail.com
 ---
 diff --git a/t/perf/p7300-clean.sh b/t/perf/p7300-clean.sh
 new file mode 100755
 index 000..3f56fb2
 --- /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 5_sub_dirs clean_test_dir 
 + mkdir 500_sub_dirs 5_sub_dirs clean_test_dir 
 + for i in $(seq 1 500)
 I think seq is bash-only, and can be easily replaced by test_seq

test_seq is definitely desirable. 'seq' is not present on some systems I use.


 + do
 + mkdir 500_sub_dirs/dir$i

You may want:

mkdir 500_sub_dirs/dir$i || return $?

to catch failure of 'mkdir'.

 + done 
 + for i in $(seq 1 100)
 + do
 + cp -r 500_sub_dirs 5_sub_dirs/dir$i

Ditto:

cp -r 500_sub_dirs 5_sub_dirs/dir$i || return $?

 + done
 +'
 +
 +test_perf 'clean many untracked sub dirs, check for nested git' '
 + rm -rf clean_test_dir/5_sub_dirs_cpy 
 + cp -r 5_sub_dirs clean_test_dir/5_sub_dirs_cpy 
 + git clean -q -f -d  clean_test_dir/ 
 + test 0 = $(ls -A clean_test_dir/ | wc -l)

 Is the ls -A portable on all systems:
 http://pubs.opengroup.org/onlinepubs/009695399/utilities/ls.html

 My feeling is that the emptiness of a directory can by tested by simply 
 removing it:
 + git clean -q -f -d  clean_test_dir/ 
 + rmdir clean_test_dir
 (And similar below)

There's also the test_dir_is_empty() function which makes the intent
even clearer than 'rmdir'.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html