[PATCH v2 0/5] speed up 'make clean'

[PATCH v2 0/5] speed up 'make clean'

[Ramsay Jones]

cover letter for v1
-------------------

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).

Changes in v2
-------------

Unfortunately, immediately after submitting the v1/RFC version of this
series, I got rather busy with other commitments until this evening.
So, I have not had time to look into the 'alternate' patch #8 (I will
get back to that soon, along with some others). Given that I started
this series late in the v2.27 cycle, it would be nice to get some
benefit from this series sooner, rather than later. So, I decided to
go all conservative:

  - remove patch #4 and send a 'standalone' patch to Pratyush (as
    required).
  - remove the controversial patches #7, and #8

I didn't redo the timings, but looking at the table above, we should
see the times on cygwin go from 23.339s to 8.430s (or there abouts)
for this revised series. Also, if Pratyush agrees to apply the new
patch #4, then we should see approximately another 2s saving.

See the range-diff against v1 below.

Thanks!

ATB,
Ramsay Jones

Ramsay Jones (5):
  Documentation/Makefile: conditionally include doc.dep
  Documentation/Makefile: conditionally include ../GIT-VERSION-FILE
  gitweb/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

 Documentation/Makefile |  4 ++++
 Makefile               | 17 +++++++++--------
 gitweb/Makefile        |  2 ++
 3 files changed, 15 insertions(+), 8 deletions(-)


range-diff against v1:
1:  6a0516f3f5 = 1:  c48e0fbaf4 Documentation/Makefile: conditionally include doc.dep
2:  26aecb6c41 = 2:  08c08f44ad Documentation/Makefile: conditionally include ../GIT-VERSION-FILE
3:  91a0bedd37 = 3:  f1b6ef23ae gitweb/Makefile: conditionally include ../GIT-VERSION-FILE
4:  008f1c8892 < -:  ---------- git-gui/Makefile: conditionally include GIT-VERSION-FILE
5:  e5c2414557 = 4:  e8eb7f1668 Makefile: don't try to clean old debian build product
6:  8255a76caf ! 5:  aad82c5116 Makefile: don't use a versioned temp distribution directory
    @@ Commit message
         Makefile: don't use a versioned temp distribution directory
     
         The 'dist' target uses a versioned temp directory, $(GIT_TARNAME), into
    -    which it copies various files added to the distibution tarball. A future
    -    patch requires the 'clean' target not to depend on the $(GIT_VERSION)
    -    variable. However, the $(GIT_TARNAME) variable contains $(GIT_VERSION) in
    -    its definition.
    +    which it copies various files added to the distribution tarball. Should
    +    it be necessary to remove this directory in the 'clean' target, since
    +    the name depends on $(GIT_VERSION), the current HEAD must be positioned
    +    on the same commit as when 'make dist' was issued. Otherwise, the target
    +    will fail to remove that directory.
     
         Create an '.dist-tmp-dir' directory and copy the various files into this
         now un-versioned directory while creating the distribution tarball. Change
7:  273f7f9394 < -:  ---------- Makefile: don't delete dist tarballs directly by name
8:  a04efa056c < -:  ---------- Makefile: conditionally include GIT-VERSION-FILE
-- 
2.29.0

Re: [PATCH v2 0/5] speed up 'make clean'

[Junio C Hamano]

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

> Ramsay Jones (5):
>   Documentation/Makefile: conditionally include doc.dep
>   Documentation/Makefile: conditionally include ../GIT-VERSION-FILE
>   gitweb/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

All five patches looked sensible.  Thanks.