diff mbox series

[v5,5/8] Makefile: move ".SUFFIXES" rule to shared.mak

Message ID patch-v5-5.8-18e0a6985f1-20220303T160155Z-avarab@gmail.com (mailing list archive)
State Accepted
Commit dad9cd7d518375799413b5a3af179f4cd95cfbe4
Headers show
Series Makefile: optimize noop runs, add shared.mak | expand

Commit Message

Ævar Arnfjörð Bjarmason March 3, 2022, 4:04 p.m. UTC
This was added in 30248886ce8 (Makefile: disable default implicit
rules, 2010-01-26), let's move it to the top of "shared.mak" so it'll
apply to all our Makefiles.

This doesn't benefit the main Makefile at all, since it already had
the rule, but since we're including shared.mak in other Makefiles
starts to benefit them. E.g. running the 'man" target is now faster:

    $ git -c hyperfine.hook.setup= hyperfine -L rev HEAD~1,HEAD~0 -s 'make -C Documentation man' 'make -C Documentation -j1 man'
    Benchmark 1: make -C Documentation -j1 man' in 'HEAD~1
      Time (mean ± σ):     121.7 ms ±   8.8 ms    [User: 105.8 ms, System: 18.6 ms]
      Range (min … max):   112.8 ms … 148.4 ms    26 runs

    Benchmark 2: make -C Documentation -j1 man' in 'HEAD~0
      Time (mean ± σ):      97.5 ms ±   8.0 ms    [User: 80.1 ms, System: 20.1 ms]
      Range (min … max):    89.8 ms … 111.8 ms    32 runs

    Summary
      'make -C Documentation -j1 man' in 'HEAD~0' ran
        1.25 ± 0.14 times faster than 'make -C Documentation -j1 man' in 'HEAD~1'

The reason for that can be seen when comparing that run with
"--debug=a". Without this change making a target like "git-status.1"
will cause "make" to consider not only "git-status.txt", but
"git-status.txt.o", as well as numerous other implicit suffixes such
as ".c", ".cc", ".cpp" etc. See [1] for a more detailed before/after
example.

So this is causing us to omit a bunch of work we didn't need to
do. For making "git-status.1" the "--debug=a" output is reduced from
~140k lines to ~6k.

1. https://lore.kernel.org/git/220222.86bkyz875k.gmgdl@evledraar.gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile   | 2 --
 shared.mak | 5 +++++
 2 files changed, 5 insertions(+), 2 deletions(-)

Comments

