{"thread":{"id":"28098","subject":"[PATCH resend] Makefile: Use computed header dependencies if the compiler supports it","startedAt":"2011-08-14T18:45:12Z","lastAt":"2011-08-18T18:41:42Z","messageCount":6,"participants":["Fredrik Kuivinen","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"173511","messageId":"1313347512-7815-1-git-send-email-frekui@gmail.com","threadId":"28098","inReplyTo":null,"subject":"[PATCH resend] Makefile: Use computed header dependencies if the compiler supports it","fromName":"Fredrik Kuivinen","fromEmail":"frekui@gmail.com","sentAt":"2011-08-14T18:45:12Z","receivedAt":"2011-08-14T18:45:12Z","isPatch":true,"sender":{"key":"frekui@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13770967?v=4"},"body":"Previously you had to manually define COMPUTE_HEADER_DEPENDENCIES to\nenable this feature. It seemed a bit sad that such a useful feature\nhad to be enabled manually.\n\nSigned-off-by: Fredrik Kuivinen \n---\n\nThis is a resend, it has been rebased on top of master but otherwise\nthe same patch was sent 2011-06-11. Jonathan Nieder has been added to\nthe Cc list as he implemented the computed header dependencies\nfeature.\n\n Makefile | 13 +++++++++----\n 1 files changed, 9 insertions(+), 4 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 8dd782f..c289074 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -250,10 +250,6 @@ all::\n # DEFAULT_EDITOR='$GIT_FALLBACK_EDITOR',\n # DEFAULT_EDITOR='\"C:\\Program Files\\Vim\\gvim.exe\" --nofork'\n #\n-# Define COMPUTE_HEADER_DEPENDENCIES if your compiler supports the -MMD option\n-# and you want to avoid rebuilding objects when an unrelated header file\n-# changes.\n-#\n # Define CHECK_HEADER_DEPENDENCIES to check for problems in the hard-coded\n # dependency rules.\n #\n@@ -1236,6 +1232,15 @@ endif\n ifdef CHECK_HEADER_DEPENDENCIES\n COMPUTE_HEADER_DEPENDENCIES =\n USE_COMPUTED_HEADER_DEPENDENCIES =\n+else\n+dep_check = $(shell sh -c \\\n+\t': > ++empty.c; \\\n+\t$(CC) -c -MF /dev/null -MMD -MP ++empty.c -o /dev/null 2>&1; \\\n+\techo $$?; \\\n+\t$(RM) ++empty.c')\n+ifeq ($(dep_check),0)\n+COMPUTE_HEADER_DEPENDENCIES=YesPlease\n+endif\n endif\n \n ifdef COMPUTE_HEADER_DEPENDENCIES\n-- \n1.7.5.3.368.g8b1b7.dirty\n"},{"id":"173512","messageId":"20110814190050.GA16819@elie.gateway.2wire.net","threadId":"28098","inReplyTo":"1313347512-7815-1-git-send-email-frekui@gmail.com","subject":"Re: [PATCH resend] Makefile: Use computed header dependencies if the compiler supports it","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-08-14T19:00:50Z","receivedAt":"2011-08-14T19:00:50Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nFredrik Kuivinen wrote:\n\n> Previously you had to manually define COMPUTE_HEADER_DEPENDENCIES to\n> enable this feature. It seemed a bit sad that such a useful feature\n> had to be enabled manually.\n\nYes! Thanks for this.\n\nI have a few thoughts about the implementation:\n\n> --- a/Makefile\n> +++ b/Makefile\n[...]\n> @@ -1236,6 +1232,15 @@ endif\n> ifdef CHECK_HEADER_DEPENDENCIES\n> COMPUTE_HEADER_DEPENDENCIES =\n> USE_COMPUTED_HEADER_DEPENDENCIES =\n> +else\n> +dep_check = $(shell sh -c \\\n> +\t': > ++empty.c; \\\n> +\t$(CC) -c -MF /dev/null -MMD -MP ++empty.c -o /dev/null 2>&1; \\\n> +\techo $$?; \\\n> +\t$(RM) ++empty.c')\n> +ifeq ($(dep_check),0)\n> +COMPUTE_HEADER_DEPENDENCIES=YesPlease\n> +endif\n\nThis causes \"make foo\" to run gcc and create a temporary file\nunconditionally, regardless of what foo is. In an ideal world:\n\n - the autodetection would only happen when building targets that\n care about it\n\n - the detection would happen once (creating some file to store the\n result) and not be repeated with each invocation of \"make\"\n\n - (maybe) there would be a way to override the detection with\n either a \"yes\" or \"no\" result, for those who really care to\n save a little time.\n\nI was about to say that the GIT_VERSION variable has some of these\nproperties, but now that I check, from the point of view of the\nMakefile it doesn't. ./GIT-VERSION-GEN is just very fast. :)\n\nI wonder if we can make do with a faster check, like\n\n\t$(CC) -c -MF /dev/null -MMD -MP git.c --help >/dev/null 2>&1\n\nWhat do you think?\n"},{"id":"173514","messageId":"CALx8hKRBjXr44gM1JA+d=RU80pmruPV56s-G3JvViz87eJ=ajQ@mail.gmail.com","threadId":"28098","inReplyTo":"20110814190050.GA16819@elie.gateway.2wire.net","subject":"Re: [PATCH resend] Makefile: Use computed header dependencies if the compiler supports it","fromName":"Fredrik Kuivinen","fromEmail":"frekui@gmail.com","sentAt":"2011-08-14T19:53:24Z","receivedAt":"2011-08-14T19:53:24Z","isPatch":true,"sender":{"key":"frekui@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13770967?v=4"},"body":"Hi Jonathan,\n\nThanks for your comments.\n\nOn Sun, Aug 14, 2011 at 21:00, Jonathan Nieder wrote:\n>> --- a/Makefile\n>> +++ b/Makefile\n> [...]\n>> @@ -1236,6 +1232,15 @@ endif\n>>  ifdef CHECK_HEADER_DEPENDENCIES\n>>  COMPUTE_HEADER_DEPENDENCIES =\n>>  USE_COMPUTED_HEADER_DEPENDENCIES =\n>> +else\n>> +dep_check = $(shell sh -c \\\n>> +     ': > ++empty.c; \\\n>> +     $(CC) -c -MF /dev/null -MMD -MP ++empty.c -o /dev/null 2>&1; \\\n>> +     echo $$?; \\\n>> +     $(RM) ++empty.c')\n>> +ifeq ($(dep_check),0)\n>> +COMPUTE_HEADER_DEPENDENCIES=YesPlease\n>> +endif\n>\n> This causes \"make foo\" to run gcc and create a temporary file\n> unconditionally, regardless of what foo is.  In an ideal world:\n>\n>  - the autodetection would only happen when building targets that\n>   care about it\n\nIn an ideal world, yes. But I don't see a simple and maintainable\nway to do it for only those targets.\n\n>  - the detection would happen once (creating some file to store the\n>   result) and not be repeated with each invocation of \"make\"\n>\n>  - (maybe) there would be a way to override the detection with\n>   either a \"yes\" or \"no\" result, for those who really care to\n>   save a little time.\n\nI have done some benchmarking (see below) and considering the\nresults I got, I think implementing any of these two is not worth it.\n\n> I was about to say that the GIT_VERSION variable has some of these\n> properties, but now that I check, from the point of view of the\n> Makefile it doesn't.  ./GIT-VERSION-GEN is just very fast. :)\n>\n> I wonder if we can make do with a faster check, like\n>\n>        $(CC) -c -MF /dev/null -MMD -MP git.c --help >/dev/null 2>&1\n>\n> What do you think?\n>\n\nHere are some benchmarks done with 'perf stat'. Before each invocation\nof perf stat I ran a plain 'make' to make sure that everything was compiled.\nNote that a fully compiled source tree is the worst case when it comes to\nthe overhead of the auto-detection.\n\nWithout patch (with COMPUTE_HEADER_DEPENDENCIES=Yes):\n Performance counter stats for 'make' (10 runs):\n\n 1,566,393 cache-misses # 1.557 M/sec ( +- 0.264% )\n 6,428,212 cache-references # 6.391 M/sec ( +- 0.168% )\n 21,245,775 branch-misses # 4.626 % ( +- 0.015% )\n 459,268,954 branches # 456.585 M/sec ( +- 0.031% )\n 2,594,717,999 instructions # 1.177 IPC ( +- 0.022% )\n 2,205,246,745 cycles # 2192.359 M/sec ( +- 0.136% )\n 43,532 page-faults # 0.043 M/sec ( +- 0.034% )\n 215 CPU-migrations # 0.000 M/sec ( +- 0.891% )\n 457 context-switches # 0.000 M/sec ( +- 0.654% )\n 1005.878305 task-clock-msecs # 1.022 CPUs ( +- 0.544% )\n\n 0.984526665 seconds time elapsed ( +- 0.591% )\n\nWith patch:\n Performance counter stats for 'make' (10 runs):\n\n 1,796,342 cache-misses # 1.732 M/sec ( +- 0.702% )\n 6,929,739 cache-references # 6.682 M/sec ( +- 0.186% )\n 21,582,772 branch-misses # 4.575 % ( +- 0.032% )\n 471,783,920 branches # 454.934 M/sec ( +- 0.024% )\n 2,662,671,428 instructions # 1.166 IPC ( +- 0.017% )\n 2,282,907,087 cycles # 2201.372 M/sec ( +- 0.162% )\n 49,244 page-faults # 0.047 M/sec ( +- 0.031% )\n 233 CPU-migrations # 0.000 M/sec ( +- 0.823% )\n 489 context-switches # 0.000 M/sec ( +- 0.460% )\n 1037.038252 task-clock-msecs # 1.022 CPUs ( +- 0.579% )\n\n 1.014409177 seconds time elapsed ( +- 0.476% )\n\nWith patch, but changed to use git.c instead of ++empty.c:\n Performance counter stats for 'make' (10 runs):\n\n 2,125,147 cache-misses # 1.737 M/sec ( +- 0.287% )\n 9,080,043 cache-references # 7.423 M/sec ( +- 0.185% )\n 24,573,023 branch-misses # 4.222 % ( +- 0.032% )\n 582,018,515 branches # 475.809 M/sec ( +- 0.012% )\n 3,185,328,930 instructions # 1.165 IPC ( +- 0.009% )\n 2,734,176,502 cycles # 2235.229 M/sec ( +- 0.122% )\n 51,032 page-faults # 0.042 M/sec ( +- 0.034% )\n 227 CPU-migrations # 0.000 M/sec ( +- 0.943% )\n 515 context-switches # 0.000 M/sec ( +- 0.351% )\n 1223.219930 task-clock-msecs # 1.019 CPUs ( +- 0.579% )\n\n 1.200869268 seconds time elapsed ( +- 0.555% )\n\n\nSo, on my machine the auto-detection logic adds a slight overhead\n(0.03s, 3% in a fully compiled tree). Using git.c is slower than using\n++empty.c. IMHO adding any extra complexity to lower the 0.03s\nis not worth it.\n\n- Fredrik\n"},{"id":"173515","messageId":"20110814200255.GC16819@elie.gateway.2wire.net","threadId":"28098","inReplyTo":"CALx8hKRBjXr44gM1JA+d=RU80pmruPV56s-G3JvViz87eJ=ajQ@mail.gmail.com","subject":"Re: [PATCH resend] Makefile: Use computed header dependencies if the compiler supports it","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-08-14T20:02:55Z","receivedAt":"2011-08-14T20:02:55Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Fredrik Kuivinen wrote:\n> On Sun, Aug 14, 2011 at 21:00, Jonathan Nieder wrote:\n\n>> I wonder if we can make do with a faster check, like\n>>\n>>        $(CC) -c -MF /dev/null -MMD -MP git.c --help >/dev/null 2>&1\n>>\n>> What do you think?\n[...]\n> Without patch (with COMPUTE_HEADER_DEPENDENCIES=Yes):\n\nThe case to compare against is when COMPUTE_HEADER_DEPENDENCIES is not\nset, I'd think, since that is the status quo. And I was talking about\ncommands like \"make clean\" that do not care about that feature, not\n\"make all\".\n\n[...]\n> With patch, but changed to use git.c instead of ++empty.c:\n\nDid you try with \"--help\"?\n"},{"id":"173767","messageId":"20110818183439.GA21560@fredrik-Q430-Q530","threadId":"28098","inReplyTo":"20110814200255.GC16819@elie.gateway.2wire.net","subject":"Re: [PATCH resend] Makefile: Use computed header dependencies if the compiler supports it","fromName":"Fredrik Kuivinen","fromEmail":"frekui@gmail.com","sentAt":"2011-08-18T18:34:39Z","receivedAt":"2011-08-18T18:34:39Z","isPatch":true,"sender":{"key":"frekui@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13770967?v=4"},"body":"On Sun, Aug 14, 2011 at 03:02:55PM -0500, Jonathan Nieder wrote:\n> Fredrik Kuivinen wrote:\n> > On Sun, Aug 14, 2011 at 21:00, Jonathan Nieder wrote:\n> \n> >> I wonder if we can make do with a faster check, like\n> >>\n> >>        $(CC) -c -MF /dev/null -MMD -MP git.c --help >/dev/null 2>&1\n> >>\n> >> What do you think?\n> [...]\n> > Without patch (with COMPUTE_HEADER_DEPENDENCIES=Yes):\n> \n> The case to compare against is when COMPUTE_HEADER_DEPENDENCIES is not\n> set, I'd think, since that is the status quo. And I was talking about\n> commands like \"make clean\" that do not care about that feature, not\n> \"make all\".\n\nThere is no measurable difference between setting and unsetting\nCOMPUTE_HEADER_DEPENDENCIES for me (not surprising as nothing is\nactually built). \"make clean\" (in a clean tree) takes more time than\n\"make\" (in a fully built tree), so the relative overhead in the make\nclean case is even smaller. The absolute overhead is, of course, the\nsame.\n\n> [...]\n> > With patch, but changed to use git.c instead of ++empty.c:\n> \n> Did you try with \"--help\"?\n\nOh, I missed \"--help\". But for me gcc always exits with status code 0\nwhen I give it \"--help\", regardless of what other flags I\nprovide. Therefore, I don't see how \"--help\" can be used to test for\nsupport of -MMD.\n\nHere is an updated patch. It avoids the ++empty.c file by giving \"-x\nc\" to the compiler. It also avoids the auto-detection when\nCOMPUTE_HEADER_DEPENDENCIES is set, so if you want to avoid the\noverhead you can set that in you config.mak.\n\n\n-- 8< --\n\nSubject: [PATCH] Makefile: Use computed header dependencies if the compiler supports it\n\nPreviously you had to manually define COMPUTE_HEADER_DEPENDENCIES to\nenable this feature. It seemed a bit sad that such a useful feature\nhad to be enabled manually.\n\nTo avoid the small overhead we don't do the auto-detection if\nCOMPUTE_HEADER_DEPENDENCIES is already set.\n\nSigned-off-by: Fredrik Kuivinen \n---\n Makefile | 13 +++++++++----\n 1 files changed, 9 insertions(+), 4 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 89cc624..c131439 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -250,10 +250,6 @@ all::\n # DEFAULT_EDITOR='$GIT_FALLBACK_EDITOR',\n # DEFAULT_EDITOR='\"C:\\Program Files\\Vim\\gvim.exe\" --nofork'\n #\n-# Define COMPUTE_HEADER_DEPENDENCIES if your compiler supports the -MMD option\n-# and you want to avoid rebuilding objects when an unrelated header file\n-# changes.\n-#\n # Define CHECK_HEADER_DEPENDENCIES to check for problems in the hard-coded\n # dependency rules.\n #\n@@ -1236,6 +1232,15 @@ endif\n ifdef CHECK_HEADER_DEPENDENCIES\n COMPUTE_HEADER_DEPENDENCIES =\n USE_COMPUTED_HEADER_DEPENDENCIES =\n+else\n+ifndef COMPUTE_HEADER_DEPENDENCIES\n+dep_check = $(shell sh -c \\\n+\t'$(CC) -c -MF /dev/null -MMD -MP -x c /dev/null -o /dev/null 2>&1; \\\n+\techo $$?')\n+ifeq ($(dep_check),0)\n+COMPUTE_HEADER_DEPENDENCIES=YesPlease\n+endif\n+endif\n endif\n \n ifdef COMPUTE_HEADER_DEPENDENCIES\n-- \n1.7.5.3.368.g8b1b7.dirty\n"},{"id":"173768","messageId":"20110818184142.GF30436@elie.gateway.2wire.net","threadId":"28098","inReplyTo":"20110818183439.GA21560@fredrik-Q430-Q530","subject":"Re: [PATCH resend] Makefile: Use computed header dependencies if the compiler supports it","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-08-18T18:41:42Z","receivedAt":"2011-08-18T18:41:42Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Fredrik Kuivinen wrote:\n\n> Oh, I missed \"--help\". But for me gcc always exits with status code 0\n> when I give it \"--help\", regardless of what other flags I\n> provide. Therefore, I don't see how \"--help\" can be used to test for\n> support of -MMD.\n\nAh, my mistake. Good catch.\n\n> Here is an updated patch. It avoids the ++empty.c file by giving \"-x\n> c\" to the compiler.\n\nMuch nicer, thanks!\n\n> It also avoids the auto-detection when\n> COMPUTE_HEADER_DEPENDENCIES is set\n\nUnfortunately \"ifdef\" in Makefiles means \"if nonempty\", so the\noverhead of detection is still there if I want to explicitly disable\nCOMPUTE_HEADER_DEPENDENCIES. That's okay, since that overhead is\nsmall. So for what it's worth,\n\nAcked-by: Jonathan Nieder \n"}]}