Re: [PATCH, RFC] Fix build problems related to profile-directed optimization

7 messages, 4 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH, RFC] Fix build problems related to profile-directed optimization

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:52:56

Theodore Tso [off-list ref] writes:
On Feb 3, 2012, at 1:00 AM, Junio C Hamano wrote:
quoted
* I am happy that this version handles this well:

  $ make PROFILE=BUILD install

  even though you did not advertise as such in INSTALL ;-).
I can mention it, although it will mean adding more verbiage about
profile-directed optimization into the INSTALL.
Oh, sorry, I didn't mean it that way.  Please read it as: "Something that
is a natural thing for people to expect after reading what is in INSTALL
works correctly. Yay! Thanks."

[PATCH] Fix build problems related to profile-directed optimization

From: Theodore Ts'o <tytso@mit.edu>
Date: 2016-06-15 22:52:57

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 ; sudo make
PROFILE=BUILD install" automatically run a two-pass build with the
test suite run in between as the sample workload for the purpose of
recording profiling information to do the profile-directed
optimization.

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 for a while, 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@mit.edu>
Cc: Andi Kleen <redacted>
---
 INSTALL  |   17 +++++++++++++----
 Makefile |   53 +++++++++++++++++++++++++++++++++++------------------
 2 files changed, 48 insertions(+), 22 deletions(-)
diff --git a/INSTALL b/INSTALL
index 6fa83fe..5b7eec1 100644
--- a/INSTALL
+++ b/INSTALL
@@ -28,16 +28,25 @@ 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
 which is a few percent faster on CPU intensive workloads.  This
 may be a good tradeoff for distribution packagers.
 
-Note that the profile feedback build stage currently generates
-a lot of additional compiler warnings.
+Or if you just want to install a profile-optimized version of git into
+your home directory, you could run:
+
+	$ make PROFILE=BUILD install
+
+As a caveat: a profile-optimized build takes a *lot* longer since it
+is the sources have to be built twice, and in order for the profiling
+measurements to work properly, ccache must be disabled and the test
+suite has to be run using only a single CPU.  In addition, the profile
+feedback build stage currently generates a lot of additional compiler
+warnings.
 
 Issues of note:
 
diff --git a/Makefile b/Makefile
index c457c34..8cea247 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,17 @@ 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"
+ifeq ($(filter all,$(MAKECMDGOALS)),all)
+all:: profile-clean
+	$(MAKE) PROFILE=GEN all
+	$(MAKE) PROFILE=GEN -j1 test
+endif
+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
@@ -2557,7 +2585,11 @@ distclean: clean
 	$(RM) configure
 	$(RM) po/git.pot
 
