[RFC PATCH 0/8] speed up 'make clean'

[RFC PATCH 0/8] speed up 'make clean'

[Ramsay Jones]

Approximately five years ago, I stopped building the documentation
locally and instead started using the 'pre-built' documentation repos
from git://git.kernel.org/pub/scm/git/git-{htmldocs,manpages}.git.
I placed them next to the git repository (ie. as siblings) and wrote
a script (mk-dist-doc.sh) to reproduce the effect of 'make dist-doc',
using git-archive, to place the distribution tarballs at the top of
the git repo worktree.

[During the creation of these patches, I actually did a 'make' in
the Documentation directory and was surprised that it succeeded!
However, it reminded me why I stopped building the docs! :-P ]

This meant that the Documentation directory was essentially unused
for build purposes, so that a 'make clean' should have nothing to
do in that directory.  However, on cygwin, I noticed a long pause
during a 'make clean' in that directory and, further, it seemed to
be GEN-erating some files!  I put an item on my TODO list to take a
look at that - during the v2.28.0 cycle, I finally found time to look
at this issue! :-D

This patch series is the result. Note that this was mainly about
speeding up 'make clean' on cygwin (and the performance numbers
in the patches reflect use on cygwin). Tonight, I did some timings
on Linux as well, so that I could show this table:

              CYGWIN              LINUX
v2.29.0   23.339s              1.709s
Patch #1  12.364s  -47.02%     0.877s  -48.68%
Patch #2  10.361s  -16.20%     0.805s   -8.21%
Patch #3   8.430s  -18.64%     0.672s  -16.52%
Patch #4   6.454s  -23.44%     0.484s  -27.98%
Patch #5   6.428s   -0.40%     0.503s   +3.93%
Patch #6   6.440s   +0.19%     0.517s   +2.78%
Patch #7   6.430s   -0.16%     0.515s   -0.39%
Patch #8   4.064s  -36.80%     0.322s  -37.48%

         23.339/4.064 = 5.74   1.709/0.322 = 5.31 

Note that patches 5-7 are just preparatory patches for the final
patch, and are not expected to show any improvement.

This series is marked RFC because I forgot that 'git-gui' patches
need to be separately submitted to Pratyush Yadav, the git-gui
maintainer. So, I need to drop patch #4 from this series and
submit that to Pratyush (the commit message needs to be re-written
as well). Unfortunately, this means that the following commit
messages will need to be re-written (and timings re-done).

Also, since this series is based on v2.29.0, I had anticipated some
conflicts with the 'rs/dist-doc-with-git-archive' branch when merging
this with the 'master', 'next' and 'seen' branches. I have just done
a trial merge with each branch and, to my surprise, there were no
conflicts and they (auto) merged clean. ;)

However, I can certainly rebase these onto the 'master' branch, if that
would be easier to deal with. (There has to be a v2 anyway, so ...)

I should note here that the main idea used in these patches, to use
the $(MAKECMDGOALS) variable to conditionally include some files, can
be defeated by invoking make with multiple goals. For example, if you
were to do 'make clean all', then $(MAKECMDGOALS) would be 'clean all'
and not 'clean'.

However, doing so with the top-level Makefile would only negate the
effect of the last patch, since all of the sub-makes are issued as
'make -C <dir> clean'. So, all of the conditional includes in patches
#1-4 will work as intended (and show the noted improvement).

[Yes, 'cd Documentation; make clean all' will be slower that a doing
separate 'make clean; make', but the extra 10s, or so, will be swamped
by the documentation build time! ;-) ]

ATB,
Ramsay Jones

Ramsay Jones (8):
  Documentation/Makefile: conditionally include doc.dep
  Documentation/Makefile: conditionally include ../GIT-VERSION-FILE
  gitweb/Makefile: conditionally include ../GIT-VERSION-FILE
  git-gui/Makefile: conditionally include GIT-VERSION-FILE
  Makefile: don't try to clean old debian build product
  Makefile: don't use a versioned temp distribution directory
  Makefile: don't delete dist tarballs directly by name
  Makefile: conditionally include GIT-VERSION-FILE

 .gitignore             |  1 +
 Documentation/Makefile |  4 ++++
 Makefile               | 28 +++++++++++++++++++---------
 git-gui/Makefile       |  2 ++
 gitweb/Makefile        |  2 ++
 5 files changed, 28 insertions(+), 9 deletions(-)

-- 
2.29.0

Re: [RFC PATCH 0/8] speed up 'make clean'

[Junio C Hamano]

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

> [Yes, 'cd Documentation; make clean all' will be slower that a doing
> separate 'make clean; make', but the extra 10s, or so, will be swamped
> by the documentation build time! ;-) ]

Hmph, the "all" part in "make clean all" needs the information we
read from these generated files, and time must be spent to generate
them whether "make clean all" or "make clean; make all" is used.  In
the latter, we may not generate and read them in the first phase,
but the second one "make all" would need to do so anyway.  So I am
puzzled why "make clean all" needs to be slower---don't we generate
and read them only once in either case?

Re: [RFC PATCH 0/8] speed up 'make clean'

[Ramsay Jones]

On 05/11/2020 21:48, Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> 
>> [Yes, 'cd Documentation; make clean all' will be slower that a doing
>> separate 'make clean; make', but the extra 10s, or so, will be swamped
>> by the documentation build time! ;-) ]
> 
> Hmph, the "all" part in "make clean all" needs the information we
> read from these generated files, and time must be spent to generate
> them whether "make clean all" or "make clean; make all" is used.  In
> the latter, we may not generate and read them in the first phase,
> but the second one "make all" would need to do so anyway.  So I am
> puzzled why "make clean all" needs to be slower---don't we generate
> and read them only once in either case?
> 

Hmm, interesting. I was all ready to explain the results of the
moc-up of this that I did about a month ago, but thought I should
just check again ... ;-)

This time I used the actual Makefile, rather than a moc-up, and got
different (in some sense, worse) results. What I was going to say
was, no the doc.dep file gets generated twice - but that is not true. :(

However, if you run 'make clean all', you will not be pleased with
the results!

  $ make clean
  ...
  $ make clean all >zzz 2>&1
  $ grep WARNING zzz | wc -l
  26
  $ tail -1 zzz
  make: *** [Makefile:362: git.1] Error 13
  $ 

If you look at the output, you will see that, while processing the
'clean' target, the 'doc.dep' file is created, -included, and then
immediately deleted. While processing the 'all' target I had expected
the 'doc.dep' file to be recreated - but it isn't. It seems to have
done the 'drop the internal data, re-read and re-parse' only the
once, and on the second go round (because it has already generated
it once) does not re-create and re-read the dependency data again.
Thus, the 'all' target is executed without any 'doc.dep' data and
falls over in a heap. :(

[BTW, just doing 'make' does not do a full build. I will look at why
that is later].

Doing 'make clean; make all' works fine, of course.

That will teach me to cut corners! Ho-Hum. Sorry about that.

Thanks.

ATB,
Ramsay Jones