git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Phillip Wood" <phillip.wood123@gmail.com>,
	"Jeff King" <peff@peff.net>, "Dan Jacques" <dnj@google.com>,
	"Eric Wong" <e@80x24.org>, "Jonathan Nieder" <jrnieder@gmail.com>,
	"Mike Hommey" <mh@glandium.org>,
	"Đoàn Trần Công Danh" <congdanhqx@gmail.com>,
	"Victoria Dye" <vdye@github.com>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Taylor Blau" <me@ttaylorr.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v3 9/9] Makefiles: add and use wildcard "mkdir -p" template
Date: Fri, 25 Feb 2022 10:04:35 +0100	[thread overview]
Message-ID: <patch-v3-9.9-85bb74aa32f-20220225T090127Z-avarab@gmail.com> (raw)
In-Reply-To: <cover-v3-0.9-00000000000-20220225T090127Z-avarab@gmail.com>

Add a template to do the "mkdir -p" of $(@D) (the parent dir of $@)
for us, and use it for the "make lint-docs" targets I added in
8650c6298c1 (doc lint: make "lint-docs" non-.PHONY, 2021-10-15).

As seen in 4c64fb5aad9 (Documentation/Makefile: fix lint-docs mkdir
dependency, 2021-10-26) maintaining these manual lists of parent
directory dependencies is fragile, in addition to being obviously
verbose.

I used this pattern at the time because I couldn't find another method
than "order-only" prerequisites to avoid doing a "mkdir -p $(@D)" for
every file being created, which as noted in [1] would be significantly
slower.

But as it turns out we can use this neat trick of only doing a "mkdir
-p" if the $(wildcard) macro tells us the path doesn't exist. A re-run
of a performance test similar to that noted downthread of [1] in [2]
shows that this is faster, in addition to being less verbose and more
reliable (this uses my "git-hyperfine" thin wrapper for "hyperfine"[3]):

    $ git -c hyperfine.hook.setup= hyperfine -L rev HEAD~1,HEAD~0 -s 'make -C Documentation lint-docs' -p 'rm -rf Documentation/.build' 'make -C Documentation -j1 lint-docs'
    Benchmark 1: make -C Documentation -j1 lint-docs' in 'HEAD~1
      Time (mean ± σ):      2.914 s ±  0.062 s    [User: 2.449 s, System: 0.489 s]
      Range (min … max):    2.834 s …  3.020 s    10 runs

    Benchmark 2: make -C Documentation -j1 lint-docs' in 'HEAD~0
      Time (mean ± σ):      2.315 s ±  0.062 s    [User: 1.950 s, System: 0.386 s]
      Range (min … max):    2.229 s …  2.397 s    10 runs

    Summary
      'make -C Documentation -j1 lint-docs' in 'HEAD~0' ran
        1.26 ± 0.04 times faster than 'make -C Documentation -j1 lint-docs' in 'HEAD~1'

So let's use that pattern both for the "lint-docs" target, and a few
miscellaneous other targets.

This method of creating parent directories is explicitly racy in that
we don't know if we're going to say always create a "foo" followed by
a "foo/bar" under parallelism, or skip the "foo" because we created
"foo/bar" first. In this case it doesn't matter for anything except
that we aren't guaranteed to get the same number of rules firing when
running make in parallel.

1. https://lore.kernel.org/git/211028.861r45y3pt.gmgdl@evledraar.gmail.com/
2. https://lore.kernel.org/git/211028.86o879vvtp.gmgdl@evledraar.gmail.com/
3. https://gitlab.com/avar/git-hyperfine/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/Makefile | 25 +++----------------------
 Makefile               | 12 +++++++-----
 shared.mak             | 17 +++++++++++++++++
 3 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 0f4ebdeda8a..1eb9192dae8 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -434,25 +434,11 @@ quick-install-html: require-htmlrepo
 print-man1:
 	@for i in $(MAN1_TXT); do echo $$i; done
 
