What do you think of this? I'm still running a test build --- "make PROFILE=BUILD all" takes quite a long time, so this is still an RFC; I figure there will still be some places where people will point out more nits to be polished. :-) (In particular, I just noticed I left the V=1 for debugging purposes in this version....) - Ted >From 4bf14e732216fd1327da2e3c8c6dfc0a3f689e1b Mon Sep 17 00:00:00 2001 From: Theodore Ts'o <tytso@xxxxxxx> Date: Thu, 2 Feb 2012 13:56:22 -0500 Subject: [PATCH] Fix build problems related to profile-directed optimization There was a number of problems I ran into when trying the profile-directed optimizations added by Andi Kleen in git commit 7ddc2710b9. (This was using gcc 4.4 found on many enterprise distros.) 1) The -fprofile-generate and -fprofile-use commands are incompatible with ccache; the code ends up looking in the wrong place for the gcda files based on the ccache object names. 2) If the makefile notices that CFLAGS are different, it will rebuild all of the binaries. Hence the recipe originally specified by the INSTALL file ("make profile-all" followed by "make install") doesn't work. It will appear to work, but the binaries will end up getting built with no optimization. This patch fixes this by using an explicit set of options passed via the PROFILE variable then using this to directly manipulate CFLAGS and EXTLIBS. The developer can run "make PROFILE=BUILD all ; make PROFILE=BUILD install" to do an automatic two-pass build using the test suite as the sample workload for the purpose of profiling. Alternatively, the profiling version of binaries can be built using: make PROFILE=GEN PROFILE_DIR=/var/cache/profile all make PROFILE=GEN install and then after git has been used a number of times, the optimized version of the binary can be built as follows: make PROFILE=USE PROFILE_DIR=/var/cache/profile all make PROFILE=USE install Signed-off-by: "Theodore Ts'o" <tytso@xxxxxxx> Cc: Andi Kleen <ak@xxxxxxxxxxxxxxx> --- INSTALL | 4 ++-- Makefile | 41 ++++++++++++++++++++++++++++++----------- 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/INSTALL b/INSTALL index 6fa83fe..73b654b 100644 --- a/INSTALL +++ b/INSTALL @@ -28,8 +28,8 @@ set up install paths (via config.mak.autogen), so you can write instead If you're willing to trade off (much) longer build time for a later faster git you can also do a profile feedback build with - $ make profile-all - # make prefix=... install + $ make --prefix=/usr PROFILE=BUILD all + # make --prefix=/usr PROFILE=BUILD install This will run the complete test suite as training workload and then rebuild git with the generated profile feedback. This results in a git diff --git a/Makefile b/Makefile index c457c34..7d66d5c 100644 --- a/Makefile +++ b/Makefile @@ -1772,6 +1772,24 @@ ifdef ASCIIDOC7 export ASCIIDOC7 endif +### profile feedback build +# + +# Can adjust this to be a global directory if you want to do extended +# data gathering +PROFILE_DIR := $(CURDIR) + +ifeq "$(PROFILE)" "GEN" + CFLAGS += -fprofile-generate=$(PROFILE_DIR) -DNO_NORETURN=1 + EXTLIBS += -lgcov + export CCACHE_DISABLE=t + V=1 +else ifneq "$PROFILE" "" + CFLAGS += -fprofile-use=$(PROFILE_DIR) -fprofile-correction -DNO_NORETURN=1 + export CCACHE_DISABLE=t + V=1 +endif + # Shell quote (do not use $(call) to accommodate ancient setups); SHA1_HEADER_SQ = $(subst ','\'',$(SHA1_HEADER)) @@ -1828,7 +1846,15 @@ export DIFF TAR INSTALL DESTDIR SHELL_PATH SHELL = $(SHELL_PATH) -all:: shell_compatibility_test $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS +all:: shell_compatibility_test + +ifeq "$(PROFILE)" "BUILD" +all:: profile-clean + $(MAKE) PROFILE=GEN all + $(MAKE) PROFILE=GEN -j1 test +endif + +all:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS ifneq (,$X) $(QUIET_BUILT_IN)$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), test -d '$p' -o '$p' -ef '$p$X' || $(RM) '$p';) endif @@ -2699,16 +2725,9 @@ cover_db_html: cover_db ### profile feedback build # -.PHONY: profile-all profile-clean - -PROFILE_GEN_CFLAGS := $(CFLAGS) -fprofile-generate -DNO_NORETURN=1 -PROFILE_USE_CFLAGS := $(CFLAGS) -fprofile-use -fprofile-correction -DNO_NORETURN=1 +.PHONY: profile-clean profile-clean: - $(RM) $(addsuffix *.gcda,$(object_dirs)) - $(RM) $(addsuffix *.gcno,$(object_dirs)) + $(RM) $(addsuffix *.gcda,$(addprefix $(PROFILE_DIR)/, $(object_dirs))) + $(RM) $(addsuffix *.gcno,$(addprefix $(PROFILE_DIR)/, $(object_dirs))) -profile-all: profile-clean - $(MAKE) CFLAGS="$(PROFILE_GEN_CFLAGS)" all - $(MAKE) CFLAGS="$(PROFILE_GEN_CFLAGS)" -j1 test - $(MAKE) CFLAGS="$(PROFILE_USE_CFLAGS)" all -- 1.7.8.11.gefc1f.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html