diff mbox series

[v7,24/29] t/perf/p7519: speed up test on Windows

Message ID 803a540cc0022e893a75eae8815b3275a7fac3af.1647972011.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series Builtin FSMonitor Part 2 | expand

Commit Message

Jeff Hostetler March 22, 2022, 6 p.m. UTC
From: Jeff Hostetler <jeffhost@microsoft.com>

Change p7519 to use `test_seq` and `xargs` rather than a `for` loop
to touch thousands of files.  This takes minutes off of test runs
on Windows because of process creation overhead.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/perf/p7519-fsmonitor.sh | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

Comments

Ævar Arnfjörð Bjarmason March 22, 2022, 6:43 p.m. UTC | #1
On Tue, Mar 22 2022, Jeff Hostetler via GitGitGadget wrote:

> From: Jeff Hostetler <jeffhost@microsoft.com>
>
> Change p7519 to use `test_seq` and `xargs` rather than a `for` loop
> to touch thousands of files.  This takes minutes off of test runs
> on Windows because of process creation overhead.
>
> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  t/perf/p7519-fsmonitor.sh | 32 ++++++++++++++++++++------------
>  1 file changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/t/perf/p7519-fsmonitor.sh b/t/perf/p7519-fsmonitor.sh
> index c8be58f3c76..0611e533951 100755
> --- a/t/perf/p7519-fsmonitor.sh
> +++ b/t/perf/p7519-fsmonitor.sh
> @@ -72,7 +72,7 @@ then
>  	fi
>  fi
>  
> -trace_start() {
> +trace_start () {
>  	if test -n "$GIT_PERF_7519_TRACE"
>  	then
>  		name="$1"
> @@ -91,13 +91,20 @@ trace_start() {
>  	fi
>  }
>  
> -trace_stop() {
> +trace_stop () {
>  	if test -n "$GIT_PERF_7519_TRACE"
>  	then
>  		unset GIT_TRACE2_PERF
>  	fi
>  }

(I think I noted in a previous version): Would be nice to have an
optimization change not do unrelated refactoring....

> +touch_files () {
> +	n=$1
> +	d="$n"_files
> +
> +	(cd $d ; test_seq 1 $n | xargs touch )


...and here we don't &&-chain.

> +}
> +
>  test_expect_success "one time repo setup" '
>  	# set untrackedCache depending on the environment
>  	if test -n "$GIT_PERF_7519_UNTRACKED_CACHE"
> @@ -119,10 +126,11 @@ test_expect_success "one time repo setup" '
>  	fi &&
>  
>  	mkdir 1_file 10_files 100_files 1000_files 10000_files &&
> -	for i in $(test_seq 1 10); do touch 10_files/$i || return 1; done &&
> -	for i in $(test_seq 1 100); do touch 100_files/$i || return 1; done &&
> -	for i in $(test_seq 1 1000); do touch 1000_files/$i || return 1; done &&
> -	for i in $(test_seq 1 10000); do touch 10000_files/$i || return 1; done &&
> +	: 1_file directory should be left empty &&
> +	touch_files 10 &&
> +	touch_files 100 &&
> +	touch_files 1000 &&
> +	touch_files 10000 &&
>  	git add 1_file 10_files 100_files 1000_files 10000_files &&
>  	git commit -qm "Add files" &&
>  
> @@ -133,7 +141,7 @@ test_expect_success "one time repo setup" '
>  	fi
>  '
>  
> -setup_for_fsmonitor() {
> +setup_for_fsmonitor () {
>  	# set INTEGRATION_SCRIPT depending on the environment
>  	if test -n "$INTEGRATION_PATH"
>  	then
> @@ -173,7 +181,7 @@ test_perf_w_drop_caches () {
>  	test_perf "$@"
>  }
>  
> -test_fsmonitor_suite() {
> +test_fsmonitor_suite () {

ditto refactoring..
Jeff Hostetler March 23, 2022, 4:33 p.m. UTC | #2
On 3/22/22 2:43 PM, Ævar Arnfjörð Bjarmason wrote:
> 
> On Tue, Mar 22 2022, Jeff Hostetler via GitGitGadget wrote:
> 
>> From: Jeff Hostetler <jeffhost@microsoft.com>
>>
>> Change p7519 to use `test_seq` and `xargs` rather than a `for` loop
>> to touch thousands of files.  This takes minutes off of test runs
>> on Windows because of process creation overhead.
>>
>> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>>   t/perf/p7519-fsmonitor.sh | 32 ++++++++++++++++++++------------
>>   1 file changed, 20 insertions(+), 12 deletions(-)
>>
>> diff --git a/t/perf/p7519-fsmonitor.sh b/t/perf/p7519-fsmonitor.sh
>> index c8be58f3c76..0611e533951 100755
>> --- a/t/perf/p7519-fsmonitor.sh
>> +++ b/t/perf/p7519-fsmonitor.sh
>> @@ -72,7 +72,7 @@ then
[...]
>>   
>> -trace_stop() {
>> +trace_stop () {
>>   	if test -n "$GIT_PERF_7519_TRACE"
>>   	then
>>   		unset GIT_TRACE2_PERF
>>   	fi
>>   }
> 
> (I think I noted in a previous version): Would be nice to have an
> optimization change not do unrelated refactoring....

In V4 24/29, Junio pushed a fixup! commit onto my branch to
add the space between the function name and the parens of the
function that I added.  And fixed the other existing instances
while he was at it.  I squashed that down into my branch during
cleanups between V4 and V5.  It seemed less trouble/noise than
splitting it into two commits.  (I was also at the 30 commit
limit of GGG in V5 and V6, so that was another reason not to
split it.)

Now that I'm at 29 in V7, I suppose that I could split it.



> 
>> +touch_files () {
>> +	n=$1
>> +	d="$n"_files
>> +
>> +	(cd $d ; test_seq 1 $n | xargs touch )
> 
> 
> ...and here we don't &&-chain.
> 

Right, thanks.

Jeff
diff mbox series

Patch

diff --git a/t/perf/p7519-fsmonitor.sh b/t/perf/p7519-fsmonitor.sh
index c8be58f3c76..0611e533951 100755
--- a/t/perf/p7519-fsmonitor.sh
+++ b/t/perf/p7519-fsmonitor.sh
@@ -72,7 +72,7 @@  then
 	fi
 fi
 
-trace_start() {
+trace_start () {
 	if test -n "$GIT_PERF_7519_TRACE"
 	then
 		name="$1"
@@ -91,13 +91,20 @@  trace_start() {
 	fi
 }
 
-trace_stop() {
+trace_stop () {
 	if test -n "$GIT_PERF_7519_TRACE"
 	then
 		unset GIT_TRACE2_PERF
 	fi
 }
 
+touch_files () {
+	n=$1
+	d="$n"_files
+
+	(cd $d ; test_seq 1 $n | xargs touch )
+}
+
 test_expect_success "one time repo setup" '
 	# set untrackedCache depending on the environment
 	if test -n "$GIT_PERF_7519_UNTRACKED_CACHE"
@@ -119,10 +126,11 @@  test_expect_success "one time repo setup" '
 	fi &&
 
 	mkdir 1_file 10_files 100_files 1000_files 10000_files &&
-	for i in $(test_seq 1 10); do touch 10_files/$i || return 1; done &&
-	for i in $(test_seq 1 100); do touch 100_files/$i || return 1; done &&
-	for i in $(test_seq 1 1000); do touch 1000_files/$i || return 1; done &&
-	for i in $(test_seq 1 10000); do touch 10000_files/$i || return 1; done &&
+	: 1_file directory should be left empty &&
+	touch_files 10 &&
+	touch_files 100 &&
+	touch_files 1000 &&
+	touch_files 10000 &&
 	git add 1_file 10_files 100_files 1000_files 10000_files &&
 	git commit -qm "Add files" &&
 
@@ -133,7 +141,7 @@  test_expect_success "one time repo setup" '
 	fi
 '
 
-setup_for_fsmonitor() {
+setup_for_fsmonitor () {
 	# set INTEGRATION_SCRIPT depending on the environment
 	if test -n "$INTEGRATION_PATH"
 	then
@@ -173,7 +181,7 @@  test_perf_w_drop_caches () {
 	test_perf "$@"
 }
 
-test_fsmonitor_suite() {
+test_fsmonitor_suite () {
 	if test -n "$INTEGRATION_SCRIPT"; then
 		DESC="fsmonitor=$(basename $INTEGRATION_SCRIPT)"
 	else
@@ -199,15 +207,15 @@  test_fsmonitor_suite() {
 
 	# Update the mtimes on upto 100k files to make status think
 	# that they are dirty.  For simplicity, omit any files with
-	# LFs (i.e. anything that ls-files thinks it needs to dquote).
-	# Then fully backslash-quote the paths to capture any
-	# whitespace so that they pass thru xargs properly.
+	# LFs (i.e. anything that ls-files thinks it needs to dquote)
+	# and any files with whitespace so that they pass thru xargs
+	# properly.
 	#
 	test_perf_w_drop_caches "status (dirty) ($DESC)" '
 		git ls-files | \
 			head -100000 | \
 			grep -v \" | \
-			sed '\''s/\(.\)/\\\1/g'\'' | \
+			grep -v " ." | \
 			xargs test-tool chmtime -300 &&
 		git status
 	'