| Message ID | a5f59bcf-9ae6-b3a8-7073-5797971d286f@ramsayjones.plus.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | speed up 'make clean' | expand |
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.
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