RFC patch, 8 partsspeed up 'make clean'

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

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

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?

On 05/11/2020 21:48, Junio C Hamano wrote:

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