Adam Dinwoodie April 5, 2022, 2:15 p.m. UTC | #1
On Thu, Mar 03, 2022 at 05:04:16PM +0100, Ævar Arnfjörð Bjarmason wrote:
> This was added in 30248886ce8 (Makefile: disable default implicit
> rules, 2010-01-26), let's move it to the top of "shared.mak" so it'll
> apply to all our Makefiles.
> 
> This doesn't benefit the main Makefile at all, since it already had
> the rule, but since we're including shared.mak in other Makefiles
> starts to benefit them. E.g. running the 'man" target is now faster:
> 
>     $ git -c hyperfine.hook.setup= hyperfine -L rev HEAD~1,HEAD~0 -s 'make -C Documentation man' 'make -C Documentation -j1 man'
>     Benchmark 1: make -C Documentation -j1 man' in 'HEAD~1
>       Time (mean ± σ):     121.7 ms ±   8.8 ms    [User: 105.8 ms, System: 18.6 ms]
>       Range (min … max):   112.8 ms … 148.4 ms    26 runs
> 
>     Benchmark 2: make -C Documentation -j1 man' in 'HEAD~0
>       Time (mean ± σ):      97.5 ms ±   8.0 ms    [User: 80.1 ms, System: 20.1 ms]
>       Range (min … max):    89.8 ms … 111.8 ms    32 runs
> 
>     Summary
>       'make -C Documentation -j1 man' in 'HEAD~0' ran
>         1.25 ± 0.14 times faster than 'make -C Documentation -j1 man' in 'HEAD~1'
> 
> The reason for that can be seen when comparing that run with
> "--debug=a". Without this change making a target like "git-status.1"
> will cause "make" to consider not only "git-status.txt", but
> "git-status.txt.o", as well as numerous other implicit suffixes such
> as ".c", ".cc", ".cpp" etc. See [1] for a more detailed before/after
> example.
> 
> So this is causing us to omit a bunch of work we didn't need to
> do. For making "git-status.1" the "--debug=a" output is reduced from
> ~140k lines to ~6k.
> 
> 1. https://lore.kernel.org/git/220222.86bkyz875k.gmgdl@evledraar.gmail.com/
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  Makefile   | 2 --
>  shared.mak | 5 +++++
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 1ac924bd844..ce362720947 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2580,8 +2580,6 @@ ASM_SRC := $(wildcard $(OBJECTS:o=S))
>  ASM_OBJ := $(ASM_SRC:S=o)
>  C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS))
>  
> -.SUFFIXES:
> -
>  $(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
>  	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<
>  $(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
> diff --git a/shared.mak b/shared.mak
> index 29f0e69ecb9..1dda948df09 100644
> --- a/shared.mak
> +++ b/shared.mak
> @@ -9,6 +9,11 @@
>  %:: s.%
>  %:: SCCS/s.%
>  
> +## Likewise delete default $(SUFFIXES). See:
> +##
> +##     info make --index-search=.SUFFIXES
> +.SUFFIXES:
> +
>  ### Flags affecting all rules
>  
>  # A GNU make extension since gmake 3.72 (released in late 1994) to

I confess I really don't understand why, but as part of testing
v2.36.0-rc0 on Cygwin, I've started getting errors building the info
pages, and bisect points to this commit as the culprit.

Specifically, I've been running

    git clean -dffx && make configure && ./configure && make -j4 info

Without this commit, that gets me a successful build; there's a bunch of
noisy warnings that have been hanging around for a long time, and I think
are fundamentally due to the slightly mismatched documentation libraries
that Cygwin has.  With this commit, I get the same noisy warnings, but I
also get the error "could not open .texi: No such file or directory".

I have to confess, I don't really understand this aspect of GNU Make, so
I'm not sure if this is a problem with Cygwin having a bad toolset or
there being something about my environment that means this doesn't work,
but regardless, it's currently causing the Cygwin Git builds to fail.
Ævar Arnfjörð Bjarmason April 5, 2022, 4:04 p.m. UTC | #2
On Tue, Apr 05 2022, Adam Dinwoodie wrote:

> On Thu, Mar 03, 2022 at 05:04:16PM +0100, Ævar Arnfjörð Bjarmason wrote:
>> This was added in 30248886ce8 (Makefile: disable default implicit
>> rules, 2010-01-26), let's move it to the top of "shared.mak" so it'll
>> apply to all our Makefiles.
>> 
>> This doesn't benefit the main Makefile at all, since it already had
>> the rule, but since we're including shared.mak in other Makefiles
>> starts to benefit them. E.g. running the 'man" target is now faster:
>> 
>>     $ git -c hyperfine.hook.setup= hyperfine -L rev HEAD~1,HEAD~0 -s 'make -C Documentation man' 'make -C Documentation -j1 man'
>>     Benchmark 1: make -C Documentation -j1 man' in 'HEAD~1
>>       Time (mean ± σ):     121.7 ms ±   8.8 ms    [User: 105.8 ms, System: 18.6 ms]
>>       Range (min … max):   112.8 ms … 148.4 ms    26 runs
>> 
>>     Benchmark 2: make -C Documentation -j1 man' in 'HEAD~0
>>       Time (mean ± σ):      97.5 ms ±   8.0 ms    [User: 80.1 ms, System: 20.1 ms]
>>       Range (min … max):    89.8 ms … 111.8 ms    32 runs
>> 
>>     Summary
>>       'make -C Documentation -j1 man' in 'HEAD~0' ran
>>         1.25 ± 0.14 times faster than 'make -C Documentation -j1 man' in 'HEAD~1'
>> 
>> The reason for that can be seen when comparing that run with
>> "--debug=a". Without this change making a target like "git-status.1"
>> will cause "make" to consider not only "git-status.txt", but
>> "git-status.txt.o", as well as numerous other implicit suffixes such
>> as ".c", ".cc", ".cpp" etc. See [1] for a more detailed before/after
>> example.
>> 
>> So this is causing us to omit a bunch of work we didn't need to
>> do. For making "git-status.1" the "--debug=a" output is reduced from
>> ~140k lines to ~6k.
>> 
>> 1. https://lore.kernel.org/git/220222.86bkyz875k.gmgdl@evledraar.gmail.com/
>> 
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>  Makefile   | 2 --
>>  shared.mak | 5 +++++
>>  2 files changed, 5 insertions(+), 2 deletions(-)
>> 
>> diff --git a/Makefile b/Makefile
>> index 1ac924bd844..ce362720947 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -2580,8 +2580,6 @@ ASM_SRC := $(wildcard $(OBJECTS:o=S))
>>  ASM_OBJ := $(ASM_SRC:S=o)
>>  C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS))
>>  
>> -.SUFFIXES:
>> -
>>  $(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
>>  	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<
>>  $(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
>> diff --git a/shared.mak b/shared.mak
>> index 29f0e69ecb9..1dda948df09 100644
>> --- a/shared.mak
>> +++ b/shared.mak
>> @@ -9,6 +9,11 @@
>>  %:: s.%
>>  %:: SCCS/s.%
>>  
>> +## Likewise delete default $(SUFFIXES). See:
>> +##
>> +##     info make --index-search=.SUFFIXES
>> +.SUFFIXES:
>> +
>>  ### Flags affecting all rules
>>  
>>  # A GNU make extension since gmake 3.72 (released in late 1994) to
>
> I confess I really don't understand why, but as part of testing
> v2.36.0-rc0 on Cygwin, I've started getting errors building the info
> pages, and bisect points to this commit as the culprit.
>
> Specifically, I've been running
>
>     git clean -dffx && make configure && ./configure && make -j4 info
>
> Without this commit, that gets me a successful build; there's a bunch of
> noisy warnings that have been hanging around for a long time, and I think
> are fundamentally due to the slightly mismatched documentation libraries
> that Cygwin has.  With this commit, I get the same noisy warnings, but I
> also get the error "could not open .texi: No such file or directory".
>
> I have to confess, I don't really understand this aspect of GNU Make, so
> I'm not sure if this is a problem with Cygwin having a bad toolset or
> there being something about my environment that means this doesn't work,
> but regardless, it's currently causing the Cygwin Git builds to fail.

Hi. I can reproduce this locally, will look at it and fix it, sorry.
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index 1ac924bd844..ce362720947 100644
--- a/Makefile
+++ b/Makefile
@@ -2580,8 +2580,6 @@  ASM_SRC := $(wildcard $(OBJECTS:o=S))
 ASM_OBJ := $(ASM_SRC:S=o)
 C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS))
 
-.SUFFIXES:
-
 $(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
 	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<
 $(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
diff --git a/shared.mak b/shared.mak
index 29f0e69ecb9..1dda948df09 100644
--- a/shared.mak
+++ b/shared.mak
@@ -9,6 +9,11 @@ 
 %:: s.%
 %:: SCCS/s.%
 
+## Likewise delete default $(SUFFIXES). See:
+##
+##     info make --index-search=.SUFFIXES
+.SUFFIXES:
+
 ### Flags affecting all rules
 
 # A GNU make extension since gmake 3.72 (released in late 1994) to