Proposal to turn on frame pointers on Nightly desktop builds

[Ehsan Akhgari]

Hi everyone,

About a couple of years ago, if memory serves, we turned off frame
pointers in order to gain some performance. The performance
improvement was about 5% according to some sources (bug 492688). One
of the implications of this change was that it made profiling
nightlies harder since you needed to do your own optimized builds with
--enable-profiling in order to get any useful results out of the
profiler.

A couple of years has passed by, and now we have tools like the Gecko
Profiler <https://developer.mozilla.org/en/Performance/Profiling_with_the_Built-in_Profiler>
and the Chrome Hang Reporter
<http://blog.mozilla.org/vdjeric/2012/06/08/cache-plugin-font-operations-most-common-in-chrome-hang-reports/>
which need to be able to walk the stacks using frame pointers to be
useful. Back in January, I started to maintain a profiling branch
<https://groups.google.com/d/msg/mozilla.dev.platform/M4l5jl75dEs/-hJbnWegMgMJ>
which was simply Nightly + --enable-profiling in the mozconfig where
we could use these tools without having to do our own builds.

Now we're at a point where we need more people to be able to use these
builds, and therefore I'm proposing to add --enable-profiling to the
default mozconfigs for Nightly desktop builds. This change will help
developers focusing on performance tremendously, but it does have the
side effect that the Nightly builds will be a bit slower than Aurora,
Beta and Release, so those who need to compare performance numbers
across Nightly and other channel builds will need to create their own
builds. If the mobile team wants this in the future, they can enable
this separately in the future.

I'm planning to file a bug with a patch to do this this week, unless
someone has a very compelling reason why we should not do this.

Cheers,
--
Ehsan
<http://ehsanakhgari.org/>

[Kyle Huey]

> I'm planning to file a bug with a patch to do this this week, unless
> someone has a very compelling reason why we should not do this.
>

I'm mildly worried about testing code with frame pointers and shipping code
without them.

- Kyle

[Ehsan Akhgari]

This proposal is only for Nightly, so we'll get testing on builds on
Aurora and Beta which have more users.

Or did you mean running unit tests on builds without frame pointers?
FWIW I don't remember to have seen many bugs which are caused only by
the compiler emitting frame pointers.

--
Ehsan
<http://ehsanakhgari.org/>

[Kyle Huey]

On Mon, Jun 11, 2012 at 11:07 AM, Ehsan Akhgari <ehsan....@gmail.com>wrote:

> Or did you mean running unit tests on builds without frame pointers?
> FWIW I don't remember to have seen many bugs which are caused only by
> the compiler emitting frame pointers.
>

Yes, unit tests. I'm worried about Lukas pushing Nightly to Aurora, tests
failing because of the difference, and having to go through a 6 week
regression range.

- Kyle

[Ehsan Akhgari]

We can keep running tests on builds without frame pointers on the
profiling branch. Note that for the past 5 months we've effectively
been running the tests on both trees and we have not noticed problems
which only happen in one but not the other (except for merging
problems.)

--
Ehsan
<http://ehsanakhgari.org/>

[Benoit Girard]

This is the primary concern but I think we need to keep in mind the value
of performance data we stand to gain at the cost of perhaps of testing
something different.

As a data point making profiling easier played a key role in improving
graphics performance in Fennec.

> _______________________________________________
> dev-platform mailing list
> dev-pl...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>

[Benjamin Smedberg]

On 6/11/12 10:41 AM, Ehsan Akhgari wrote:
> Now we're at a point where we need more people to be able to use these
> builds, and therefore I'm proposing to add --enable-profiling to the
> default mozconfigs for Nightly desktop builds.

Overall I think this is a good move, but primarily because it can
significantly improve the chromehang/telemetry data. I don't think that
overall we'll get so many people actively using the profiler that the
win would be big enough by itself. I do wonder if we have plans to
automate profiling/telemetry in additional cases in the future which
would make the wins even bigger.

