{"thread":{"id":"54778","subject":"[PATCH v2 0/5] speed up 'make clean'","startedAt":"2020-12-07T00:31:29Z","lastAt":"2020-12-07T07:48:19Z","messageCount":2,"participants":["Ramsay Jones","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":5},"messages":[{"id":"411572","messageId":"a5f59bcf-9ae6-b3a8-7073-5797971d286f@ramsayjones.plus.com","threadId":"54778","inReplyTo":null,"subject":"[PATCH v2 0/5] speed up 'make clean'","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2020-12-07T00:30:31Z","receivedAt":"2020-12-07T00:31:29Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\ncover letter for v1\n-------------------\n\nApproximately five years ago, I stopped building the documentation\nlocally and instead started using the 'pre-built' documentation repos\nfrom git://git.kernel.org/pub/scm/git/git-{htmldocs,manpages}.git.\nI placed them next to the git repository (ie. as siblings) and wrote\na script (mk-dist-doc.sh) to reproduce the effect of 'make dist-doc',\nusing git-archive, to place the distribution tarballs at the top of\nthe git repo worktree.\n\n[During the creation of these patches, I actually did a 'make' in\nthe Documentation directory and was surprised that it succeeded!\nHowever, it reminded me why I stopped building the docs! :-P ]\n\nThis meant that the Documentation directory was essentially unused\nfor build purposes, so that a 'make clean' should have nothing to\ndo in that directory. However, on cygwin, I noticed a long pause\nduring a 'make clean' in that directory and, further, it seemed to\nbe GEN-erating some files! I put an item on my TODO list to take a\nlook at that - during the v2.28.0 cycle, I finally found time to look\nat this issue! :-D\n\nThis patch series is the result. Note that this was mainly about\nspeeding up 'make clean' on cygwin (and the performance numbers\nin the patches reflect use on cygwin). Tonight, I did some timings\non Linux as well, so that I could show this table:\n\n CYGWIN LINUX\nv2.29.0 23.339s 1.709s\nPatch #1 12.364s -47.02% 0.877s -48.68%\nPatch #2 10.361s -16.20% 0.805s -8.21%\nPatch #3 8.430s -18.64% 0.672s -16.52%\nPatch #4 6.454s -23.44% 0.484s -27.98%\nPatch #5 6.428s -0.40% 0.503s +3.93%\nPatch #6 6.440s +0.19% 0.517s +2.78%\nPatch #7 6.430s -0.16% 0.515s -0.39%\nPatch #8 4.064s -36.80% 0.322s -37.48%\n\n 23.339/4.064 = 5.74 1.709/0.322 = 5.31 \n\nNote that patches 5-7 are just preparatory patches for the final\npatch, and are not expected to show any improvement.\n\nThis series is marked RFC because I forgot that 'git-gui' patches\nneed to be separately submitted to Pratyush Yadav, the git-gui\nmaintainer. So, I need to drop patch #4 from this series and\nsubmit that to Pratyush (the commit message needs to be re-written\nas well). Unfortunately, this means that the following commit\nmessages will need to be re-written (and timings re-done).\n\nAlso, since this series is based on v2.29.0, I had anticipated some\nconflicts with the 'rs/dist-doc-with-git-archive' branch when merging\nthis with the 'master', 'next' and 'seen' branches. I have just done\na trial merge with each branch and, to my surprise, there were no\nconflicts and they (auto) merged clean. ;)\n\nHowever, I can certainly rebase these onto the 'master' branch, if that\nwould be easier to deal with. (There has to be a v2 anyway, so ...)\n\nI should note here that the main idea used in these patches, to use\nthe $(MAKECMDGOALS) variable to conditionally include some files, can\nbe defeated by invoking make with multiple goals. For example, if you\nwere to do 'make clean all', then $(MAKECMDGOALS) would be 'clean all'\nand not 'clean'.\n\nHowever, doing so with the top-level Makefile would only negate the\neffect of the last patch, since all of the sub-makes are issued as\n'make -C clean'. So, all of the conditional includes in patches\n#1-4 will work as intended (and show the noted improvement).\n\nChanges in v2\n-------------\n\nUnfortunately, immediately after submitting the v1/RFC version of this\nseries, I got rather busy with other commitments until this evening.\nSo, I have not had time to look into the 'alternate' patch #8 (I will\nget back to that soon, along with some others). Given that I started\nthis series late in the v2.27 cycle, it would be nice to get some\nbenefit from this series sooner, rather than later. So, I decided to\ngo all conservative:\n\n - remove patch #4 and send a 'standalone' patch to Pratyush (as\n required).\n - remove the controversial patches #7, and #8\n\nI didn't redo the timings, but looking at the table above, we should\nsee the times on cygwin go from 23.339s to 8.430s (or there abouts)\nfor this revised series. Also, if Pratyush agrees to apply the new\npatch #4, then we should see approximately another 2s saving.\n\nSee the range-diff against v1 below.\n\nThanks!\n\nATB,\nRamsay Jones\n\nRamsay Jones (5):\n Documentation/Makefile: conditionally include doc.dep\n Documentation/Makefile: conditionally include ../GIT-VERSION-FILE\n gitweb/Makefile: conditionally include ../GIT-VERSION-FILE\n Makefile: don't try to clean old debian build product\n Makefile: don't use a versioned temp distribution directory\n\n Documentation/Makefile | 4 ++++\n Makefile | 17 +++++++++--------\n gitweb/Makefile | 2 ++\n 3 files changed, 15 insertions(+), 8 deletions(-)\n\n\nrange-diff against v1:\n1: 6a0516f3f5 = 1: c48e0fbaf4 Documentation/Makefile: conditionally include doc.dep\n2: 26aecb6c41 = 2: 08c08f44ad Documentation/Makefile: conditionally include ../GIT-VERSION-FILE\n3: 91a0bedd37 = 3: f1b6ef23ae gitweb/Makefile: conditionally include ../GIT-VERSION-FILE\n4: 008f1c8892 < -: ---------- git-gui/Makefile: conditionally include GIT-VERSION-FILE\n5: e5c2414557 = 4: e8eb7f1668 Makefile: don't try to clean old debian build product\n6: 8255a76caf ! 5: aad82c5116 Makefile: don't use a versioned temp distribution directory\n @@ Commit message\n Makefile: don't use a versioned temp distribution directory\n \n The 'dist' target uses a versioned temp directory, $(GIT_TARNAME), into\n - which it copies various files added to the distibution tarball. A future\n - patch requires the 'clean' target not to depend on the $(GIT_VERSION)\n - variable. However, the $(GIT_TARNAME) variable contains $(GIT_VERSION) in\n - its definition.\n + which it copies various files added to the distribution tarball. Should\n + it be necessary to remove this directory in the 'clean' target, since\n + the name depends on $(GIT_VERSION), the current HEAD must be positioned\n + on the same commit as when 'make dist' was issued. Otherwise, the target\n + will fail to remove that directory.\n \n Create an '.dist-tmp-dir' directory and copy the various files into this\n now un-versioned directory while creating the distribution tarball. Change\n7: 273f7f9394 < -: ---------- Makefile: don't delete dist tarballs directly by name\n8: a04efa056c < -: ---------- Makefile: conditionally include GIT-VERSION-FILE\n-- \n2.29.0\n"},{"id":"411590","messageId":"xmqqtusyawh8.fsf@gitster.c.googlers.com","threadId":"54778","inReplyTo":"a5f59bcf-9ae6-b3a8-7073-5797971d286f@ramsayjones.plus.com","subject":"Re: [PATCH v2 0/5] speed up 'make clean'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-07T07:47:31Z","receivedAt":"2020-12-07T07:48:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones writes:\n\n> Ramsay Jones (5):\n> Documentation/Makefile: conditionally include doc.dep\n> Documentation/Makefile: conditionally include ../GIT-VERSION-FILE\n> gitweb/Makefile: conditionally include ../GIT-VERSION-FILE\n> Makefile: don't try to clean old debian build product\n> Makefile: don't use a versioned temp distribution directory\n\nAll five patches looked sensible. Thanks.\n"}]}