* [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).