237239 - Use profile-directed block ordering for release builds

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, defect)

Description

[Brian Ryner (not reading)]

gcc's -fprofile-arcs option can be used to make a build that generates arc
profiling information at runtime.  This information can then be used in a
subsequent build when -fbranch-probabilities is used.  This enables us to use
(hoepfully) real-world data about which branches are taken most often to improve
block ordering.

To test this, I made a -fprofile-arcs build and ran it against two cycles of the
tinderbox pageload test.  I then rebuilt with -fbranch-probabilities.  The
resulting build was 8% faster on the pageload test than a stock build (both
builds were using -O2, on gcc 3.3.2, and the test machine is a 500 MHz P3).

In gcc 3.4 and above, the options we want to use are -fprofile-generate and
-fprofile-use (the functionality has been extended to record more than just
branch-taken information).

I'm attaching a patch which adds some support for this to the build system and
can automate the profile generation.  One caveat is that this doesn't seem to
work with DSO's, so this option will only be useful for static builds.  (it
might be worth investigating whether we can create a static build, but with
-fPIC, and apply that data to a non-static build).

Comment 1

[Brian Ryner (not reading)]

Attachment: patch

This patch adds a new client.mk target, profiledbuild. To use this, you should
have something like this in your mozconfig:

mk_add_options PROFILE_GEN_SCRIPT=@TOPSRCDIR@/build/profile_pageloader.pl

The app is compiled once using -fprofile-arcs (or -fprofile-generate) and then
the profiling script is run.  The profile_pageloader.pl script I included works
for seamonkey or firefox; obviously different scripts would be desired for
other applications.  Then the build is clobbered (leaving the generated .da
files in the objdir) and rebuilt using -fbranch-probabilities (or
-fprofile-use).

(Now, we all just need faster machines to build on)

Comment 2

[Brian Ryner (not reading)]

Comment on attachment 143700 [details] [diff] [review]
patch

(also, as if it wasn't obvious, parts of profile_pageloader.pl came from
tinderbox's build-seamonkey-util.pl)

Comment 3

[Brian Ryner (not reading)]

Just fyi: I was able to get this optimization to work on Mac as well (gcc 3.3),
but there wasn't much of a speedup (about 1%, could be noise) measured on a dual
1ghz g4.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 143700 [details] [diff] [review]
patch

r=dbaron, although it might be nice to improve the perl script by pulling over
the code to use a separate (empty) home directory -- and perhaps do startup
again after the pageload run to exercise reading in a profile that's been used
(and hopefully fastload)

Comment 5

[Brendan Eich [:brendan]]

Comment on attachment 143700 [details] [diff] [review]
patch

sr=me.	How about a followup bug on what dbaron suggested?

/be

Comment 6

[Brian Ryner (not reading)]

Attachment: revised profile_pageloader.pl

This version of profile_pageloader has some improvements dbaron and I talked
about.	It sets the home directory to the dist/bin directory (I know, this
doesn't work for Win/Mac, someone can fix this later), creates a fresh profile,
runs the pageloader test, then does a second start to exercise fastload and
reading the history file.

Comment 7

[Brian Ryner (not reading)]

Comment on attachment 143700 [details] [diff] [review]
patch

requesting 1.7 approval.  This doesn't affect the default build, though we may
want to use this for the releases.

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

IMHO, if we do want to do this for releases then we should do it for nightlies
too. Otherwise we might stumble into timing/compiler bugs that only happens in
releasebuilds.

Comment 9

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

oh, and I do think we should do it for releases (and nightlies). This is a nice
speedboost.

Comment 10

[Brian Ryner (not reading)]

Firefox linux nightlies starting on 03-27 will be using profiledbuild.  Keeping
this bug open since I'll be attaching another patch soon to make this work for
non-static builds.

Comment 11

[Brian Ryner (not reading)]

Attachment: first-cut patch to support non-static builds

There's a bit of an "eww" factor here, but I'll try to explain what I'm doing
at least.  The requirements for generating a profile are:

- use the same code generation options for the profile build as you will for
the final build
- code compiled using -fprofile-arcs cannot be linked into a DSO

So what I did was to make the profile build using -fPIC but do a static link. 
Then the final build is done using the same options but linked into DSOs. 
Seems to work, though the speedup is only about 4%, not the 7-8% I measured for
static builds.	Probably because there are other penalties in a non-static
build like reduced register availability and PLT overhead.

Comment 12

[Brian Ryner (not reading)]

Comment on attachment 145202 [details] [diff] [review]
first-cut patch to support non-static builds

Requesting 1.7 approval.  This would be a great optimization to get in for 1.7
release builds.

Comment 13

[Asa Dotzler [:asa]]

Comment on attachment 145202 [details] [diff] [review]
first-cut patch to support non-static builds

a=asa (on behalf of drivers) for checkin to 1.7

Comment 14

[Brian Ryner (not reading)]

checked in.

Comment 15

[Brian Ryner (not reading)]

Oops, I regressed static profiled builds.

Comment 16

[Brian Ryner (not reading)]

Attachment: fix static profiled build

This restores the original behavior for static builds.	I tested this patch
with static and dynamic profiled builds and it works correctly in both cases.

Comment 17

[Brian Ryner (not reading)]

Comment on attachment 145790 [details] [diff] [review]
fix static profiled build

requesting 1.7 approval.  this is an important regression fix for static
builds.

Comment 18

[Brendan Eich [:brendan]]

Comment on attachment 145790 [details] [diff] [review]
fix static profiled build

a=[e-mail] for 1.7final.

/be

Comment 19

[Brian Ryner (not reading)]

checked in.