Re: [PATCH resend] Makefile: Use computed header dependencies if the compiler supports it

From: Fredrik Kuivinen <frekui@gmail.com>
Date: Aug 14, 2011, 19:53 UTC
Message-ID: <CALx8hKRBjXr44gM1JA+d=RU80pmruPV56s-G3JvViz87eJ=ajQ@mail.gmail.com>
In-Reply-To: <20110814190050.GA16819@elie.gateway.2wire.net>

Hi Jonathan,

Thanks for your comments.

On Sun, Aug 14, 2011 at 21:00, Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 22 quoted lines

>> --- a/Makefile
>> +++ b/Makefile
> [...]
>> @@ -1236,6 +1232,15 @@ endif
>>  ifdef CHECK_HEADER_DEPENDENCIES
>>  COMPUTE_HEADER_DEPENDENCIES =
>>  USE_COMPUTED_HEADER_DEPENDENCIES =
>> +else
>> +dep_check = $(shell sh -c \
>> +     ': > ++empty.c; \
>> +     $(CC) -c -MF /dev/null -MMD -MP ++empty.c -o /dev/null 2>&1; \
>> +     echo $$?; \
>> +     $(RM) ++empty.c')
>> +ifeq ($(dep_check),0)
>> +COMPUTE_HEADER_DEPENDENCIES=YesPlease
>> +endif
>
> This causes "make foo" to run gcc and create a temporary file
> unconditionally, regardless of what foo is.  In an ideal world:
>
>  - the autodetection would only happen when building targets that
>   care about it

In an ideal world, yes. But I don't see a simple and maintainable way to do it for only those targets.

Show 6 quoted lines

>  - the detection would happen once (creating some file to store the
>   result) and not be repeated with each invocation of "make"
>
>  - (maybe) there would be a way to override the detection with
>   either a "yes" or "no" result, for those who really care to
>   save a little time.

I have done some benchmarking (see below) and considering the results I got, I think implementing any of these two is not worth it.

Show 10 quoted lines

> I was about to say that the GIT_VERSION variable has some of these
> properties, but now that I check, from the point of view of the
> Makefile it doesn't.  ./GIT-VERSION-GEN is just very fast. :)
>
> I wonder if we can make do with a faster check, like
>
>        $(CC) -c -MF /dev/null -MMD -MP git.c --help >/dev/null 2>&1
>
> What do you think?
>

Here are some benchmarks done with 'perf stat'. Before each invocation of perf stat I ran a plain 'make' to make sure that everything was compiled. Note that a fully compiled source tree is the worst case when it comes to the overhead of the auto-detection.

Without patch (with COMPUTE_HEADER_DEPENDENCIES=Yes):
 Performance counter stats for 'make' (10 runs):

         1,566,393 cache-misses             #      1.557 M/sec   ( +-   0.264% )
         6,428,212 cache-references         #      6.391 M/sec   ( +-   0.168% )
        21,245,775 branch-misses            #      4.626 %       ( +-   0.015% )
       459,268,954 branches                 #    456.585 M/sec   ( +-   0.031% )
     2,594,717,999 instructions             #      1.177 IPC     ( +-   0.022% )
     2,205,246,745 cycles                   #   2192.359 M/sec   ( +-   0.136% )
            43,532 page-faults              #      0.043 M/sec   ( +-   0.034% )
               215 CPU-migrations           #      0.000 M/sec   ( +-   0.891% )
               457 context-switches         #      0.000 M/sec   ( +-   0.654% )
       1005.878305 task-clock-msecs         #      1.022 CPUs    ( +-   0.544% )

        0.984526665  seconds time elapsed   ( +-   0.591% )

With patch:
 Performance counter stats for 'make' (10 runs):

         1,796,342 cache-misses             #      1.732 M/sec   ( +-   0.702% )
         6,929,739 cache-references         #      6.682 M/sec   ( +-   0.186% )
        21,582,772 branch-misses            #      4.575 %       ( +-   0.032% )
       471,783,920 branches                 #    454.934 M/sec   ( +-   0.024% )
     2,662,671,428 instructions             #      1.166 IPC     ( +-   0.017% )
     2,282,907,087 cycles                   #   2201.372 M/sec   ( +-   0.162% )
            49,244 page-faults              #      0.047 M/sec   ( +-   0.031% )
               233 CPU-migrations           #      0.000 M/sec   ( +-   0.823% )
               489 context-switches         #      0.000 M/sec   ( +-   0.460% )
       1037.038252 task-clock-msecs         #      1.022 CPUs    ( +-   0.579% )

        1.014409177  seconds time elapsed   ( +-   0.476% )

With patch, but changed to use git.c instead of ++empty.c:
 Performance counter stats for 'make' (10 runs):

         2,125,147 cache-misses             #      1.737 M/sec   ( +-   0.287% )
         9,080,043 cache-references         #      7.423 M/sec   ( +-   0.185% )
        24,573,023 branch-misses            #      4.222 %       ( +-   0.032% )
       582,018,515 branches                 #    475.809 M/sec   ( +-   0.012% )
     3,185,328,930 instructions             #      1.165 IPC     ( +-   0.009% )
     2,734,176,502 cycles                   #   2235.229 M/sec   ( +-   0.122% )
            51,032 page-faults              #      0.042 M/sec   ( +-   0.034% )
               227 CPU-migrations           #      0.000 M/sec   ( +-   0.943% )
               515 context-switches         #      0.000 M/sec   ( +-   0.351% )
       1223.219930 task-clock-msecs         #      1.019 CPUs    ( +-   0.579% )

        1.200869268  seconds time elapsed   ( +-   0.555% )

So, on my machine the auto-detection logic adds a slight overhead (0.03s, 3% in a fully compiled tree). Using git.c is slower than using ++empty.c. IMHO adding any extra complexity to lower the 0.03s is not worth it.

- Fredrik