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 v4 0/9] Makefile: optimize noop runs, add shared.mak
Date: Wed,  2 Mar 2022 13:49:08 +0100	[thread overview]
Message-ID: <cover-v4-0.9-00000000000-20220302T124320Z-avarab@gmail.com> (raw)
In-Reply-To: <cover-v3-0.9-00000000000-20220225T090127Z-avarab@gmail.com>

As the v1 notes (among other things):
https://lore.kernel.org/git/cover-0.8-00000000000-20211217T012902Z-avarab@gmail.com/

    This speeds up noop runs of "make" by a lot. After a "make" running a
    "make -j1" with this is ~1.5 faster than on "master"[2], and around 3x
    as fast with "make -j1 NO_TCLTK=Y" (the TCL part takes a lot of time,
    but that's another matter).

This v4 re-roll (for v3, see
https://lore.kernel.org/git/cover-v3-0.9-00000000000-20220225T090127Z-avarab@gmail.com/):

 * The "all" boilerplate goes first now, before "include"
 * Elaborated on DELETE_ON_ERROR ina commit message.
 * No longer change a ":=" to "=" while moving code.
 * Rephrased other commit messages (one of which referred to a
   function from another future series)
 * Typo fix in commit message.

Ævar Arnfjörð Bjarmason (9):
  scalar Makefile: use "The default target of..." pattern
  Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it
  Makefile: disable GNU make built-in wildcard rules
  Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES)
  Makefile: move ".SUFFIXES" rule to shared.mak
  Makefile: move $(comma), $(empty) and $(space) to shared.mak
  Makefile: add "$(QUIET)" boilerplate to shared.mak
  Makefile: use $(wspfx) for $(QUIET...) in shared.mak
  Makefiles: add and use wildcard "mkdir -p" template

 Documentation/Makefile    |  63 ++------------------
 Makefile                  | 118 ++++++++++++--------------------------
 config.mak.uname          |   1 -
 contrib/scalar/Makefile   |  20 ++-----
 contrib/scalar/t/Makefile |   3 +
 shared.mak                | 109 +++++++++++++++++++++++++++++++++++
 t/Makefile                |   3 +
 t/interop/Makefile        |   3 +
 t/perf/Makefile           |   3 +
 templates/Makefile        |   8 +--
 10 files changed, 171 insertions(+), 160 deletions(-)
 create mode 100644 shared.mak

Range-diff against v3:
 1:  2404c4d8b96 <  -:  ----------- scalar Makefile: set the default target after the includes
 -:  ----------- >  1:  26c6bb897cf scalar Makefile: use "The default target of..." pattern
 2:  96a490bec54 !  2:  74692458b70 Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it
    @@ Commit message
         .DELETE_ON_ERROR, 2021-05-21) for the addition and use of the
         ".DELETE_ON_ERROR" flag.
     
    -    This does have the potential downside that if e.g. templates/Makefile
    -    would like to include this "shared.mak" in the future the semantics of
    -    such a Makefile will change, but as noted in the above commits (and
    -    GNU make's own documentation) any such change would be for the better,
    -    so it's safe to do this.
    +    I.e. this changes the behavior of existing rules in the altered
    +    Makefiles (except "Makefile" & "Documentation/Makefile"). I'm
    +    confident that this is safe having read the relevant rules in those
    +    Makfiles, and as the GNU make manual notes that it isn't the default
    +    behavior is out of an abundance of backwards compatibility
    +    caution. From edition 0.75 of its manual, covering GNU make 4.3:
     
    -    This also doesn't introduce a bug by e.g. having this
    +        [Enabling '.DELETE_ON_ERROR' is] almost always what you want
    +        'make' to do, but it is not historical practice; so for
    +        compatibility, you must explicitly request it.
    +
    +    This doesn't introduce a bug by e.g. having this
         ".DELETE_ON_ERROR" flag only apply to this new shared.mak, Makefiles
         have no such scoping semantics.
     
    +    It does increase the danger that any Makefile without an explicit "The
    +    default target of this Makefile is..." snippet to define the default
    +    target as "all" could have its default rule changed if our new
    +    shared.mak ever defines a "real" rule. In subsequent commits we'll be
    +    careful not to do that, and such breakage would be obvious e.g. in the
    +    case of "make -C t".
    +
    +    We might want to make that less fragile still (e.g. by using
    +    ".DEFAULT_GOAL" as noted in the preceding commit), but for now let's
    +    simply include "shared.mak" without adding that boilerplate to all the
    +    Makefiles that don't have it already. Most of those are already
    +    exposed to that potential caveat e.g. due to including "config.mak*".
    +
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## Documentation/Makefile ##
    @@ Documentation/Makefile: doc-l10n install-l10n::
     
      ## Makefile ##
     @@
    -+# Import tree-wide shared Makefile behavior and libraries
    -+include shared.mak
    -+
      # The default target of this Makefile is...
      all::
      
    ++# Import tree-wide shared Makefile behavior and libraries
    ++include shared.mak
    ++
    + # Define V=1 to have a more verbose compile.
    + #
    + # Define SHELL_PATH to a POSIX shell if your /bin/sh is broken.
     @@ Makefile: shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell
      strip: $(PROGRAMS) git$X
      	$(STRIP) $(STRIP_OPTS) $^
    @@ Makefile: shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell
     
      ## contrib/scalar/Makefile ##
     @@
    + # The default target of this Makefile is...
    + all::
    + 
     +# Import tree-wide shared Makefile behavior and libraries
     +include ../../shared.mak
     +
    - QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
    - QUIET_SUBDIR1  =
    - 
    + include ../../config.mak.uname
    + -include ../../config.mak.autogen
    + -include ../../config.mak
     
      ## contrib/scalar/t/Makefile ##
     @@
 3:  9392e3c3e97 !  3:  0fbdeeffc7b Makefile: disable GNU make built-in wildcard rules
    @@ Commit message
         contents from RCS or SCCS. See [1] for an old mailing list discussion
         about how to disable these.
     
    -    The speed-up may wary. I've seen 1-10% depending on the speed of the
    +    The speed-up may vary. I've seen 1-10% depending on the speed of the
         local disk, caches, -jN etc. Running:
     
             strace -f -c -S calls make -j1 NO_TCLTK=Y
 4:  07cf9daa9d6 =  4:  ea6b835308a Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES)
 5:  16f2e3ff35b =  5:  c2339694cf7 Makefile: move ".SUFFIXES" rule to shared.mak
 6:  1b6ecb27f02 !  6:  741fdfd48e2 Makefile: move $(comma), $(empty) and $(space) to shared.mak
    @@ Metadata
      ## Commit message ##
         Makefile: move $(comma), $(empty) and $(space) to shared.mak
     
    -    Move these variables over to the shared.max, we'll make use of them in
    -    a subsequent commit. There was no reason for these to be "simply
    -    expanded variables", so let's use the normal lazy "=" assignment here.
    +    Move these variables over to the shared.mak, we'll make use of them in
    +    a subsequent commit.
    +
    +    Note that there's reason for these to be "simply expanded variables",
    +    i.e. to use ":=" assignments instead of lazily expanded "="
    +    assignments. We could use "=", but let's leave this as-is for now for
    +    ease of review.
     
         See 425ca6710b2 (Makefile: allow combining UBSan with other
         sanitizers, 2017-07-15) for the commit that introduced these.
    @@ shared.mak
     +
     +## comma, empty, space: handy variables as these tokens are either
     +## special or can be hard to spot among other Makefile syntax.
    -+comma = ,
    -+empty =
    -+space = $(empty) $(empty)
    ++comma := ,
    ++empty :=
    ++space := $(empty) $(empty)
 7:  471067deefc !  7:  a723cbce270 Makefile: add "$(QUIET)" boilerplate to shared.mak
    @@ config.mak.uname: vcxproj:
      
     
      ## contrib/scalar/Makefile ##
    -@@
    - # Import tree-wide shared Makefile behavior and libraries
    - include ../../shared.mak
    +@@ contrib/scalar/Makefile: include ../../config.mak.uname
    + -include ../../config.mak.autogen
    + -include ../../config.mak
      
     -QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
     -QUIET_SUBDIR1  =
    @@ contrib/scalar/Makefile
     -endif
     -endif
     -
    - include ../../config.mak.uname
    - -include ../../config.mak.autogen
    - -include ../../config.mak
    + TARGETS = scalar$(X) scalar.o
    + GITLIBS = ../../common-main.o ../../libgit.a ../../xdiff/lib.a
    + 
     
      ## shared.mak ##
     @@
    - comma = ,
    - empty =
    - space = $(empty) $(empty)
    + comma := ,
    + empty :=
    + space := $(empty) $(empty)
     +
     +### Quieting
     +## common
 8:  510306d2219 !  8:  3733b0c8df1 Makefile: use $(wspfx) for $(QUIET...) in shared.mak
    @@ Commit message
         Makefile: use $(wspfx) for $(QUIET...) in shared.mak
     
         Change the mostly move-only change in the preceding commit to use the
    -    $(wspfx) variable for defining the QUIET padding, to guarantee that
    -    it's consistent with the "TRACK_template" template.
    +    $(wspfx) variable for defining the QUIET padding. This refactoring
    +    will make it easier to emit that exact amount of padding in functions
    +    that we might add to shared.mak in the future.
     
    -        $ make CFLAGS=-I$RANDOM grep.o wspfx='$(space)->'
    -         -> GIT-CFLAGS PARAMETERS (changed)
    -         -> CC grep.o
    +    Such a function is not part of this patch series, but a
    +    "TRACK_template" that I'd like to add as a follow-up to it makes use
    +    of this. Let's make this change now while modifying these QUIET rules
    +    is fresh in our minds.
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## shared.mak ##
    -@@ shared.mak: comma = ,
    - empty =
    - space = $(empty) $(empty)
    +@@ shared.mak: comma := ,
    + empty :=
    + space := $(empty) $(empty)
      
     +## wspfx: the whitespace prefix padding for $(QUIET...) and similarly
     +## aligned output.
 9:  85bb74aa32f =  9:  4cc4aeabb20 Makefiles: add and use wildcard "mkdir -p" template
-- 
2.35.1.1228.g56895c6ee86


  parent reply	other threads:[~2022-03-02 12:49 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   ` [PATCH v3 9/9] Makefiles: add and use wildcard "mkdir -p" template Ævar Arnfjörð Bjarmason
2022-03-02 12:49   ` Ævar Arnfjörð Bjarmason [this message]
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=cover-v4-0.9-00000000000-20220302T124320Z-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).