-clean:
+profile-clean:
+	$(RM) $(addsuffix *.gcda,$(addprefix $(PROFILE_DIR)/, $(object_dirs)))
+	$(RM) $(addsuffix *.gcno,$(addprefix $(PROFILE_DIR)/, $(object_dirs)))
+
+clean: profile-clean
 	$(RM) *.o block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o vcs-svn/*.o \
 		builtin/*.o $(LIB_FILE) $(XDIFF_LIB) $(VCSSVN_LIB)
 	$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X
@@ -2587,7 +2619,7 @@ ifndef NO_TCLTK
 endif
 	$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-GUI-VARS GIT-BUILD-OPTIONS
 
-.PHONY: all install clean strip
+.PHONY: all install profile-clean clean strip
 .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
 .PHONY: FORCE cscope
 
@@ -2697,18 +2729,3 @@ cover_db: coverage-report
 cover_db_html: cover_db
 	cover -report html -outputdir 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
-
-profile-clean:
-	$(RM) $(addsuffix *.gcda,$(object_dirs))
-	$(RM) $(addsuffix *.gcno,$(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.9.107.g8e04a

Re: [PATCH] Fix build problems related to profile-directed optimization

From: Jeff King <hidden>
Date: 2016-06-15 22:52:59

On Sun, Feb 05, 2012 at 07:44:50PM -0500, Theodore Ts'o wrote:
quoted hunk
diff --git a/INSTALL b/INSTALL
index 6fa83fe..5b7eec1 100644
--- a/INSTALL
+++ b/INSTALL
@@ -28,16 +28,25 @@ 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
Eh? --prefix?
+As a caveat: a profile-optimized build takes a *lot* longer since it
+is the sources have to be built twice, and in order for the profiling
s/it is//
quoted hunk
diff --git a/Makefile b/Makefile
index c457c34..8cea247 100644
--- a/Makefile
+++ b/Makefile
@@ -1772,6 +1772,24 @@ ifdef ASCIIDOC7
[...]
+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
Did you mean "$(PROFILE)" in the second conditional?

-Peff

Re: [PATCH] Fix build problems related to profile-directed optimization

From: Ted Ts'o <tytso@mit.edu>
Date: 2016-06-15 22:53:00

On Sun, Feb 05, 2012 at 11:18:39PM -0500, Jeff King wrote:
quoted
-	$ make profile-all
-	# make prefix=... install
+	$ make --prefix=/usr PROFILE=BUILD all
+	# make --prefix=/usr PROFILE=BUILD install
Eh? --prefix?
Oops, configure meme strikes; will fix.
quoted
+As a caveat: a profile-optimized build takes a *lot* longer since it
+is the sources have to be built twice, and in order for the profiling
s/it is//
Thanks, will fix.
quoted
+ifeq "$(PROFILE)" "GEN"
+	CFLAGS += -fprofile-generate=$(PROFILE_DIR) -DNO_NORETURN=1
+	EXTLIBS += -lgcov
+	export CCACHE_DISABLE=t
+	V=1
+else ifneq "$PROFILE" ""
Did you mean "$(PROFILE)" in the second conditional?
Yes, thanks.   Will fix.

						- Ted

[PATCH] Fix build problems related to profile-directed optimization

From: Theodore Ts'o <tytso@mit.edu>
Date: 2016-06-15 22:53:00

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 ; sudo make
PROFILE=BUILD install" automatically run a two-pass build with the
test suite run in between as the sample workload for the purpose of
recording profiling information to do the profile-directed
optimization.

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 for a while, 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@mit.edu>
Cc: Andi Kleen <redacted>
---
 INSTALL  |   17 +++++++++++++----
 Makefile |   53 +++++++++++++++++++++++++++++++++++------------------
 2 files changed, 48 insertions(+), 22 deletions(-)
diff --git a/INSTALL b/INSTALL
index 6fa83fe..58b2b86 100644
--- a/INSTALL
+++ b/INSTALL
@@ -28,16 +28,25 @@ 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
 which is a few percent faster on CPU intensive workloads.  This
 may be a good tradeoff for distribution packagers.
 
-Note that the profile feedback build stage currently generates
-a lot of additional compiler warnings.
+Or if you just want to install a profile-optimized version of git into
+your home directory, you could run:
+
+	$ make PROFILE=BUILD install
+
+As a caveat: a profile-optimized build takes a *lot* longer since the
+git tree must be built twice, and in order for the profiling
+measurements to work properly, ccache must be disabled and the test
+suite has to be run using only a single CPU.  In addition, the profile
+feedback build stage currently generates a lot of additional compiler
+warnings.
 
 Issues of note:
 
diff --git a/Makefile b/Makefile
index c457c34..719ffca 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,17 @@ 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"
+ifeq ($(filter all,$(MAKECMDGOALS)),all)
+all:: profile-clean
+	$(MAKE) PROFILE=GEN all
+	$(MAKE) PROFILE=GEN -j1 test
+endif
+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
@@ -2557,7 +2585,11 @@ distclean: clean
 	$(RM) configure
 	$(RM) po/git.pot
 
-clean:
+profile-clean:
+	$(RM) $(addsuffix *.gcda,$(addprefix $(PROFILE_DIR)/, $(object_dirs)))
+	$(RM) $(addsuffix *.gcno,$(addprefix $(PROFILE_DIR)/, $(object_dirs)))
+
+clean: profile-clean
 	$(RM) *.o block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o vcs-svn/*.o \
 		builtin/*.o $(LIB_FILE) $(XDIFF_LIB) $(VCSSVN_LIB)
 	$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X
@@ -2587,7 +2619,7 @@ ifndef NO_TCLTK
 endif
 	$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-GUI-VARS GIT-BUILD-OPTIONS
 
-.PHONY: all install clean strip
+.PHONY: all install profile-clean clean strip
 .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
 .PHONY: FORCE cscope
 
@@ -2697,18 +2729,3 @@ cover_db: coverage-report
 cover_db_html: cover_db
 	cover -report html -outputdir 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
-
-profile-clean:
-	$(RM) $(addsuffix *.gcda,$(object_dirs))
-	$(RM) $(addsuffix *.gcno,$(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.9.107.g8e04a

Re: [PATCH] Fix build problems related to profile-directed optimization

From: Ted Ts'o <tytso@mit.edu>
Date: 2016-06-15 22:53:01

Junio, any comments on my most recent spin of this patch?  Any changes
you'd like to see?

Thanks,

					- Ted

Re: [PATCH] Fix build problems related to profile-directed optimization

From: Johannes Sixt <hidden>
Date: 2016-06-15 22:53:01

Am 2/8/2012 19:53, schrieb Ted Ts'o:
Junio, any comments on my most recent spin of this patch?  Any changes
you'd like to see?
I need the following to unbreak my build on Windows.
--- >8 ---
From: Johannes Sixt <redacted>
Subject: [PATCH] Makefile: fix syntax for older make

It is necessary to write the else branch as a nested conditional. Also,
write the conditions with parentheses because we use them throughout the
Makefile.

Signed-off-by: Johannes Sixt <redacted>
---
 Makefile |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index bfc5daa..01a3c77 100644
--- a/Makefile
+++ b/Makefile
@@ -1784,16 +1784,18 @@ endif
 # data gathering
 PROFILE_DIR := $(CURDIR)
 
-ifeq "$(PROFILE)" "GEN"
+ifeq ("$(PROFILE)","GEN")
 	CFLAGS += -fprofile-generate=$(PROFILE_DIR) -DNO_NORETURN=1
 	EXTLIBS += -lgcov
 	export CCACHE_DISABLE=t
 	V=1
-else ifneq "$(PROFILE)" ""
+else
+ifneq ("$(PROFILE)","")
 	CFLAGS += -fprofile-use=$(PROFILE_DIR) -fprofile-correction -DNO_NORETURN=1
 	export CCACHE_DISABLE=t
 	V=1
 endif
+endif
 
 # Shell quote (do not use $(call) to accommodate ancient setups);
 
-- 
1.7.9.1420.gae2d6
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help