* [PATCH resend] Makefile: Use computed header dependencies if the compiler supports it
@ 2011-08-14 18:45 Fredrik Kuivinen
2011-08-14 19:00 ` Jonathan Nieder
0 siblings, 1 reply; 6+ messages in thread
From: Fredrik Kuivinen @ 2011-08-14 18:45 UTC (permalink / raw)
To: git; +Cc: Fredrik Kuivinen, Jonathan Nieder
Previously you had to manually define COMPUTE_HEADER_DEPENDENCIES to
enable this feature. It seemed a bit sad that such a useful feature
had to be enabled manually.
Signed-off-by: Fredrik Kuivinen <frekui@gmail.com>
---
This is a resend, it has been rebased on top of master but otherwise
the same patch was sent 2011-06-11. Jonathan Nieder has been added to
the Cc list as he implemented the computed header dependencies
feature.
Makefile | 13 +++++++++----
1 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/Makefile b/Makefile
index 8dd782f..c289074 100644
--- a/Makefile
+++ b/Makefile
@@ -250,10 +250,6 @@ all::
# DEFAULT_EDITOR='$GIT_FALLBACK_EDITOR',
# DEFAULT_EDITOR='"C:\Program Files\Vim\gvim.exe" --nofork'
#
-# Define COMPUTE_HEADER_DEPENDENCIES if your compiler supports the -MMD option
-# and you want to avoid rebuilding objects when an unrelated header file
-# changes.
-#
# Define CHECK_HEADER_DEPENDENCIES to check for problems in the hard-coded
# dependency rules.
#
@@ -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
endif
ifdef COMPUTE_HEADER_DEPENDENCIES
--
1.7.5.3.368.g8b1b7.dirty
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH resend] Makefile: Use computed header dependencies if the compiler supports it
2011-08-14 18:45 [PATCH resend] Makefile: Use computed header dependencies if the compiler supports it Fredrik Kuivinen
@ 2011-08-14 19:00 ` Jonathan Nieder
2011-08-14 19:53 ` Fredrik Kuivinen
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Nieder @ 2011-08-14 19:00 UTC (permalink / raw)
To: Fredrik Kuivinen; +Cc: git
Hi,
Fredrik Kuivinen wrote:
> Previously you had to manually define COMPUTE_HEADER_DEPENDENCIES to
> enable this feature. It seemed a bit sad that such a useful feature
> had to be enabled manually.
Yes! Thanks for this.
I have a few thoughts about the implementation:
> --- 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
- 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 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?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH resend] Makefile: Use computed header dependencies if the compiler supports it
2011-08-14 19:00 ` Jonathan Nieder
@ 2011-08-14 19:53 ` Fredrik Kuivinen
2011-08-14 20:02 ` Jonathan Nieder
0 siblings, 1 reply; 6+ messages in thread
From: Fredrik Kuivinen @ 2011-08-14 19:53 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git
Hi Jonathan,
Thanks for your comments.
On Sun, Aug 14, 2011 at 21:00, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> --- 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.
> - 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.
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH resend] Makefile: Use computed header dependencies if the compiler supports it
2011-08-14 19:53 ` Fredrik Kuivinen
@ 2011-08-14 20:02 ` Jonathan Nieder
2011-08-18 18:34 ` Fredrik Kuivinen
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Nieder @ 2011-08-14 20:02 UTC (permalink / raw)
To: Fredrik Kuivinen; +Cc: git
Fredrik Kuivinen wrote:
> On Sun, Aug 14, 2011 at 21:00, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> 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?
[...]
> Without patch (with COMPUTE_HEADER_DEPENDENCIES=Yes):
The case to compare against is when COMPUTE_HEADER_DEPENDENCIES is not
set, I'd think, since that is the status quo. And I was talking about
commands like "make clean" that do not care about that feature, not
"make all".
[...]
> With patch, but changed to use git.c instead of ++empty.c:
Did you try with "--help"?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH resend] Makefile: Use computed header dependencies if the compiler supports it
2011-08-14 20:02 ` Jonathan Nieder
@ 2011-08-18 18:34 ` Fredrik Kuivinen
2011-08-18 18:41 ` Jonathan Nieder
0 siblings, 1 reply; 6+ messages in thread
From: Fredrik Kuivinen @ 2011-08-18 18:34 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git
On Sun, Aug 14, 2011 at 03:02:55PM -0500, Jonathan Nieder wrote:
> Fredrik Kuivinen wrote:
> > On Sun, Aug 14, 2011 at 21:00, Jonathan Nieder <jrnieder@gmail.com> wrote:
>
> >> 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?
> [...]
> > Without patch (with COMPUTE_HEADER_DEPENDENCIES=Yes):
>
> The case to compare against is when COMPUTE_HEADER_DEPENDENCIES is not
> set, I'd think, since that is the status quo. And I was talking about
> commands like "make clean" that do not care about that feature, not
> "make all".
There is no measurable difference between setting and unsetting
COMPUTE_HEADER_DEPENDENCIES for me (not surprising as nothing is
actually built). "make clean" (in a clean tree) takes more time than
"make" (in a fully built tree), so the relative overhead in the make
clean case is even smaller. The absolute overhead is, of course, the
same.
> [...]
> > With patch, but changed to use git.c instead of ++empty.c:
>
> Did you try with "--help"?
Oh, I missed "--help". But for me gcc always exits with status code 0
when I give it "--help", regardless of what other flags I
provide. Therefore, I don't see how "--help" can be used to test for
support of -MMD.
Here is an updated patch. It avoids the ++empty.c file by giving "-x
c" to the compiler. It also avoids the auto-detection when
COMPUTE_HEADER_DEPENDENCIES is set, so if you want to avoid the
overhead you can set that in you config.mak.
-- 8< --
Subject: [PATCH] Makefile: Use computed header dependencies if the compiler supports it
Previously you had to manually define COMPUTE_HEADER_DEPENDENCIES to
enable this feature. It seemed a bit sad that such a useful feature
had to be enabled manually.
To avoid the small overhead we don't do the auto-detection if
COMPUTE_HEADER_DEPENDENCIES is already set.
Signed-off-by: Fredrik Kuivinen <frekui@gmail.com>
---
Makefile | 13 +++++++++----
1 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/Makefile b/Makefile
index 89cc624..c131439 100644
--- a/Makefile
+++ b/Makefile
@@ -250,10 +250,6 @@ all::
# DEFAULT_EDITOR='$GIT_FALLBACK_EDITOR',
# DEFAULT_EDITOR='"C:\Program Files\Vim\gvim.exe" --nofork'
#
-# Define COMPUTE_HEADER_DEPENDENCIES if your compiler supports the -MMD option
-# and you want to avoid rebuilding objects when an unrelated header file
-# changes.
-#
# Define CHECK_HEADER_DEPENDENCIES to check for problems in the hard-coded
# dependency rules.
#
@@ -1236,6 +1232,15 @@ endif
ifdef CHECK_HEADER_DEPENDENCIES
COMPUTE_HEADER_DEPENDENCIES =
USE_COMPUTED_HEADER_DEPENDENCIES =
+else
+ifndef COMPUTE_HEADER_DEPENDENCIES
+dep_check = $(shell sh -c \
+ '$(CC) -c -MF /dev/null -MMD -MP -x c /dev/null -o /dev/null 2>&1; \
+ echo $$?')
+ifeq ($(dep_check),0)
+COMPUTE_HEADER_DEPENDENCIES=YesPlease
+endif
+endif
endif
ifdef COMPUTE_HEADER_DEPENDENCIES
--
1.7.5.3.368.g8b1b7.dirty
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH resend] Makefile: Use computed header dependencies if the compiler supports it
2011-08-18 18:34 ` Fredrik Kuivinen
@ 2011-08-18 18:41 ` Jonathan Nieder
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Nieder @ 2011-08-18 18:41 UTC (permalink / raw)
To: Fredrik Kuivinen; +Cc: git, Junio C Hamano
Fredrik Kuivinen wrote:
> Oh, I missed "--help". But for me gcc always exits with status code 0
> when I give it "--help", regardless of what other flags I
> provide. Therefore, I don't see how "--help" can be used to test for
> support of -MMD.
Ah, my mistake. Good catch.
> Here is an updated patch. It avoids the ++empty.c file by giving "-x
> c" to the compiler.
Much nicer, thanks!
> It also avoids the auto-detection when
> COMPUTE_HEADER_DEPENDENCIES is set
Unfortunately "ifdef" in Makefiles means "if nonempty", so the
overhead of detection is still there if I want to explicitly disable
COMPUTE_HEADER_DEPENDENCIES. That's okay, since that overhead is
small. So for what it's worth,
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-08-18 18:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-08-14 18:45 [PATCH resend] Makefile: Use computed header dependencies if the compiler supports it Fredrik Kuivinen
2011-08-14 19:00 ` Jonathan Nieder
2011-08-14 19:53 ` Fredrik Kuivinen
2011-08-14 20:02 ` Jonathan Nieder
2011-08-18 18:34 ` Fredrik Kuivinen
2011-08-18 18:41 ` Jonathan Nieder
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).