My primary concerns with this relate to regressions and tracking of
crash stats. In particular, we know that having frame pointers on will
alter the reported crash signatures that crash-stats sees. This means
that crash signatures may be different from nightly to Aurora. Also,
turning frame pointers on will mask bugs or changes in the breakpad
stack walking system, which doesn't have a lot of automated tests. So if
we do this, we need to be very careful about changes to build
configuration (compiler versions, build flags and PGO settings).

--BDS

[Ehsan Akhgari]

On Mon, Jun 11, 2012 at 3:13 PM, Benjamin Smedberg
<benj...@smedbergs.us> wrote:
> On 6/11/12 10:41 AM, Ehsan Akhgari wrote:
>>
>> Now we're at a point where we need more people to be able to use these
>> builds, and therefore I'm proposing to add --enable-profiling to the
>> default mozconfigs for Nightly desktop builds.
>
>
> Overall I think this is a good move, but primarily because it can
> significantly improve the chromehang/telemetry data. I don't think that
> overall we'll get so many people actively using the profiler that the win
> would be big enough by itself. I do wonder if we have plans to automate
> profiling/telemetry in additional cases in the future which would make the
> wins even bigger.

We do in fact have ideas on how we can use the profiler to build tools
which would allow us to gather better performance data from our users.
The current add-on already makes it very easy to file performance
bugs with the profile, and there's a lot more that we can do in this
area.

> My primary concerns with this relate to regressions and tracking of crash
> stats. In particular, we know that having frame pointers on will alter the
> reported crash signatures that crash-stats sees. This means that crash
> signatures may be different from nightly to Aurora.

This is a valid concern. Is there any way in Breakpad/Socorro to
sanitize the signatures? I know that it's currently possible to mark
functions as being non-significant in the stacks, but I don't know how
flexible that is.

> Also, turning frame
> pointers on will mask bugs or changes in the breakpad stack walking system,
> which doesn't have a lot of automated tests. So if we do this, we need to be
> very careful about changes to build configuration (compiler versions, build
> flags and PGO settings).

It's true. Is that something which would cause you to vote against
turning frame pointers on for Nightly?

[Benjamin Smedberg]

On 6/11/12 12:48 PM, Ehsan Akhgari wrote:
>
>> My primary concerns with this relate to regressions and tracking of crash
>> stats. In particular, we know that having frame pointers on will alter the
>> reported crash signatures that crash-stats sees. This means that crash
>> signatures may be different from nightly to Aurora.
> This is a valid concern. Is there any way in Breakpad/Socorro to
> sanitize the signatures? I know that it's currently possible to mark
> functions as being non-significant in the stacks, but I don't know how
> flexible that is.

It will be unpredictable, so I don't think that there's predictive
santization/normalization we could do. And it probably won't matter a
whole much unless we hit the appendlist a lot.

>
>> Also, turning frame
>> pointers on will mask bugs or changes in the breakpad stack walking system,
>> which doesn't have a lot of automated tests. So if we do this, we need to be
>> very careful about changes to build configuration (compiler versions, build
>> flags and PGO settings).
> It's true. Is that something which would cause you to vote against
> turning frame pointers on for Nightly?

No. I think we just need to be very careful to sanity-check our
crash-stats after the central->aurora merge.

--BDS

[Ehsan Akhgari]

I filed bug 764216 for this.

--
Ehsan
<http://ehsanakhgari.org/>

[Ehsan Akhgari]

OK, I just landed this change on mozilla-central. Please note that
this will incur some performance regression in Talos numbers, which is
expected.

Also, I repurposed the profiling branch to build without
--enable-profiling. I'm going to file some bugs to get it renamed to
something more descriptive. Important note: if you're currently
running the profiling branch nightlies, you probably want to switch to
the regular nightly builds now, since the profiling nightlies will no
longer be useful. I've filed bug 765814 to ask RelEng to do that, but
I'm not sure how long that will take to happen.

--
Ehsan
<http://ehsanakhgari.org/>