Subject: Re: [PATCH v6 24/30] t/perf/p7519: speed up test on Windows

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Mon, 07 Mar 2022 12:09:36 +0100

On Tue, Mar 01 2022, Jeff Hostetler via GitGitGadget wrote:

> From: Jeff Hostetler <[email-masked]>
>
> 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 <[email-masked]>
> Signed-off-by: Junio C Hamano <[email-masked]>
> ---
>  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..aed7b1146b0 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
>  }

These minor unrelated style fixups could be split up / sent seperately?
Especially as the seem not to conflict hunk-wise with the actual changes
here.

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

Missing &&-chaining for "cd"

> -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 unrelated style changes.

>  	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'\'' | \
> +			egrep -v " ." | \

Per dcf9a748cab (t7700: replace egrep with grep, 2019-12-04) should we
be adding more egrep?

Also, even if we did want that, why does a the " ." regex need ERE over
BRE here?