| Message ID | 20170926175508.29950-2-dianders@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Douglas, 2017-09-27 2:55 GMT+09:00 Douglas Anderson <dianders@chromium.org>: > While timing a "no-op" build of the kernel (incrementally building the > kernel even though nothing changed) in the Chrome OS build system I > found that it was much slower than I expected. > > Digging into things a bit, I found that quite a bit of the time was > spent invoking the C compiler even though we weren't actually building > anything. Currently in the Chrome OS build system the C compiler is > called through a number of wrappers (one of which is written in > python!) and can take upwards of 100 ms to invoke even if we're not > doing anything difficult, so these invocations of the compiler were > taking a lot of time. Worse the invocations couldn't seem to take > advantage of the multiple cores on my system. > > Certainly it seems like we could make the compiler invocations in the > Chrome OS build system faster, but only to a point. Inherently > invoking a program as big as a C compiler is a fairly heavy > operation. Thus even if we can speed the compiler calls it made sense > to track down what was happening. > > It turned out that all the compiler invocations were coming from > usages like this in the kernel's Makefile: > > KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks,) > > Due to the way cc-option and similar statements work the above > contains an implicit call to the C compiler. ...and due to the fact > that we're storing the result in KBUILD_CFLAGS, a simply expanded > variable, the call will happen every time the Makefile is parsed, even > if there are no users of KBUILD_CFLAGS. > > Rather than redoing this computation every time, it makes a lot of > sense to cache the result of all of the Makefile's compiler calls just > like we do when we compile a ".c" file to a ".o" file. Conceptually > this is quite a simple idea. ...and since the calls to invoke the > compiler and similar tools are centrally located in the Kbuild.include > file this doesn't even need to be super invasive. > > Implementing the cache in a simple-to-use and efficient way is not > quite as simple as it first sounds, though. To get maximum speed we > really want the cache in a format that make can natively understand > and make doesn't really have an ability to load/parse files. ...but > make _can_ import other Makefiles, so the solution is to store the > cache in Makefile format. This requires coming up with a valid/unique > Makefile variable name for each value to be cached, but that's > solvable with some cleverness. > > After this change, we'll automatically create a "Makefile-cache.o" > file that will contain our cached variables. We'll load this on each > invocation of make and will avoid recomputing anything that's already > in our cache. The cache is stored in a format that it shouldn't need > any invalidation since anything that might change should affect the > "key" and any old cached value won't be used. The cache will be > cleaned by virtue of the ".o" suffix by a "make clean". > > Signed-off-by: Douglas Anderson <dianders@chromium.org> Thanks for the patches. The idea is interesting. I am not a Chrome developer, but cc-option could be improved somehow. I examined two approaches to mitigate the pain. [1] Skip cc-option completely when we run non-build targets such as "make help", "make clean", etc. [2] Cache the result of cc-option like this patch I wrote some patches for [1] https://patchwork.kernel.org/patch/9983825/ https://patchwork.kernel.org/patch/9983829/ https://patchwork.kernel.org/patch/9983833/ https://patchwork.kernel.org/patch/9983827/ Comments are welcome. :) [1] does not solve the slowness in the incremental build. Instead, it makes non-build targets faster from a clean source tree because cc-option is zero cost. [2] solves the issues in the incremental build. One funny thing is, it computes cc-option and store the cache file even for "make clean", where we know the cache file will be immediately deleted. We can apply [1] or [2], or maybe both of them because [1] and [2] are trying to solve the different issues. The cache approach seemed clever, but I do not like the implementation. The code is even more unreadable with lots of escape sequence. Here is my suggestion for simpler implementation (including bike-shed) Implement a new function "shell-cache". It works like $(shell ...) The difference is $(call shell-cached,...) returns the cached result, or run the command if not cached. Also, add try-run-cached. This is a cached variant of try-run. I just played with it, and seems working. (I did not have spend much time for testing, more wider test is needed.) The code is like something like this: make-cache := $(if $(KBUILD_EXTMOD),$(KBUILD_EXTMOD)/,$(if $(obj),$(obj)/)).cache.mk -include $(make-cache) __sanitize-opt = $(subst $$,_,$(subst $(right_paren),_,$(subst $(left_paren),_,$(subst \,_,$(subst =,_,$(subst $(space),_,$(subst $(comma),_,$(1)))))))) define __run-and-store ifeq ($(origin $(1)),undefined) $$(eval $(1) := $$(shell $$2)) $$(shell echo '$(1) := $$($(1))' >> $(make-cache)) endif endef # $(call,shell-cached,my_command) # This works like $(shell my_command), but the result is cached __shell-cached = $(eval $(call __run-and-store,$1))$($1) shell-cached = $(call __shell-cached,__cached_$(call __sanitize-opt,$(1)),$1) ... __try-run = set -e; \ TMP="$(TMPOUT).$$$$.tmp"; \ TMPO="$(TMPOUT).$$$$.o"; \ if ($(1)) >/dev/null 2>&1; \ then echo "$(2)"; \ else echo "$(3)"; \ fi; \ rm -f "$$TMP" "$$TMPO" # try-run # Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise) # Exit code chooses option. "$$TMP" serves as a temporary file and is # automatically cleaned up. try-run = $(shell $(__try-run)) # try-run-cached # This works like try-run, but the result is cached. try-run-cached = $(call shell-cached,$(__try-run)) Then, you can replace $(shell $(CONFIG_SHELL) $(srctree)/scripts/gcc-goto.sh $(CC) $(KBUILD_CFLAGS) with $(call shell-cached,$(CONFIG_SHELL) $(srctree)/scripts/gcc-goto.sh $(CC) $(KBUILD_CFLAGS) replace __cc-option = $(call try-run,\ $(1) -Werror $(2) $(3) -c -x c /dev/null -o "$$TMP",$(3),$(4)) with __cc-option = $(call try-run-cached,\ $(1) -Werror $(2) $(3) -c -x c /dev/null -o "$$TMP",$(3),$(4)) What do you think? A little more comments below. > +# Tools for caching Makefile variables that are "expensive" to compute. > +# > +# Here we want to help deal with variables that take a long time to compute > +# by making it easy to store these variables in a cache. > +# > +# The canonical example here is testing for compiler flags. On a simple system > +# each call to the compiler takes 10 ms, but on a system with a compiler that's > +# called through various wrappers it can take upwards of 100 ms. If we have > +# 100 calls to the compiler this can take 1 second (on a simple system) or 10 > +# seconds (on a complicated system). > +# > +# The "cache" will be in Makefile syntax and can be directly included. > +# Any time we try to reference a variable that's not in the cache we'll > +# calculate it and store it in the cache for next time. > + > +# Include values from last time > +-include Makefile-cache.o Kbuild.include is included, so is Makefile-cache.o every time the build system descend into sub-directories. It is not efficient to include cached data unneeded in sub-directories. I prefixed $(obj)/ Another problem is when building external modules. Makefile-cache.o is always created/updated in the kernel build tree. When we build external modules, the kernel source may be located under /usr/src/ , which is generally read-only for normal users. So, I prefixed $(KBUILD_EXTMOD) to create the cache file in the module tree if KBUILD_EXTMOD is defined. I do not like the suffix .o I prefer file name to be something else that starts with . to hide it. > +# Usage: $(call cached-val,variable_name,escaped_expensive_operation) > +# Example: $(call cached-val,md5_val,$$(shell md5sum /usr/bin/gcc) > +# > +# If the variable is already defined we'll just use it. If it's not, it will > +# be calculated and stored in a persistent (disk-based) cache for the next > +# invocation of Make. The call will evaluate to the value of the variable. > +# > +# This is a bit of voodoo, but basic explanation is that if the variable > +# was undefined then we'll evaluate the expensive operation and store it into > +# the variable. We'll then store that value in the cache and finally output > +# the value. > +define __set-and-store > +ifeq ($(origin $(1)),undefined) > + $$(eval $(1) := $$(2)) > + $$(shell echo '$(1) := $$($(1))' >> Makefile-cache.o) > +endif > +endef > +cached-val = $(eval $(call __set-and-store,__cached_$(1)))$(__cached_$(1)) This seems working, but I do not understand this trick. __set-and-store takes two arguments, but here, it is called with one argument __cached_$(1) Probably, $$(try-run, ...) will be passed as $2, but I do not know why this works. > +# Usage: $(call __sanitize-opt,Hello=Hola$(comma)Goodbye Adios) > +# > +# Convert all '$', ')', '(', '\', '=', ' ', ',' to '_' > +__sanitize-opt = $(subst $$,_,$(subst $(right_paren),_,$(subst $(left_paren),_,$(subst \,_,$(subst =,_,$(subst $(space),_,$(subst $(comma),_,$(1)))))))) > + > +# Usage = $(call __comma,something_with_comma) > +# > +# Convert ',' to '$(comma)' which can help it getting interpreted by eval. > +__comma = $(subst $(comma),$$(comma),$(1)) > + > # cc-cross-prefix > # Usage: CROSS_COMPILE := $(call cc-cross-prefix, m68k-linux-gnu- m68k-linux-) > # Return first prefix where a prefix$(CC) is found in PATH. > @@ -99,19 +148,34 @@ try-run = $(shell set -e; \ > # as-option > # Usage: cflags-y += $(call as-option,-Wa$(comma)-isa=foo,) > > -as-option = $(call try-run,\ > - $(CC) $(KBUILD_CFLAGS) $(1) -c -x assembler /dev/null -o "$$TMP",$(1),$(2)) > +as-option = \ > + $(call cached-val,$(call __sanitize-opt,\ > + as_opt_$(CC)_$(KBUILD_CFLAGS)_$(1)_$(2)),\ > + $$(call try-run,\ > + $(CC) $(call __comma,$(KBUILD_CFLAGS)) $(call __comma,$(1)) \ > + -c -x assembler /dev/null \ > + -o "$$$$TMP",$(call __comma,$(1)),$(call __comma,$(2)))) > > # as-instr > # Usage: cflags-y += $(call as-instr,instr,option1,option2) > > -as-instr = $(call try-run,\ > - printf "%b\n" "$(1)" | $(CC) $(KBUILD_AFLAGS) -c -x assembler -o "$$TMP" -,$(2),$(3)) > +as-instr = \ > + $(call cached-val,$(call __sanitize-opt,\ > + as_instr_$(CC)_$(KBUILD_AFLAGS)_$(1)_$(2)_$(3)),\ > + $$(call try-run,\ > + printf "%b\n" "$(call __comma,$(1))" | \ > + $(CC) $(call __comma,$(KBUILD_AFLAGS)) -c -x assembler \ > + -o "$$$$TMP" -,$(call __comma,$(2)),$(call __comma,$(3)))) > > # __cc-option > # Usage: MY_CFLAGS += $(call __cc-option,$(CC),$(MY_CFLAGS),-march=winchip-c6,-march=i586) > -__cc-option = $(call try-run,\ > - $(1) -Werror $(2) $(3) -c -x c /dev/null -o "$$TMP",$(3),$(4)) > +__cc-option = \ > + $(call cached-val,$(call __sanitize-opt,__cc_opt_$(1)_$(2)_$(3)_$(4)),\ > + $$(call try-run,\ > + $(call __comma,$(1)) -Werror \ > + $(call __comma,$(2)) \ > + $(call __comma,$(3)) -c -x c /dev/null \ > + -o "$$$$TMP",$(call __comma,$(strip $(3))),$(call __comma,$(strip $(4))))) I did not follow how this was working here...
Hi, On Tue, Oct 3, 2017 at 9:05 PM, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > Thanks for the patches. The idea is interesting. > > I am not a Chrome developer, but cc-option could be improved somehow. > > > I examined two approaches to mitigate the pain. > > [1] Skip cc-option completely when we run non-build targets > such as "make help", "make clean", etc. > > [2] Cache the result of cc-option like this patch > > > I wrote some patches for [1] > https://patchwork.kernel.org/patch/9983825/ > https://patchwork.kernel.org/patch/9983829/ > https://patchwork.kernel.org/patch/9983833/ > https://patchwork.kernel.org/patch/9983827/ > > Comments are welcome. :) OK, I'll take a look at them. I'm not massively familiar with the kernel Makefiles, but hopefully I can figure some of it out. :-P > [1] does not solve the slowness in the incremental build. > Instead, it makes non-build targets faster > from a clean source tree because cc-option is zero cost. > > > [2] solves the issues in the incremental build. > One funny thing is, it computes cc-option and store the cache file > even for "make clean", where we know the cache file will be > immediately deleted. > > > We can apply [1] or [2], or maybe both of them > because [1] and [2] are trying to solve the different issues. Yeah, I'm much more interested in [2] than [1]. I run incremental builds almost constantly and hate the slowness. :( I can certainly appreciate that the inefficient compiler setup in Chrome OS isn't your problem and that an extra .5 seconds for an incremental build for most people isn't that huge, though. ...but I'l probably move forward with [2] since it still helps me a lot and still should help others. Solving [1] seems worthwhile too, though... > The cache approach seemed clever, but I do not like the implementation. > The code is even more unreadable with lots of escape sequence. > > > Here is my suggestion for simpler implementation (including bike-shed) > > > Implement a new function "shell-cache". It works like $(shell ...) > > The difference is > $(call shell-cached,...) returns the cached result, or run the command > if not cached. > > > > Also, add try-run-cached. This is a cached variant of try-run. > > > I just played with it, and seems working. > (I did not have spend much time for testing, more wider test is needed.) > > > The code is like something like this: > > > > make-cache := $(if $(KBUILD_EXTMOD),$(KBUILD_EXTMOD)/,$(if > $(obj),$(obj)/)).cache.mk > > -include $(make-cache) Thanks, this is much better! :) > define __run-and-store > ifeq ($(origin $(1)),undefined) > $$(eval $(1) := $$(shell $$2)) > $$(shell echo '$(1) := $$($(1))' >> $(make-cache)) > endif > endef I like your idea. Essentially you're saying that we can just defer the shell command, which was the really essential part that needed to be deferred. Nice! It seems much nicer / cleaner. Still a tiny bit of voodoo, but with all of your improvements the voodoo is at least contained to a very small part of the file. OK, things seem pretty nice with this and I'll submit out a new patch. I'm a bit conflicted about whether I should put myself as authorship or you, since mostly the patch is your code now. ;-) For now I'll leave my authorship but feel free to claim it and change me to just "Suggested-by". :-) NOTE: one thing I noticed about your example code is that you weren't always consistent about $(1) vs. $1. I've changed things to be consistent at the expense of extra parenthesis. If you hate it, let me know. > # $(call,shell-cached,my_command) > # This works like $(shell my_command), but the result is cached > __shell-cached = $(eval $(call __run-and-store,$1))$($1) > shell-cached = $(call __shell-cached,__cached_$(call __sanitize-opt,$(1)),$1) > > ... > > > __try-run = set -e; \ > TMP="$(TMPOUT).$$$$.tmp"; \ > TMPO="$(TMPOUT).$$$$.o"; \ > if ($(1)) >/dev/null 2>&1; \ > then echo "$(2)"; \ > else echo "$(3)"; \ > fi; \ > rm -f "$$TMP" "$$TMPO" > > # try-run > # Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise) > # Exit code chooses option. "$$TMP" serves as a temporary file and is > # automatically cleaned up. > try-run = $(shell $(__try-run)) > > # try-run-cached > # This works like try-run, but the result is cached. > try-run-cached = $(call shell-cached,$(__try-run)) > > > > > Then, you can replace > > $(shell $(CONFIG_SHELL) $(srctree)/scripts/gcc-goto.sh $(CC) $(KBUILD_CFLAGS) > > with > > $(call shell-cached,$(CONFIG_SHELL) $(srctree)/scripts/gcc-goto.sh > $(CC) $(KBUILD_CFLAGS) > > > > > replace > > __cc-option = $(call try-run,\ > $(1) -Werror $(2) $(3) -c -x c /dev/null -o "$$TMP",$(3),$(4)) > > with > > __cc-option = $(call try-run-cached,\ > $(1) -Werror $(2) $(3) -c -x c /dev/null -o "$$TMP",$(3),$(4)) > > > > > What do you think? Yes, much better! Due to where you've placed this I now seem to get by without my $(call __comma,...) calls and also the "$$$$" conversion. Yay! ...interestingly I need to add ":" to the "__sanitize-opt" now, though. That's for the arm64 line: brokengasinst := $(call as-instr,1:\n.inst 0\n.rept . - 1b\n\nnop\n.endr\n,,-DCONFIG_BROKEN_GAS_INST=1) > A little more comments below. > > > > >> +# Tools for caching Makefile variables that are "expensive" to compute. >> +# >> +# Here we want to help deal with variables that take a long time to compute >> +# by making it easy to store these variables in a cache. >> +# >> +# The canonical example here is testing for compiler flags. On a simple system >> +# each call to the compiler takes 10 ms, but on a system with a compiler that's >> +# called through various wrappers it can take upwards of 100 ms. If we have >> +# 100 calls to the compiler this can take 1 second (on a simple system) or 10 >> +# seconds (on a complicated system). >> +# >> +# The "cache" will be in Makefile syntax and can be directly included. >> +# Any time we try to reference a variable that's not in the cache we'll >> +# calculate it and store it in the cache for next time. >> + >> +# Include values from last time >> +-include Makefile-cache.o > > > Kbuild.include is included, so is Makefile-cache.o > every time the build system descend into sub-directories. > > It is not efficient to include cached data unneeded in sub-directories. > I prefixed $(obj)/ > > Another problem is when building external modules. > Makefile-cache.o is always created/updated in the kernel build tree. > > When we build external modules, the kernel source may be located under > /usr/src/ , which is generally read-only for normal users. > So, I prefixed $(KBUILD_EXTMOD) to create the cache file > in the module tree if KBUILD_EXTMOD is defined. > > I do not like the suffix .o > I prefer file name to be something else > that starts with . to hide it. Thanks for all the improvements! >> +# Usage: $(call cached-val,variable_name,escaped_expensive_operation) >> +# Example: $(call cached-val,md5_val,$$(shell md5sum /usr/bin/gcc) >> +# >> +# If the variable is already defined we'll just use it. If it's not, it will >> +# be calculated and stored in a persistent (disk-based) cache for the next >> +# invocation of Make. The call will evaluate to the value of the variable. >> +# >> +# This is a bit of voodoo, but basic explanation is that if the variable >> +# was undefined then we'll evaluate the expensive operation and store it into >> +# the variable. We'll then store that value in the cache and finally output >> +# the value. >> +define __set-and-store >> +ifeq ($(origin $(1)),undefined) >> + $$(eval $(1) := $$(2)) >> + $$(shell echo '$(1) := $$($(1))' >> Makefile-cache.o) >> +endif >> +endef >> +cached-val = $(eval $(call __set-and-store,__cached_$(1)))$(__cached_$(1)) > > > This seems working, but I do not understand this trick. > > > __set-and-store takes two arguments, > but here, it is called with one argument __cached_$(1) > > Probably, $$(try-run, ...) will be passed as $2, > but I do not know why this works. Whew! It's not just me that's confused by all this... ;-) It looks like you continued to use this in your suggestion, so I guess you thought it was useful, at least... Yeah, it's a bit of magic. The goal was to have one less set of evaluating and passing going around. So really '__set-and-store' takes _1_ argument. It outputs a string where it uses this argument. Also part of the output string is a reference to $(2). This will refer to the caller's second variable. === Maybe a "simpler" example? define example $$(eval $(1) := $$(2)) endef ex_usage = $(eval $(call example,$(1)))$($(1)) .PHONY: ex ex: @echo $(call ex_usage,myvar,myval) @echo $(myvar) If you do "make ex" you'll see "myval" printed twice. -- Walking through that, let's first remove the "call" which we can do in this case pretty easily because there are no "ifdefs", unlike the real code. Since the first argument to "example" is $(1), the above is the same as: ex_usage = $(eval $$(eval $(1) := $$(2)))$($(1)) ...now we need to resolve that recursively where $(1) is 'myvar' and $(2) is 'myval'. So we get: 1. $(eval $$(eval $(1) := $$(2)))$($(1)) 2. $(eval $$(eval myvar := $$(2)))$(myvar) 3. $(eval myvar := $(2))$(myvar) 4. $(eval myvar := myval)$(myvar) -- An alternate version of my "simpler" example that doesn't use this magic: define example $$(eval $(1) := $(2)) endef ex_usage = $(eval $(call example,$(1),$(2)))$($(1)) .PHONY: ex ex: @echo $(call ex_usage,myvar,myval) @echo $(myvar) ...that works OK for the simple example case but you get into weird esoteric issues because of the extra eval. You'll find that the two definitions behave differently for: @echo $(call ex_usage,myvar,my$$$$(cat /proc/cmdline)val) ...in that example the "intention" is that bash should be passed the string: echo my$(cat /proc/cmdline)val, so it escapes the "$" once to "defer" it and twice because it wants the "$" to go through to bash. -- ...and yes, I already punched myself in the face for the complexity of all of the above. ;-) It's certainly possible that there's some way to get rid of an eval in the above. If you have a suggestion that would reduce the number needed that'd be interesting. >> +# Usage: $(call __sanitize-opt,Hello=Hola$(comma)Goodbye Adios) >> +# >> +# Convert all '$', ')', '(', '\', '=', ' ', ',' to '_' >> +__sanitize-opt = $(subst $$,_,$(subst $(right_paren),_,$(subst $(left_paren),_,$(subst \,_,$(subst =,_,$(subst $(space),_,$(subst $(comma),_,$(1)))))))) >> + >> +# Usage = $(call __comma,something_with_comma) >> +# >> +# Convert ',' to '$(comma)' which can help it getting interpreted by eval. >> +__comma = $(subst $(comma),$$(comma),$(1)) >> + >> # cc-cross-prefix >> # Usage: CROSS_COMPILE := $(call cc-cross-prefix, m68k-linux-gnu- m68k-linux-) >> # Return first prefix where a prefix$(CC) is found in PATH. >> @@ -99,19 +148,34 @@ try-run = $(shell set -e; \ >> # as-option >> # Usage: cflags-y += $(call as-option,-Wa$(comma)-isa=foo,) >> >> -as-option = $(call try-run,\ >> - $(CC) $(KBUILD_CFLAGS) $(1) -c -x assembler /dev/null -o "$$TMP",$(1),$(2)) >> +as-option = \ >> + $(call cached-val,$(call __sanitize-opt,\ >> + as_opt_$(CC)_$(KBUILD_CFLAGS)_$(1)_$(2)),\ >> + $$(call try-run,\ >> + $(CC) $(call __comma,$(KBUILD_CFLAGS)) $(call __comma,$(1)) \ >> + -c -x assembler /dev/null \ >> + -o "$$$$TMP",$(call __comma,$(1)),$(call __comma,$(2)))) >> >> # as-instr >> # Usage: cflags-y += $(call as-instr,instr,option1,option2) >> >> -as-instr = $(call try-run,\ >> - printf "%b\n" "$(1)" | $(CC) $(KBUILD_AFLAGS) -c -x assembler -o "$$TMP" -,$(2),$(3)) >> +as-instr = \ >> + $(call cached-val,$(call __sanitize-opt,\ >> + as_instr_$(CC)_$(KBUILD_AFLAGS)_$(1)_$(2)_$(3)),\ >> + $$(call try-run,\ >> + printf "%b\n" "$(call __comma,$(1))" | \ >> + $(CC) $(call __comma,$(KBUILD_AFLAGS)) -c -x assembler \ >> + -o "$$$$TMP" -,$(call __comma,$(2)),$(call __comma,$(3)))) >> >> # __cc-option >> # Usage: MY_CFLAGS += $(call __cc-option,$(CC),$(MY_CFLAGS),-march=winchip-c6,-march=i586) >> -__cc-option = $(call try-run,\ >> - $(1) -Werror $(2) $(3) -c -x c /dev/null -o "$$TMP",$(3),$(4)) >> +__cc-option = \ >> + $(call cached-val,$(call __sanitize-opt,__cc_opt_$(1)_$(2)_$(3)_$(4)),\ >> + $$(call try-run,\ >> + $(call __comma,$(1)) -Werror \ >> + $(call __comma,$(2)) \ >> + $(call __comma,$(3)) -c -x c /dev/null \ >> + -o "$$$$TMP",$(call __comma,$(strip $(3))),$(call __comma,$(strip $(4))))) > > > I did not follow how this was working here... Hoo boy. Let's see... Which part were you curious about? I can try to explain the bits and you can tell me if I got them all. Note that many of these aren't super important anymore since they're not needed with your improvements: * $(call __sanitize-opt,__cc_opt_$(1)_$(2)_$(3)_$(4)): Tries to make a variable name that will be different if you have any different parameters. * $$(call try-run The "$$" around this call is because it's deferred and a general concept for "cached-val". If (and only if) we decide that we need to recompute this value then we'll run this through an "eval". By having the $$ then this action won't be taken unless the string is eval-ed. Note that plenty of stuff _will_ still happen right away, though. All of the $(call __comma,$(1)) are _not_ deferred. That's fine since that doesn't lead to a fork-exec and thus isn't "expensive". * $(call __comma,$(1)) I'll admit that I didn't dig all the way into this. I found at least one case where "make" was incorrectly mixing up arguments due to the "eval" and the "__comma" fixed it. I decided it was important to do this for all arguments. Possibly other things might need to be escaped too, but I didn't find any cases that needed it... ...but no longer needed with your improvements... * $$$$TMP An extra level of escaping is needed since it goes through an extra "eval". This means $$ => $$$$ * $(strip $(3)) This was important because otherwise the cache otherwise wasn't completely "stable". If you ran "make" the first time you'd get a cache, then running "make" the 2nd time would give you extra entries in the cache. After the 3rd time you were OK, but it seemed nice to make it stable faster. To understand this, first understand that some people call __cc_option with a space after the ','. Make treats this as important, thus these two are different: val := $(call cc-option, -opt1, -opt2) val := $(call cc-option,-opt1,-opt2) In one case val would be " -opt1" or " -opt2". In the other case "-opt1" or "-opt2". However, when we cache things the space suddenly becomes irrelevant. That's because we've end up with something like: __cached_result := -opt1 ...and even though there are two spaces after the ":=" make throws that away. The result of all of that is that we'd get slightly different numbers of spaces after the cache was populated. That could probably be ignored except that some flags are additive and previous flags are passed as arguments to later calls to __cc-option. That means that previous flags are passed to __sanitize-opt. ...and in __sanitize-opt spaces are significant. ...so you'd come up with a slightly different variable name on the first run and the subsequent runs... Probably waaaay more than you expected for a simple $(strip) call, huh? In any case this doesn't seem to matter anymore with your improvements (I didn't dig into why), though I have noticed that the cache does still return slightly different spacing for some of the results... --- OK, v2 coming right up! Thanks again for all your help on this! -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Douglas, 2017-10-05 7:38 GMT+09:00 Doug Anderson <dianders@chromium.org>: > Hi, > > On Tue, Oct 3, 2017 at 9:05 PM, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: >> Thanks for the patches. The idea is interesting. >> >> I am not a Chrome developer, but cc-option could be improved somehow. >> >> >> I examined two approaches to mitigate the pain. >> >> [1] Skip cc-option completely when we run non-build targets >> such as "make help", "make clean", etc. >> >> [2] Cache the result of cc-option like this patch >> >> >> I wrote some patches for [1] >> https://patchwork.kernel.org/patch/9983825/ >> https://patchwork.kernel.org/patch/9983829/ >> https://patchwork.kernel.org/patch/9983833/ >> https://patchwork.kernel.org/patch/9983827/ >> >> Comments are welcome. :) > > OK, I'll take a look at them. I'm not massively familiar with the > kernel Makefiles, but hopefully I can figure some of it out. :-P > > >> [1] does not solve the slowness in the incremental build. >> Instead, it makes non-build targets faster >> from a clean source tree because cc-option is zero cost. >> >> >> [2] solves the issues in the incremental build. >> One funny thing is, it computes cc-option and store the cache file >> even for "make clean", where we know the cache file will be >> immediately deleted. >> >> >> We can apply [1] or [2], or maybe both of them >> because [1] and [2] are trying to solve the different issues. > > Yeah, I'm much more interested in [2] than [1]. I run incremental > builds almost constantly and hate the slowness. :( I can certainly > appreciate that the inefficient compiler setup in Chrome OS isn't your > problem and that an extra .5 seconds for an incremental build for most > people isn't that huge, though. ...but I'l probably move forward with > [2] since it still helps me a lot and still should help others. > Solving [1] seems worthwhile too, though... I agree. [2] is more helpful because developers spend most of time for the incremental build. [1] can improve it even more, but it just addresses some minor issues. > >> The cache approach seemed clever, but I do not like the implementation. >> The code is even more unreadable with lots of escape sequence. >> >> >> Here is my suggestion for simpler implementation (including bike-shed) >> >> >> Implement a new function "shell-cache". It works like $(shell ...) >> >> The difference is >> $(call shell-cached,...) returns the cached result, or run the command >> if not cached. >> >> >> >> Also, add try-run-cached. This is a cached variant of try-run. >> >> >> I just played with it, and seems working. >> (I did not have spend much time for testing, more wider test is needed.) >> >> >> The code is like something like this: >> >> >> >> make-cache := $(if $(KBUILD_EXTMOD),$(KBUILD_EXTMOD)/,$(if >> $(obj),$(obj)/)).cache.mk >> >> -include $(make-cache) > > Thanks, this is much better! :) > > >> define __run-and-store >> ifeq ($(origin $(1)),undefined) >> $$(eval $(1) := $$(shell $$2)) >> $$(shell echo '$(1) := $$($(1))' >> $(make-cache)) >> endif >> endef > > I like your idea. Essentially you're saying that we can just defer > the shell command, which was the really essential part that needed to > be deferred. Nice! It seems much nicer / cleaner. Still a tiny bit > of voodoo, but with all of your improvements the voodoo is at least > contained to a very small part of the file. > > OK, things seem pretty nice with this and I'll submit out a new patch. > I'm a bit conflicted about whether I should put myself as authorship > or you, since mostly the patch is your code now. ;-) For now I'll > leave my authorship but feel free to claim it and change me to just > "Suggested-by". :-) No worry. Without your patches, I would have never come up with this. The code improvement happened in general review process. > NOTE: one thing I noticed about your example code is that you weren't > always consistent about $(1) vs. $1. I've changed things to be > consistent at the expense of extra parenthesis. If you hate it, let > me know. I accept your choice. Keeping the consistent coding style is good. If we decide to use $1 instead of $(1), we should convert all instances tree-wide for consistency. >> >> __set-and-store takes two arguments, >> but here, it is called with one argument __cached_$(1) >> >> Probably, $$(try-run, ...) will be passed as $2, >> but I do not know why this works. > > Whew! It's not just me that's confused by all this... ;-) It looks > like you continued to use this in your suggestion, so I guess you > thought it was useful, at least... Yeah, it's a bit of magic. The > goal was to have one less set of evaluating and passing going around. > > So really '__set-and-store' takes _1_ argument. It outputs a string > where it uses this argument. Also part of the output string is a > reference to $(2). This will refer to the caller's second variable. > > === > > Maybe a "simpler" example? > > define example > $$(eval $(1) := $$(2)) > endef > > ex_usage = $(eval $(call example,$(1)))$($(1)) > > .PHONY: ex > ex: > @echo $(call ex_usage,myvar,myval) > @echo $(myvar) > > If you do "make ex" you'll see "myval" printed twice. > > -- > > Walking through that, let's first remove the "call" which we can do in > this case pretty easily because there are no "ifdefs", unlike the real > code. Since the first argument to "example" is $(1), the above is the > same as: > > ex_usage = $(eval $$(eval $(1) := $$(2)))$($(1)) > > ...now we need to resolve that recursively where $(1) is 'myvar' and > $(2) is 'myval'. So we get: > > 1. $(eval $$(eval $(1) := $$(2)))$($(1)) > 2. $(eval $$(eval myvar := $$(2)))$(myvar) > 3. $(eval myvar := $(2))$(myvar) > 4. $(eval myvar := myval)$(myvar) This example is very helpful! Now I understood this. Thanks for clear explanation! > -- > > An alternate version of my "simpler" example that doesn't use this magic: > > define example > $$(eval $(1) := $(2)) > endef > > ex_usage = $(eval $(call example,$(1),$(2)))$($(1)) > > .PHONY: ex > ex: > @echo $(call ex_usage,myvar,myval) > @echo $(myvar) > > ...that works OK for the simple example case but you get into weird > esoteric issues because of the extra eval. You'll find that the two > definitions behave differently for: > > @echo $(call ex_usage,myvar,my$$$$(cat /proc/cmdline)val) > > ...in that example the "intention" is that bash should be passed the > string: echo my$(cat /proc/cmdline)val, so it escapes the "$" once to > "defer" it and twice because it wants the "$" to go through to bash. You are right. If we want to get the same result by using the alternate version, we need one more level of escaping, so end up with crazy escaping: @echo $(call ex_usage,myvar,my$$$$$$$$(cat /proc/cmdline)val) So, the magic is unclear, but very helpful. >>> >>> # __cc-option >>> # Usage: MY_CFLAGS += $(call __cc-option,$(CC),$(MY_CFLAGS),-march=winchip-c6,-march=i586) >>> -__cc-option = $(call try-run,\ >>> - $(1) -Werror $(2) $(3) -c -x c /dev/null -o "$$TMP",$(3),$(4)) >>> +__cc-option = \ >>> + $(call cached-val,$(call __sanitize-opt,__cc_opt_$(1)_$(2)_$(3)_$(4)),\ >>> + $$(call try-run,\ >>> + $(call __comma,$(1)) -Werror \ >>> + $(call __comma,$(2)) \ >>> + $(call __comma,$(3)) -c -x c /dev/null \ >>> + -o "$$$$TMP",$(call __comma,$(strip $(3))),$(call __comma,$(strip $(4))))) >> >> >> I did not follow how this was working here... > > Hoo boy. Let's see... Which part were you curious about? I can try > to explain the bits and you can tell me if I got them all. Note that > many of these aren't super important anymore since they're not needed > with your improvements: Right, all of these are not necessary any more. I did not mean that you should explain all of these. I did not want read them, so I started to consider simpler idea that I can understand. Sorry if you used much time for me, but this discussion helped me a lot to understand it deeply. > > > * $(call __sanitize-opt,__cc_opt_$(1)_$(2)_$(3)_$(4)): > > Tries to make a variable name that will be different if you have any > different parameters. Yes, this is an easy part. > > * $$(call try-run > > The "$$" around this call is because it's deferred and a general > concept for "cached-val". If (and only if) we decide that we need to > recompute this value then we'll run this through an "eval". By having > the $$ then this action won't be taken unless the string is eval-ed. > Note that plenty of stuff _will_ still happen right away, though. All > of the $(call __comma,$(1)) are _not_ deferred. That's fine since > that doesn't lead to a fork-exec and thus isn't "expensive". Actually, I could not understand how this worked in v1, but with the help of your simplified example above, the intention is clear. Thanks. > * $(call __comma,$(1)) > > I'll admit that I didn't dig all the way into this. I found at least > one case where "make" was incorrectly mixing up arguments due to the > "eval" and the "__comma" fixed it. I decided it was important to do > this for all arguments. Possibly other things might need to be > escaped too, but I didn't find any cases that needed it... ...but no > longer needed with your improvements... OK, this is probably for corner cases, not that important part. > > * $$$$TMP > > An extra level of escaping is needed since it goes through an extra > "eval". This means $$ => $$$$ This is now clear to me with the simplified example and its alternate version. > > * $(strip $(3)) > > This was important because otherwise the cache otherwise wasn't > completely "stable". If you ran "make" the first time you'd get a > cache, then running "make" the 2nd time would give you extra entries > in the cache. After the 3rd time you were OK, but it seemed nice to > make it stable faster. > > To understand this, first understand that some people call __cc_option > with a space after the ','. Make treats this as important, thus these > two are different: > > val := $(call cc-option, -opt1, -opt2) > val := $(call cc-option,-opt1,-opt2) > > In one case val would be " -opt1" or " -opt2". In the other case > "-opt1" or "-opt2". > > However, when we cache things the space suddenly becomes irrelevant. > That's because we've end up with something like: > > __cached_result := -opt1 > > ...and even though there are two spaces after the ":=" make throws that away. > > The result of all of that is that we'd get slightly different numbers > of spaces after the cache was populated. That could probably be > ignored except that some flags are additive and previous flags are > passed as arguments to later calls to __cc-option. That means that > previous flags are passed to __sanitize-opt. ...and in __sanitize-opt > spaces are significant. ...so you'd come up with a slightly different > variable name on the first run and the subsequent runs... > > Probably waaaay more than you expected for a simple $(strip) call, huh? Understood. The whitespace problem is nasty. > In any case this doesn't seem to matter anymore with your improvements > (I didn't dig into why), though I have noticed that the cache does > still return slightly different spacing for some of the results... > As far as I tested, I always see only one space after ":=" in v2. I did not consider this deeply, but something is working nicely behind the scene.
Hi, On Thu, Oct 5, 2017 at 12:26 AM, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > As far as I tested, I always see only one space after ":=" in v2. > > I did not consider this deeply, > but something is working nicely behind the scene. Try adding this to the end of the main Makefile: +$(info LDFLAGS_BUILD_ID = $(LDFLAGS_BUILD_ID)) +$(info KBUILD_ARFLAGS = $(KBUILD_ARFLAGS)) +$(info KBUILD_CFLAGS = $(KBUILD_CFLAGS)) +$(info KBUILD_AFLAGS = $(KBUILD_AFLAGS)) +$(info KBUILD_CPPFLAGS = $(KBUILD_CPPFLAGS)) +$(info REALMODE_CFLAGS = $(REALMODE_CFLAGS)) + endif # skip-makefile Record what you see. Then apply my patches and run your build again (actually, run it twice and look at the 2nd time, just to be sure). I think you'll see slightly different spacing in the flags for the two runs. I don't think this is terribly important, though. -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2017-10-06 5:58 GMT+09:00 Doug Anderson <dianders@chromium.org>: > Hi, > > On Thu, Oct 5, 2017 at 12:26 AM, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: >> As far as I tested, I always see only one space after ":=" in v2. >> >> I did not consider this deeply, >> but something is working nicely behind the scene. > > Try adding this to the end of the main Makefile: > > +$(info LDFLAGS_BUILD_ID = $(LDFLAGS_BUILD_ID)) > +$(info KBUILD_ARFLAGS = $(KBUILD_ARFLAGS)) > +$(info KBUILD_CFLAGS = $(KBUILD_CFLAGS)) > +$(info KBUILD_AFLAGS = $(KBUILD_AFLAGS)) > +$(info KBUILD_CPPFLAGS = $(KBUILD_CPPFLAGS)) > +$(info REALMODE_CFLAGS = $(REALMODE_CFLAGS)) > + > endif # skip-makefile > > Record what you see. Then apply my patches and run your build again > (actually, run it twice and look at the 2nd time, just to be sure). I > think you'll see slightly different spacing in the flags for the two > runs. I don't think this is terribly important, though. > Yes. I see slight difference in the resulted KBUILD_CFLAGS etc. What I meant is: In v2, I always see only one space after the assignment operator ":=" in .cache.mk even without $(strip ) $(call cc-option,-fno-PIE) $(call cc-option, -fno-PIE) $(call cc-option, -fno-PIE) All of the three gave me the same result.
diff --git a/Documentation/kbuild/makefiles.txt b/Documentation/kbuild/makefiles.txt index 329e740adea7..679e8483cb4f 100644 --- a/Documentation/kbuild/makefiles.txt +++ b/Documentation/kbuild/makefiles.txt @@ -581,6 +581,27 @@ more details, with real examples. LDFLAGS_vmlinux += $(call ld-option, -X) +--- 3.14 $(LD) support function cache + +One thing to realize about all the calls to the above support functions +is that each use of them requires a full invocation of an external tool, like +the C compiler, assembler, or linker. If nothing else that invocation will +cause a fork/exec/shared library link. In some build environments, however, it +could also involve traversing thorough one or more wrappers. To put some +numbers on it, I've measured compiler invocations of as fast as 4ms or +as slow as 150ms. + +Many of the above support functions are used in places where they are +evaluated on each invocation of Make before anything else can run. Even on +a simple command like "make help" we need to evaluate every single one of the +variables. + +To avoid this slow overhead we can cache the result of these invocations. +If we store this cache in a way that's easy for Make to use (like in Makefile +format) then it will be very quick and we'll save a lot of time with each +invocation of make. + + === 4 Host Program support Kbuild supports building executables on the host for use during the diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include index 9ffd3dda3889..de88120f1763 100644 --- a/scripts/Kbuild.include +++ b/scripts/Kbuild.include @@ -8,6 +8,8 @@ squote := ' empty := space := $(empty) $(empty) space_escape := _-_SPACE_-_ +right_paren := ) +left_paren := ( ### # Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o @@ -69,6 +71,53 @@ endef # gcc support functions # See documentation in Documentation/kbuild/makefiles.txt +# Tools for caching Makefile variables that are "expensive" to compute. +# +# Here we want to help deal with variables that take a long time to compute +# by making it easy to store these variables in a cache. +# +# The canonical example here is testing for compiler flags. On a simple system +# each call to the compiler takes 10 ms, but on a system with a compiler that's +# called through various wrappers it can take upwards of 100 ms. If we have +# 100 calls to the compiler this can take 1 second (on a simple system) or 10 +# seconds (on a complicated system). +# +# The "cache" will be in Makefile syntax and can be directly included. +# Any time we try to reference a variable that's not in the cache we'll +# calculate it and store it in the cache for next time. + +# Include values from last time +-include Makefile-cache.o + +# Usage: $(call cached-val,variable_name,escaped_expensive_operation) +# Example: $(call cached-val,md5_val,$$(shell md5sum /usr/bin/gcc) +# +# If the variable is already defined we'll just use it. If it's not, it will +# be calculated and stored in a persistent (disk-based) cache for the next +# invocation of Make. The call will evaluate to the value of the variable. +# +# This is a bit of voodoo, but basic explanation is that if the variable +# was undefined then we'll evaluate the expensive operation and store it into +# the variable. We'll then store that value in the cache and finally output +# the value. +define __set-and-store +ifeq ($(origin $(1)),undefined) + $$(eval $(1) := $$(2)) + $$(shell echo '$(1) := $$($(1))' >> Makefile-cache.o) +endif +endef +cached-val = $(eval $(call __set-and-store,__cached_$(1)))$(__cached_$(1)) + +# Usage: $(call __sanitize-opt,Hello=Hola$(comma)Goodbye Adios) +# +# Convert all '$', ')', '(', '\', '=', ' ', ',' to '_' +__sanitize-opt = $(subst $$,_,$(subst $(right_paren),_,$(subst $(left_paren),_,$(subst \,_,$(subst =,_,$(subst $(space),_,$(subst $(comma),_,$(1)))))))) + +# Usage = $(call __comma,something_with_comma) +# +# Convert ',' to '$(comma)' which can help it getting interpreted by eval. +__comma = $(subst $(comma),$$(comma),$(1)) + # cc-cross-prefix # Usage: CROSS_COMPILE := $(call cc-cross-prefix, m68k-linux-gnu- m68k-linux-) # Return first prefix where a prefix$(CC) is found in PATH. @@ -99,19 +148,34 @@ try-run = $(shell set -e; \ # as-option # Usage: cflags-y += $(call as-option,-Wa$(comma)-isa=foo,) -as-option = $(call try-run,\ - $(CC) $(KBUILD_CFLAGS) $(1) -c -x assembler /dev/null -o "$$TMP",$(1),$(2)) +as-option = \ + $(call cached-val,$(call __sanitize-opt,\ + as_opt_$(CC)_$(KBUILD_CFLAGS)_$(1)_$(2)),\ + $$(call try-run,\ + $(CC) $(call __comma,$(KBUILD_CFLAGS)) $(call __comma,$(1)) \ + -c -x assembler /dev/null \ + -o "$$$$TMP",$(call __comma,$(1)),$(call __comma,$(2)))) # as-instr # Usage: cflags-y += $(call as-instr,instr,option1,option2) -as-instr = $(call try-run,\ - printf "%b\n" "$(1)" | $(CC) $(KBUILD_AFLAGS) -c -x assembler -o "$$TMP" -,$(2),$(3)) +as-instr = \ + $(call cached-val,$(call __sanitize-opt,\ + as_instr_$(CC)_$(KBUILD_AFLAGS)_$(1)_$(2)_$(3)),\ + $$(call try-run,\ + printf "%b\n" "$(call __comma,$(1))" | \ + $(CC) $(call __comma,$(KBUILD_AFLAGS)) -c -x assembler \ + -o "$$$$TMP" -,$(call __comma,$(2)),$(call __comma,$(3)))) # __cc-option # Usage: MY_CFLAGS += $(call __cc-option,$(CC),$(MY_CFLAGS),-march=winchip-c6,-march=i586) -__cc-option = $(call try-run,\ - $(1) -Werror $(2) $(3) -c -x c /dev/null -o "$$TMP",$(3),$(4)) +__cc-option = \ + $(call cached-val,$(call __sanitize-opt,__cc_opt_$(1)_$(2)_$(3)_$(4)),\ + $$(call try-run,\ + $(call __comma,$(1)) -Werror \ + $(call __comma,$(2)) \ + $(call __comma,$(3)) -c -x c /dev/null \ + -o "$$$$TMP",$(call __comma,$(strip $(3))),$(call __comma,$(strip $(4))))) # Do not attempt to build with gcc plugins during cc-option tests. # (And this uses delayed resolution so the flags will be up to date.) @@ -130,24 +194,42 @@ hostcc-option = $(call __cc-option, $(HOSTCC),\ # cc-option-yn # Usage: flag := $(call cc-option-yn,-march=winchip-c6) -cc-option-yn = $(call try-run,\ - $(CC) -Werror $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",y,n) +cc-option-yn = \ + $(call cached-val,$(call __sanitize-opt,\ + cc_opt_yn_$(CC)_$(KBUILD_CPPFLAGS)_$(CC_OPTION_CFLAGS)$(1)),\ + $$(call try-run,\ + $(CC) -Werror $(call __comma,$(KBUILD_CPPFLAGS)) \ + $(call __comma,$(CC_OPTION_CFLAGS)) \ + $(call __comma,$(1)) \ + -c -x c /dev/null -o "$$$$TMP",y,n)) # cc-disable-warning # Usage: cflags-y += $(call cc-disable-warning,unused-but-set-variable) -cc-disable-warning = $(call try-run,\ - $(CC) -Werror $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1))) +cc-disable-warning = \ + $(call cached-val,$(call __sanitize-opt,\ + cc_dis_wn_$(CC)_$(KBUILD_CPPFLAGS)_$(CC_OPTION_CFLAGS)$(1)_$(2)),\ + $$(call try-run,\ + $(CC) -Werror $(call __comma,$(KBUILD_CPPFLAGS)) \ + $(call __comma,$(CC_OPTION_CFLAGS)) \ + -W$(strip $(call __comma,$(1))) -c -x c /dev/null \ + -o "$$$$TMP",-Wno-$(strip $(call __comma,$(1))))) # cc-name # Expands to either gcc or clang -cc-name = $(shell $(CC) -v 2>&1 | grep -q "clang version" && echo clang || echo gcc) +cc-name = \ + $(call cached-val,$(call __sanitize-opt,cc_name_$(CC)),\ + $$(shell $(CC) -v 2>&1 | grep -q "clang version" && echo clang || echo gcc)) # cc-version -cc-version = $(shell $(CONFIG_SHELL) $(srctree)/scripts/gcc-version.sh $(CC)) +cc-version = \ + $(call cached-val,$(call __sanitize-opt,cc_version_$(CC)),\ + $$(shell $(CONFIG_SHELL) $(srctree)/scripts/gcc-version.sh $(CC))) # cc-fullversion -cc-fullversion = $(shell $(CONFIG_SHELL) \ - $(srctree)/scripts/gcc-version.sh -p $(CC)) +cc-fullversion = \ + $(call cached-val,$(call __sanitize-opt,cc_full_version_$(CC)),\ + $$(shell $(CONFIG_SHELL) \ + $(srctree)/scripts/gcc-version.sh -p $(CC))) # cc-ifversion # Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1) @@ -159,18 +241,31 @@ cc-if-fullversion = $(shell [ $(cc-fullversion) $(1) $(2) ] && echo $(3) || echo # cc-ldoption # Usage: ldflags += $(call cc-ldoption, -Wl$(comma)--hash-style=both) -cc-ldoption = $(call try-run,\ - $(CC) $(1) -nostdlib -x c /dev/null -o "$$TMP",$(1),$(2)) +cc-ldoption = \ + $(call cached-val,$(call __sanitize-opt,\ + cc_ld_opt_$(CC)_$(1)_$(2)),\ + $$(call try-run,\ + $(CC) $(call __comma,$(1)) -nostdlib -x c /dev/null \ + -o "$$$$TMP",$(call __comma,$(1)),$(call __comma,$(2)))) # ld-option # Usage: LDFLAGS += $(call ld-option, -X) -ld-option = $(call try-run,\ - $(CC) -x c /dev/null -c -o "$$TMPO" ; $(LD) $(1) "$$TMPO" -o "$$TMP",$(1),$(2)) +ld-option = \ + $(call cached-val,$(call __sanitize-opt,\ + ld_opt_$(CC)_$(LD)_$(1)_$(2)),\ + $$(call try-run,\ + $(CC) -x c /dev/null -c -o "$$$$TMPO" ; \ + $(LD) $(call __comma,$(1)) "$$$$TMPO" \ + -o "$$$$TMP",$(call __comma,$(1)),$(call __comma,$(2)))) # ar-option # Usage: KBUILD_ARFLAGS := $(call ar-option,D) # Important: no spaces around options -ar-option = $(call try-run, $(AR) rc$(1) "$$TMP",$(1),$(2)) +ar-option = \ + $(call cached-val,$(call __sanitize-opt,\ + ar_opt_$(AR)_$(1)_$(2)),\ + $$(call try-run, $(AR) rc$(call __comma,$(1)) \ + "$$$$TMP",$(call __comma,$(1)),$(call __comma,$(2)))) # ld-version # Note this is mainly for HJ Lu's 3 number binutil versions
While timing a "no-op" build of the kernel (incrementally building the kernel even though nothing changed) in the Chrome OS build system I found that it was much slower than I expected. Digging into things a bit, I found that quite a bit of the time was spent invoking the C compiler even though we weren't actually building anything. Currently in the Chrome OS build system the C compiler is called through a number of wrappers (one of which is written in python!) and can take upwards of 100 ms to invoke even if we're not doing anything difficult, so these invocations of the compiler were taking a lot of time. Worse the invocations couldn't seem to take advantage of the multiple cores on my system. Certainly it seems like we could make the compiler invocations in the Chrome OS build system faster, but only to a point. Inherently invoking a program as big as a C compiler is a fairly heavy operation. Thus even if we can speed the compiler calls it made sense to track down what was happening. It turned out that all the compiler invocations were coming from usages like this in the kernel's Makefile: KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks,) Due to the way cc-option and similar statements work the above contains an implicit call to the C compiler. ...and due to the fact that we're storing the result in KBUILD_CFLAGS, a simply expanded variable, the call will happen every time the Makefile is parsed, even if there are no users of KBUILD_CFLAGS. Rather than redoing this computation every time, it makes a lot of sense to cache the result of all of the Makefile's compiler calls just like we do when we compile a ".c" file to a ".o" file. Conceptually this is quite a simple idea. ...and since the calls to invoke the compiler and similar tools are centrally located in the Kbuild.include file this doesn't even need to be super invasive. Implementing the cache in a simple-to-use and efficient way is not quite as simple as it first sounds, though. To get maximum speed we really want the cache in a format that make can natively understand and make doesn't really have an ability to load/parse files. ...but make _can_ import other Makefiles, so the solution is to store the cache in Makefile format. This requires coming up with a valid/unique Makefile variable name for each value to be cached, but that's solvable with some cleverness. After this change, we'll automatically create a "Makefile-cache.o" file that will contain our cached variables. We'll load this on each invocation of make and will avoid recomputing anything that's already in our cache. The cache is stored in a format that it shouldn't need any invalidation since anything that might change should affect the "key" and any old cached value won't be used. The cache will be cleaned by virtue of the ".o" suffix by a "make clean". Signed-off-by: Douglas Anderson <dianders@chromium.org> --- Documentation/kbuild/makefiles.txt | 21 ++++++ scripts/Kbuild.include | 133 +++++++++++++++++++++++++++++++------ 2 files changed, 135 insertions(+), 19 deletions(-)