[PATCH v2 1/5] Documentation/Makefile: conditionally include doc.dep

[PATCH v2 1/5] Documentation/Makefile: conditionally include doc.dep

[Ramsay Jones]

The 'clean' target is noticeably slow on cygwin, even for a 'do-nothing'
invocation of 'make clean'. For example, the second 'make clean' below:

  $ make clean >/dev/null 2>&1
  $ make clean
  GIT_VERSION = 2.29.0
  ...
  make[1]: Entering directory '/home/ramsay/git/Documentation'
      GEN mergetools-list.made
      GEN cmd-list.made
      GEN doc.dep
  ...
  $

has been timed at 23.339s, using git v2.29.0, on my laptop (on old core
i5-4200M @ 2.50GHz, 8GB RAM, 1TB HDD).

Notice that, since the 'doc.dep' file does not exist, make takes the
time (about 8s) to generate several files in order to create the doc.dep
include file. (If an 'include' file is missing, but a target for the
said file is present in the Makefile, make will execute that target
and, if that file now exists, throw away all its internal data and
re-read and re-parse the Makefile). Having spent the time to include
the 'doc.dep' file, the 'clean' target immediately deletes those files.

In order to eliminate such wasted effort, use the value of the internal
$(MAKECMDGOALS) variable to only '-include doc.dep' when the target is
not 'clean'. (This drops the time down to 12.364s, on my laptop, giving
an improvement of 47.02%).

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 Documentation/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 80d1908a44..652d57a1b6 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -286,7 +286,9 @@ doc.dep : $(docdep_prereqs) $(wildcard *.txt) $(wildcard config/*.txt) build-doc
 	$(PERL_PATH) ./build-docdep.perl >$@+ $(QUIET_STDERR) && \
 	mv $@+ $@
 
+ifneq ($(MAKECMDGOALS),clean)
 -include doc.dep
+endif
 
 cmds_txt = cmds-ancillaryinterrogators.txt \
 	cmds-ancillarymanipulators.txt \
-- 
2.29.0

Re: [PATCH v2 1/5] Documentation/Makefile: conditionally include doc.dep

[Felipe Contreras]

On Sun, Dec 6, 2020 at 6:35 PM Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:
>
>
> The 'clean' target is noticeably slow on cygwin, even for a 'do-nothing'
> invocation of 'make clean'. For example, the second 'make clean' below:
>
>   $ make clean >/dev/null 2>&1
>   $ make clean
>   GIT_VERSION = 2.29.0
>   ...
>   make[1]: Entering directory '/home/ramsay/git/Documentation'
>       GEN mergetools-list.made
>       GEN cmd-list.made
>       GEN doc.dep
>   ...
>   $
>
> has been timed at 23.339s, using git v2.29.0, on my laptop (on old core
> i5-4200M @ 2.50GHz, 8GB RAM, 1TB HDD).
>
> Notice that, since the 'doc.dep' file does not exist, make takes the
> time (about 8s) to generate several files in order to create the doc.dep
> include file. (If an 'include' file is missing, but a target for the
> said file is present in the Makefile, make will execute that target
> and, if that file now exists, throw away all its internal data and
> re-read and re-parse the Makefile). Having spent the time to include
> the 'doc.dep' file, the 'clean' target immediately deletes those files.
>
> In order to eliminate such wasted effort, use the value of the internal
> $(MAKECMDGOALS) variable to only '-include doc.dep' when the target is
> not 'clean'. (This drops the time down to 12.364s, on my laptop, giving
> an improvement of 47.02%).

All this makes sense, but I had to do "make doc.dep" and take a look
at that file to understand why:

doc.dep contains make rules with targets and dependencies that will
not be used in "make clean".

This is in my opinion the important information. Maybe mention
something like that in the commit message?

Cheers.

-- 
Felipe Contreras

Re: [PATCH v2 1/5] Documentation/Makefile: conditionally include doc.dep

[Junio C Hamano]

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:

> Notice that, since the 'doc.dep' file does not exist, make takes the
> time (about 8s) to generate several files in order to create the doc.dep
> include file. (If an 'include' file is missing, but a target for the
> said file is present in the Makefile, make will execute that target
> and, if that file now exists, throw away all its internal data and
> re-read and re-parse the Makefile). Having spent the time to include
> the 'doc.dep' file, the 'clean' target immediately deletes those files.
>
> In order to eliminate such wasted effort, use the value of the internal
> $(MAKECMDGOALS) variable to only '-include doc.dep' when the target is
> not 'clean'. (This drops the time down to 12.364s, on my laptop, giving
> an improvement of 47.02%).

Nicely explained.  It might be worth saying

    ... the clean target immediately deletes those files.  The rules
    and definitions of doc.dep however does not affect what 'clean'
    target removes otherwise, so we can do without all this.

The last paragraph made me wonder what should happen for 'distclean'
etc., but luckily there is only 'clean' in Documentation/Makefile ;-)

> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
> ---
>  Documentation/Makefile | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index 80d1908a44..652d57a1b6 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -286,7 +286,9 @@ doc.dep : $(docdep_prereqs) $(wildcard *.txt) $(wildcard config/*.txt) build-doc
>  	$(PERL_PATH) ./build-docdep.perl >$@+ $(QUIET_STDERR) && \
>  	mv $@+ $@
>  
> +ifneq ($(MAKECMDGOALS),clean)
>  -include doc.dep
> +endif
>  
>  cmds_txt = cmds-ancillaryinterrogators.txt \
>  	cmds-ancillarymanipulators.txt \

Re: [PATCH v2 1/5] Documentation/Makefile: conditionally include doc.dep

[Ramsay Jones]

On 07/12/2020 03:18, Felipe Contreras wrote:
> On Sun, Dec 6, 2020 at 6:35 PM Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:
[snip commit message]
> 
> All this makes sense, but I had to do "make doc.dep" and take a look
> at that file to understand why:
> 
> doc.dep contains make rules with targets and dependencies that will
> not be used in "make clean".
> 
> This is in my opinion the important information. Maybe mention
> something like that in the commit message?

Yep, Junio made a similar comment. v3 comming soon ... :-D

ATB,
Ramsay Jones

Re: [PATCH v2 1/5] Documentation/Makefile: conditionally include doc.dep

[Ramsay Jones]

On 07/12/2020 07:44, Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
[snip commit message]
> 
> Nicely explained.  It might be worth saying
> 
>     ... the clean target immediately deletes those files.  The rules
>     and definitions of doc.dep however does not affect what 'clean'
>     target removes otherwise, so we can do without all this.

Yep, I have attempted to improve the message along these lines ... Thanks!

v3 coming soon ...

ATB,
Ramsay Jones