Subject: Re: [PATCH 6/8] t/perf: add Scalar performance tests

From: Victoria Dye <[email-masked]>
Date: Thu, 1 Sep 2022 09:21:00 -0700

Victoria Dye wrote:
> Johannes Schindelin wrote:
>> Hi Victoria,
>>
>> On Wed, 31 Aug 2022, Victoria Dye via GitGitGadget wrote:
>>
>>> [...]
>>> +
>>> +test_compare_perf () {
>>> +	command="$@"
>>> +	test_perf "$command (scalar)" "
>>> +		(
>>> +			cd scalar-clone/src &&
>>> +			$command
>>> +		)
>>> +	"
>>> +
>>> +	test_perf "$command (non-scalar)" "
>>> +		(
>>> +			cd git-clone &&
>>> +			$command
>>> +		)
>>> +	"
>>> +}
>>> +
>>> +test_compare_perf git status
>>> +test_compare_perf test_commit --append --no-tag A
>>
>> Given the small numbers presented in the commit message, I suspect that
>> even so much as running the command in a subshell might skew the timings
>> at least on Windows, where subshells are very, very expensive.
>>
>> Given that both `git` and `test_commit` understand the `-C <directory>`
>> syntax, this variant would resolve my concern:
>>
>> 	test_compare_perf () {
>> 	     command=$1
>>              shift
>> 	     args="$*"
>>
>> 	     test_perf "$command (scalar)" "
>> 	             $command -C scalar-clone/src $args
>> 	     "
>>
>> 	     test_perf "$command (non-scalar)" "
>> 	             $command -C git-clone $args
>> 	     "
>> 	}
>>
>> What do you think?
> 
> Makes sense to me! Although, out of curiosity, is there a reason you prefer
> "$1 -> shift -> $*" over '$1' and '$@'? 

Whoops, I completely misread your snippet; the 'shift' is necessary to
separate the '$command' out so that we can inject '-C'.

Thanks!

>> Ciao,
>> Dscho