On 05/11/2020 21:52, Junio C Hamano wrote:
> Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxxx> writes:
>
>> The 'clean' target is still noticeably slow on cygwin, despite the
>> substantial improvement made by the previous patch. For example, the
>> second invocation of 'make clean' below:
>>
>> $ make clean >/dev/null 2>&1
>> $ make clean
>> ...
>> make[1]: Entering directory '/home/ramsay/git/Documentation'
>> make[2]: Entering directory '/home/ramsay/git'
>> make[2]: 'GIT-VERSION-FILE' is up to date.
>> make[2]: Leaving directory '/home/ramsay/git'
>> ...
>> $
>>
>> has been timed at 12.364s on my laptop (on old core i5-4200M @ 2.50GHz,
>> 8GB RAM, 1TB HDD).
>>
>> Notice that the 'clean' target is making a nested call to the parent
>> Makefile to ensure that the GIT-VERSION-FILE is up-to-date (prior to
>> the previous patch, there would have been _two_ such invocations).
>> This is to ensure that the $(GIT_VERSION) make variable is set, once
>> that file had been included. However, the 'clean' target does not use
>> the $(GIT_VERSION) variable, so this is wasted effort.
>>
>> In order to eliminate such wasted effort, use the value of the internal
>> $(MAKECMDGOALS) variable to only '-include ../GIT-VERSION-FILE' when the
>> target is not 'clean'. (This drops the time down to 10.361s, on my laptop,
>> giving an improvement of 16.20%).
>
> This obviously relies on the fact that none of our build products to
> be cleaned are named using $(GIT_VERSION)---in other words, if our
> cleaning rule contained
>
> rm -f git-$(GIT_VERSION)-manual.html
>
> this optimization would not work well.
Yes, indeed.
> Luckily, I think we use GIT_VERSION only to engrave version number
> in the resulting document, and it does not affect the name of the
> build product, so this change is safe, I think.
Yes, as I said in the commit message above:
"However, the 'clean' target does not use
the $(GIT_VERSION) variable, so this is wasted effort."
Thanks.
ATB,
Ramsay Jones