-## Lint: Common
-.build:
-	$(QUIET)mkdir $@
-.build/lint-docs: | .build
-	$(QUIET)mkdir $@
-
 ## Lint: gitlink
-.build/lint-docs/gitlink: | .build/lint-docs
-	$(QUIET)mkdir $@
-.build/lint-docs/gitlink/howto: | .build/lint-docs/gitlink
-	$(QUIET)mkdir $@
-.build/lint-docs/gitlink/config: | .build/lint-docs/gitlink
-	$(QUIET)mkdir $@
 LINT_DOCS_GITLINK = $(patsubst %.txt,.build/lint-docs/gitlink/%.ok,$(HOWTO_TXT) $(DOC_DEP_TXT))
-$(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink
-$(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink/howto
-$(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink/config
 $(LINT_DOCS_GITLINK): lint-gitlink.perl
 $(LINT_DOCS_GITLINK): .build/lint-docs/gitlink/%.ok: %.txt
+	$(call mkdir_p_parent_template)
 	$(QUIET_LINT_GITLINK)$(PERL_PATH) lint-gitlink.perl \
 		$< \
 		$(HOWTO_TXT) $(DOC_DEP_TXT) \
@@ -463,23 +449,18 @@ $(LINT_DOCS_GITLINK): .build/lint-docs/gitlink/%.ok: %.txt
 lint-docs-gitlink: $(LINT_DOCS_GITLINK)
 
 ## Lint: man-end-blurb
-.build/lint-docs/man-end-blurb: | .build/lint-docs
-	$(QUIET)mkdir $@
 LINT_DOCS_MAN_END_BLURB = $(patsubst %.txt,.build/lint-docs/man-end-blurb/%.ok,$(MAN_TXT))
-$(LINT_DOCS_MAN_END_BLURB): | .build/lint-docs/man-end-blurb
 $(LINT_DOCS_MAN_END_BLURB): lint-man-end-blurb.perl
 $(LINT_DOCS_MAN_END_BLURB): .build/lint-docs/man-end-blurb/%.ok: %.txt
+	$(call mkdir_p_parent_template)
 	$(QUIET_LINT_MANEND)$(PERL_PATH) lint-man-end-blurb.perl $< >$@
 .PHONY: lint-docs-man-end-blurb
-lint-docs-man-end-blurb: $(LINT_DOCS_MAN_END_BLURB)
 
 ## Lint: man-section-order
-.build/lint-docs/man-section-order: | .build/lint-docs
-	$(QUIET)mkdir $@
 LINT_DOCS_MAN_SECTION_ORDER = $(patsubst %.txt,.build/lint-docs/man-section-order/%.ok,$(MAN_TXT))
-$(LINT_DOCS_MAN_SECTION_ORDER): | .build/lint-docs/man-section-order
 $(LINT_DOCS_MAN_SECTION_ORDER): lint-man-section-order.perl
 $(LINT_DOCS_MAN_SECTION_ORDER): .build/lint-docs/man-section-order/%.ok: %.txt
+	$(call mkdir_p_parent_template)
 	$(QUIET_LINT_MANSEC)$(PERL_PATH) lint-man-section-order.perl $< >$@
 .PHONY: lint-docs-man-section-order
 lint-docs-man-section-order: $(LINT_DOCS_MAN_SECTION_ORDER)
diff --git a/Makefile b/Makefile
index 9f93ee1532c..549ca6e7a5c 100644
--- a/Makefile
+++ b/Makefile
@@ -2745,7 +2745,8 @@ all:: $(MOFILES)
 endif
 
 po/build/locale/%/LC_MESSAGES/git.mo: po/%.po
-	$(QUIET_MSGFMT)mkdir -p $(dir $@) && $(MSGFMT) -o $@ $<
+	$(call mkdir_p_parent_template)
+	$(QUIET_MSGFMT)$(MSGFMT) -o $@ $<
 
 LIB_PERL := $(wildcard perl/Git.pm perl/Git/*.pm perl/Git/*/*.pm perl/Git/*/*/*.pm)
 LIB_PERL_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_PERL))
@@ -2761,15 +2762,16 @@ NO_PERL_CPAN_FALLBACKS_SQ = $(subst ','\'',$(NO_PERL_CPAN_FALLBACKS))
 endif
 
 perl/build/lib/%.pm: perl/%.pm GIT-PERL-DEFINES
-	$(QUIET_GEN)mkdir -p $(dir $@) && \
+	$(call mkdir_p_parent_template)
+	$(QUIET_GEN) \
 	sed -e 's|@@LOCALEDIR@@|$(perl_localedir_SQ)|g' \
 	    -e 's|@@NO_GETTEXT@@|$(NO_GETTEXT_SQ)|g' \
 	    -e 's|@@NO_PERL_CPAN_FALLBACKS@@|$(NO_PERL_CPAN_FALLBACKS_SQ)|g' \
 	< $< > $@
 
 perl/build/man/man3/Git.3pm: perl/Git.pm
-	$(QUIET_GEN)mkdir -p $(dir $@) && \
-	pod2man $< $@
+	$(call mkdir_p_parent_template)
+	$(QUIET_GEN)pod2man $< $@
 
 $(ETAGS_TARGET): $(FOUND_SOURCE_FILES)
 	$(QUIET_GEN)$(RM) $@+ && \
@@ -2903,7 +2905,7 @@ test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(
 all:: $(TEST_PROGRAMS) $(test_bindir_programs)
 
 bin-wrappers/%: wrap-for-bin.sh
-	@mkdir -p bin-wrappers
+	$(call mkdir_p_parent_template)
 	$(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
 	     -e 's|@@BUILD_DIR@@|$(shell pwd)|' \
 	     -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%$(X),$(@F))$(patsubst git%,$(X),$(filter $(@F),$(BINDIR_PROGRAMS_NEED_X)))|' < $< > $@ && \
diff --git a/shared.mak b/shared.mak
index 6e0f103b65f..7ba04049c8a 100644
--- a/shared.mak
+++ b/shared.mak
@@ -59,6 +59,8 @@ ifndef V
 	QUIET          = @
 	QUIET_GEN      = @echo $(wspfx_SQ) GEN $@;
 
+	QUIET_MKDIR_P_PARENT  = @echo $(wspfx_SQ) MKDIR -p $(@D);
+
 ## Used in "Makefile"
 	QUIET_CC       = @echo $(wspfx_SQ) CC $@;
 	QUIET_AR       = @echo $(wspfx_SQ) AR $@;
@@ -90,3 +92,18 @@ ifndef V
 	export V
 endif
 endif
+
+### Templates
+
+## mkdir_p_parent: lazily "mkdir -p" the path needed for a $@
+## file. Uses $(wildcard) to avoid the "mkdir -p" if it's not
+## needed.
+##
+## Is racy, but in a good way; we might redundantly (and safely)
+## "mkdir -p" when running in parallel, but won't need to exhaustively create
+## individual rules for "a" -> "prefix" -> "dir" -> "file" if given a
+## "a/prefix/dir/file". This can instead be inserted at the start of
+## the "a/prefix/dir/file" rule.
+define mkdir_p_parent_template
+$(if $(wildcard $(@D)),,$(QUIET_MKDIR_P_PARENT)$(shell mkdir -p $(@D)))
+endef
-- 
2.35.1.1175.gf9e1b23ea35


  parent reply	other threads:[~2022-02-25  9:05 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-24 17:37 [PATCH v2 0/8] Makefile: optimize noop runs, add shared.mak Ævar Arnfjörð Bjarmason
2021-12-24 17:37 ` [PATCH v2 1/8] Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it Ævar Arnfjörð Bjarmason
2021-12-24 17:37 ` [PATCH v2 2/8] Makefile: disable GNU make built-in wildcard rules Ævar Arnfjörð Bjarmason
2021-12-24 17:37 ` [PATCH v2 3/8] Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES) Ævar Arnfjörð Bjarmason
2021-12-24 17:37 ` [PATCH v2 4/8] Makefile: move ".SUFFIXES" rule to shared.mak Ævar Arnfjörð Bjarmason
2022-02-22  0:22   ` Taylor Blau
2022-02-22 11:14     ` Ævar Arnfjörð Bjarmason
2021-12-24 17:37 ` [PATCH v2 5/8] Makefile: move $(comma), $(empty) and $(space) " Ævar Arnfjörð Bjarmason
2021-12-24 17:37 ` [PATCH v2 6/8] Makefile: add "$(QUIET)" boilerplate " Ævar Arnfjörð Bjarmason
2021-12-24 17:37 ` [PATCH v2 7/8] Makefile: use $(wspfx) for $(QUIET...) in shared.mak Ævar Arnfjörð Bjarmason
2021-12-24 17:37 ` [PATCH v2 8/8] Makefiles: add and use wildcard "mkdir -p" template Ævar Arnfjörð Bjarmason
2022-02-21 20:17 ` [PATCH v2 0/8] Makefile: optimize noop runs, add shared.mak Ævar Arnfjörð Bjarmason
2022-02-25  9:04 ` [PATCH v3 0/9] " Ævar Arnfjörð Bjarmason
2022-02-25  9:04   ` [PATCH v3 1/9] scalar Makefile: set the default target after the includes Ævar Arnfjörð Bjarmason
2022-02-25 22:43     ` Junio C Hamano
2022-02-25  9:04   ` [PATCH v3 2/9] Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it Ævar Arnfjörð Bjarmason
2022-02-25 22:47     ` Junio C Hamano
2022-02-25 23:05       ` Ævar Arnfjörð Bjarmason
2022-02-25 23:42         ` Junio C Hamano
2022-02-28 10:56     ` Phillip Wood
2022-02-28 11:16       ` Ævar Arnfjörð Bjarmason
2022-02-28 15:51         ` Phillip Wood
2022-02-28 16:34           ` Ævar Arnfjörð Bjarmason
2022-02-25  9:04   ` [PATCH v3 3/9] Makefile: disable GNU make built-in wildcard rules Ævar Arnfjörð Bjarmason
2022-02-25 23:17     ` Junio C Hamano
2022-02-25  9:04   ` [PATCH v3 4/9] Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES) Ævar Arnfjörð Bjarmason
2022-02-25  9:04   ` [PATCH v3 5/9] Makefile: move ".SUFFIXES" rule to shared.mak Ævar Arnfjörð Bjarmason
2022-02-25 23:19     ` Junio C Hamano
2022-02-25  9:04   ` [PATCH v3 6/9] Makefile: move $(comma), $(empty) and $(space) " Ævar Arnfjörð Bjarmason
2022-02-25 23:24     ` Junio C Hamano
2022-02-25  9:04   ` [PATCH v3 7/9] Makefile: add "$(QUIET)" boilerplate " Ævar Arnfjörð Bjarmason
2022-02-25 23:27     ` Junio C Hamano
2022-02-25  9:04   ` [PATCH v3 8/9] Makefile: use $(wspfx) for $(QUIET...) in shared.mak Ævar Arnfjörð Bjarmason
2022-02-25 23:30     ` Junio C Hamano
2022-02-25  9:04   ` Ævar Arnfjörð Bjarmason [this message]
2022-03-02 12:49   ` [PATCH v4 0/9] Makefile: optimize noop runs, add shared.mak Ævar Arnfjörð Bjarmason
2022-03-02 12:49     ` [PATCH v4 1/9] scalar Makefile: use "The default target of..." pattern Ævar Arnfjörð Bjarmason
2022-03-02 19:35       ` Junio C Hamano
2022-03-02 12:49     ` [PATCH v4 2/9] Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it Ævar Arnfjörð Bjarmason
2022-03-02 12:49     ` [PATCH v4 3/9] Makefile: disable GNU make built-in wildcard rules Ævar Arnfjörð Bjarmason
2022-03-02 12:49     ` [PATCH v4 4/9] Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES) Ævar Arnfjörð Bjarmason
2022-03-02 12:49     ` [PATCH v4 5/9] Makefile: move ".SUFFIXES" rule to shared.mak Ævar Arnfjörð Bjarmason
2022-03-02 12:49     ` [PATCH v4 6/9] Makefile: move $(comma), $(empty) and $(space) " Ævar Arnfjörð Bjarmason
2022-03-02 12:49     ` [PATCH v4 7/9] Makefile: add "$(QUIET)" boilerplate " Ævar Arnfjörð Bjarmason
2022-03-02 12:49     ` [PATCH v4 8/9] Makefile: use $(wspfx) for $(QUIET...) in shared.mak Ævar Arnfjörð Bjarmason
2022-03-02 19:26       ` Junio C Hamano
2022-03-02 12:49     ` [PATCH v4 9/9] Makefiles: add and use wildcard "mkdir -p" template Ævar Arnfjörð Bjarmason
2022-03-02 20:38       ` Junio C Hamano
2022-03-02 20:39     ` [PATCH v4 0/9] Makefile: optimize noop runs, add shared.mak Junio C Hamano
2022-03-03 14:08     ` Phillip Wood
2022-03-03 16:04     ` [PATCH v5 0/8] " Ævar Arnfjörð Bjarmason
2022-03-03 16:04       ` [PATCH v5 1/8] scalar Makefile: use "The default target of..." pattern Ævar Arnfjörð Bjarmason
2022-03-03 16:04       ` [PATCH v5 2/8] Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it Ævar Arnfjörð Bjarmason
2022-03-03 16:04       ` [PATCH v5 3/8] Makefile: disable GNU make built-in wildcard rules Ævar Arnfjörð Bjarmason
2022-04-11 10:05         ` Rene Kita
2022-04-11 10:11           ` Ævar Arnfjörð Bjarmason
2022-03-03 16:04       ` [PATCH v5 4/8] Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES) Ævar Arnfjörð Bjarmason
2022-03-03 16:04       ` [PATCH v5 5/8] Makefile: move ".SUFFIXES" rule to shared.mak Ævar Arnfjörð Bjarmason
2022-04-05 14:15         ` Adam Dinwoodie
2022-04-05 16:04           ` Ævar Arnfjörð Bjarmason
2022-04-05 19:56           ` [PATCH] Documentation/Makefile: fix "make info" regression in dad9cd7d518 Ævar Arnfjörð Bjarmason
2022-04-06  8:26             ` Adam Dinwoodie
2022-04-06 16:43               ` Junio C Hamano
2022-04-06 17:05                 ` Taylor Blau
2022-03-03 16:04       ` [PATCH v5 6/8] Makefile: move $(comma), $(empty) and $(space) to shared.mak Ævar Arnfjörð Bjarmason
2022-03-03 16:04       ` [PATCH v5 7/8] Makefile: add "$(QUIET)" boilerplate " Ævar Arnfjörð Bjarmason
2022-03-03 16:04       ` [PATCH v5 8/8] Makefiles: add and use wildcard "mkdir -p" template Ævar Arnfjörð Bjarmason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=patch-v3-9.9-85bb74aa32f-20220225T090127Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=congdanhqx@gmail.com \
    --cc=dnj@google.com \
    --cc=e@80x24.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=me@ttaylorr.com \
    --cc=mh@glandium.org \
    --cc=peff@peff.net \
    --cc=phillip.wood123@gmail.com \
    --cc=sunshine@sunshineco.com \
    --cc=vdye@github.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).