Default Rust optimization level decreased from 2 to 1

490 views
Skip to first unread message

Gregory Szorc

unread,
Oct 25, 2017, 1:35:13 PM10/25/17
to dev-platform
Compiling Rust code with optimizations is significantly slower than
compiling without optimizations. As was measured in bug 1411081, the
difference between rustc's -Copt-level 1 and 2 on an i7-6700K (4+4 cores)
for a recent revision of mozilla-central was 325s/802s wall/CPU versus
625s/1282s. This made Rust compilation during Firefox builds stand out as a
long pole and significantly slowed down builds.

Because we couldn't justify the benefits of level 2 for the build time
overhead it added, we've changed the build system default so Rust is
compiled with -Copt-level=1 (instead of 2).

Adding --enable-release to your mozconfig (the configuration for builds we
ship to users) enables -Copt-level=2. (i.e. we didn't change optimization
settings for builds we ship to users.)

Adding --disable-optimize sets to -Copt-level=0. (This behavior is
unchanged.)

If you want explicit control over -Copt-level, you can `export
RUSTC_OPT_LEVEL=<value>` in your mozconfig and that value will always be
used. --enable-release implies a number of other changes. So if you just
want to restore the old build system behavior, set this variable in your
mozconfig.

Also, due to ongoing work around Rust integration in the build system, it
is dangerous to rely on manual `cargo` invocations to compile Rust because
bypassing the build system (not using `mach build`) may not use the same
set of RUSTFLAGS that direct `cargo` invocations do. Things were mostly in
sync before. But this change and anticipated future changes will cause more
drift. If you want the correct behavior, use `mach`.

Boris Zbarsky

unread,
Oct 25, 2017, 1:59:35 PM10/25/17
to
On 10/25/17 1:34 PM, Gregory Szorc wrote:
> Also, due to ongoing work around Rust integration in the build system, it
> is dangerous to rely on manual `cargo` invocations to compile Rust because
> bypassing the build system (not using `mach build`) may not use the same
> set of RUSTFLAGS that direct `cargo` invocations do. Things were mostly in
> sync before. But this change and anticipated future changes will cause more
> drift. If you want the correct behavior, use `mach`.

Is something like "mach cargo-geckolib check" from within the servo/
directory still expected to work sanely?

In general, it would be good to have a way to do a "check" on Rust code
rather than a full compile with code generation when all you want is to
find out all the ways in which your code is going to fail to compile...


-Boris

Emilio Cobos Álvarez

unread,
Oct 25, 2017, 2:51:19 PM10/25/17
to Boris Zbarsky, dev-pl...@lists.mozilla.org


On 10/25/2017 07:59 PM, Boris Zbarsky wrote:
> On 10/25/17 1:34 PM, Gregory Szorc wrote:
>> Also, due to ongoing work around Rust integration in the build system, it
>> is dangerous to rely on manual `cargo` invocations to compile Rust
>> because
>> bypassing the build system (not using `mach build`) may not use the same
>> set of RUSTFLAGS that direct `cargo` invocations do. Things were
>> mostly in
>> sync before. But this change and anticipated future changes will cause
>> more
>> drift. If you want the correct behavior, use `mach`.
>
> Is something like "mach cargo-geckolib check" from within the servo/
> directory still expected to work sanely?

That's from the servo/ tree, right?

>From mozilla-central I've been using ./mach cargo check gkrust, and I
certainly expect it to keep working.

-- Emilio

Gregory Szorc

unread,
Oct 25, 2017, 3:03:43 PM10/25/17
to Emilio Cobos Álvarez, Boris Zbarsky, dev-platform
On Wed, Oct 25, 2017 at 11:50 AM, Emilio Cobos Álvarez <emi...@crisal.io>
wrote:

>
>
> On 10/25/2017 07:59 PM, Boris Zbarsky wrote:
> > On 10/25/17 1:34 PM, Gregory Szorc wrote:
> >> Also, due to ongoing work around Rust integration in the build system,
> it
> >> is dangerous to rely on manual `cargo` invocations to compile Rust
> >> because
> >> bypassing the build system (not using `mach build`) may not use the same
> >> set of RUSTFLAGS that direct `cargo` invocations do. Things were
> >> mostly in
> >> sync before. But this change and anticipated future changes will cause
> >> more
> >> drift. If you want the correct behavior, use `mach`.
> >
> > Is something like "mach cargo-geckolib check" from within the servo/
> > directory still expected to work sanely?
>
> That's from the servo/ tree, right?
>
> From mozilla-central I've been using ./mach cargo check gkrust, and I
> certainly expect it to keep working.
>

There is a `mach cargo` defined in both
python/mozbuild/mozbuild/mach_commands.py and
servo/python/servo/mach_commands.py. The former is accessed via `mach` and
the latter via `servo/mach`.

mozilla-central's `mach cargo` goes through the Firefox build system to
invoke cargo and appears to do the right thing.

Boris Zbarsky

unread,
Oct 25, 2017, 4:04:34 PM10/25/17
to
On 10/25/17 2:50 PM, Emilio Cobos Álvarez wrote:
> From mozilla-central I've been using ./mach cargo check gkrust, and I
> certainly expect it to keep working.

Ah, excellent.

-Boris

Gregory Szorc

unread,
Oct 26, 2017, 2:36:07 PM10/26/17
to Gregory Szorc, dev-platform
On Wed, Oct 25, 2017 at 10:34 AM, Gregory Szorc <g...@mozilla.com> wrote:

> Compiling Rust code with optimizations is significantly slower than
> compiling without optimizations. As was measured in bug 1411081, the
> difference between rustc's -Copt-level 1 and 2 on an i7-6700K (4+4 cores)
> for a recent revision of mozilla-central was 325s/802s wall/CPU versus
> 625s/1282s. This made Rust compilation during Firefox builds stand out as a
> long pole and significantly slowed down builds.
>
> Because we couldn't justify the benefits of level 2 for the build time
> overhead it added, we've changed the build system default so Rust is
> compiled with -Copt-level=1 (instead of 2).
>
> Adding --enable-release to your mozconfig (the configuration for builds we
> ship to users) enables -Copt-level=2. (i.e. we didn't change optimization
> settings for builds we ship to users.)
>
> Adding --disable-optimize sets to -Copt-level=0. (This behavior is
> unchanged.)
>
> If you want explicit control over -Copt-level, you can `export
> RUSTC_OPT_LEVEL=<value>` in your mozconfig and that value will always be
> used. --enable-release implies a number of other changes. So if you just
> want to restore the old build system behavior, set this variable in your
> mozconfig.
>
> Also, due to ongoing work around Rust integration in the build system, it
> is dangerous to rely on manual `cargo` invocations to compile Rust because
> bypassing the build system (not using `mach build`) may not use the same
> set of RUSTFLAGS that direct `cargo` invocations do. Things were mostly in
> sync before. But this change and anticipated future changes will cause more
> drift. If you want the correct behavior, use `mach`.
>

Heads up: the code change uncovered a subtle bug in sccache's argument
parser. If you see "error: codegen option `opt-level` requires a string (C
opt-level=<value>)", you've hit it.

Until the fix merges around, the workaround is to disable sccache or graft
https://hg.mozilla.org/integration/autoland/rev/bc103ec9d0b3.

Sorry for any disruption. Caching and argument parsing are hard :/

Jeff Muizelaar

unread,
Oct 26, 2017, 2:51:48 PM10/26/17
to Gregory Szorc, dev-platform
FWIW, WebRender becomes unusable opt-level=1. It also looks like style
performance takes quite a hit as well which means that our default
developer builds become unusable for performance work. I worry that
people will forget this and end up rediscovering only when they look
at profiles (as mstange just did). What's the use case for a
--enable-optimize, opt-level=1 build?

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

Gregory Szorc

unread,
Oct 26, 2017, 3:09:20 PM10/26/17
to Jeff Muizelaar, dev-platform, Gregory Szorc
On Thu, Oct 26, 2017 at 11:51 AM, Jeff Muizelaar <jmuiz...@mozilla.com>
wrote:

> FWIW, WebRender becomes unusable opt-level=1. It also looks like style
> performance takes quite a hit as well which means that our default
> developer builds become unusable for performance work. I worry that
> people will forget this and end up rediscovering only when they look
> at profiles (as mstange just did). What's the use case for a
> --enable-optimize, opt-level=1 build?
>

Yes, this was a painful trade-off. I acknowledge we still may not have the
proper set of defaults or tweakable options in play.

Currently, --enable-optimize is the default and it is tied to both C/C++
and Rust (Rust inherited the option).

Would it help if we had a separate --enable-optimize-rust (or similar)
option to control Rust optimizations so we have separate knobs? If we did
that, --disable-optimize-rust could be opt-level 0 or 1 and
--enable-optimize-rust could be opt-level=2. The local defaults would
probably be --enable-optimize/--disable-optimize-rust (mirroring today).

I'm not sure if it is possible, but per-crate optimization levels might
help. Although, the data shows that the style crate is one of the slowest
to compile. And, this crate's optimization is also apparently very
important for accurate performance testing. That's a really unfortunate
conflict to have and it would be terrific if we could make the style crate
compile faster so this conflict goes away. I've filed bug 1412077 to track
improvements here.

Jeff Muizelaar

unread,
Oct 26, 2017, 3:20:04 PM10/26/17
to Gregory Szorc, dev-platform
On Thu, Oct 26, 2017 at 3:08 PM, Gregory Szorc <g...@mozilla.com> wrote:
> Would it help if we had a separate --enable-optimize-rust (or similar)
> option to control Rust optimizations so we have separate knobs? If we did
> that, --disable-optimize-rust could be opt-level 0 or 1 and
> --enable-optimize-rust could be opt-level=2. The local defaults would
> probably be --enable-optimize/--disable-optimize-rust (mirroring today).

Yeah, that would probably be more user friendly than the environment
variable solution that we have today. Still it's hard to know what the
correct defaults are.

> I'm not sure if it is possible, but per-crate optimization levels might
> help. Although, the data shows that the style crate is one of the slowest to
> compile. And, this crate's optimization is also apparently very important
> for accurate performance testing. That's a really unfortunate conflict to
> have and it would be terrific if we could make the style crate compile
> faster so this conflict goes away. I've filed bug 1412077 to track
> improvements here.

Hopefully the thinlto work that Alex is doing
(https://internals.rust-lang.org/t/help-test-out-thinlto/6017) will
make a difference here.

-Jeff

Boris Zbarsky

unread,
Oct 26, 2017, 3:27:20 PM10/26/17
to
On 10/26/17 2:51 PM, Jeff Muizelaar wrote:
> What's the use case for a
> --enable-optimize, opt-level=1 build?

Fwiw, I ended up doing a fair amount of my work recently in a
"--enable-optimize, --disable-debug, --enable-rust-debug" build, for the
following reasons:

1) I was modifying Rust style system code a lot and wanted a sane
compile time for incremental modifications. The time it took to build a
fully-optimized gkrust was prohibitive.

2) I was running tests locally on my modifications, and debug build
startup time in our test harnesses is horrible, to the point where
compiling my changes would take less time than running a single reftest.
Hence the --enable-optimize.

So that's a use case...

I agree that I constantly had to remember to never ever do any
performance measurement in that build; I treated it like I do my debug
builds in terms of what things I did with it, basically.

-Boris

Markus Stange

unread,
Oct 30, 2017, 1:23:42 PM10/30/17
to
On 2017-10-25 1:34 PM, Gregory Szorc wrote:
> Adding --enable-release to your mozconfig (the configuration for builds we
> ship to users) enables -Copt-level=2. (i.e. we didn't change optimization
> settings for builds we ship to users.)

I've added a note about this to our benchmarking instructions at
https://developer.mozilla.org/en-US/docs/Mozilla/Benchmarking#Rust_optimization_level
.

-Markus

Randell Jesup

unread,
Oct 31, 2017, 2:31:03 PM10/31/17
to
Note that this means that profiles taken with local builds will be "off"
compared to Nightly/etc, unless you rebuild with extra options. How
much off? probably not a lot unless you're super-focused on Rust code,
but that's just a guess.

--
Randell Jesup, Mozilla Corp
remove "news" for personal email

Jeff Muizelaar

unread,
Oct 31, 2017, 3:02:32 PM10/31/17
to Gregory Szorc, dev-platform
As another piece of evidence in support opt-level=1 being the wrong
default, Glenn also got bitten profiling with the wrong options.
https://github.com/servo/webrender/issues/1817#issuecomment-340553613

-Jeff

On Thu, Oct 26, 2017 at 2:51 PM, Jeff Muizelaar <jmuiz...@mozilla.com> wrote:
> FWIW, WebRender becomes unusable opt-level=1. It also looks like style
> performance takes quite a hit as well which means that our default
> developer builds become unusable for performance work. I worry that
> people will forget this and end up rediscovering only when they look
> at profiles (as mstange just did). What's the use case for a
> --enable-optimize, opt-level=1 build?
>
> -Jeff
>
> On Wed, Oct 25, 2017 at 1:34 PM, Gregory Szorc <g...@mozilla.com> wrote:
>> Compiling Rust code with optimizations is significantly slower than
>> compiling without optimizations. As was measured in bug 1411081, the
>> difference between rustc's -Copt-level 1 and 2 on an i7-6700K (4+4 cores)
>> for a recent revision of mozilla-central was 325s/802s wall/CPU versus
>> 625s/1282s. This made Rust compilation during Firefox builds stand out as a
>> long pole and significantly slowed down builds.
>>
>> Because we couldn't justify the benefits of level 2 for the build time
>> overhead it added, we've changed the build system default so Rust is
>> compiled with -Copt-level=1 (instead of 2).
>>
>> Adding --enable-release to your mozconfig (the configuration for builds we
>> ship to users) enables -Copt-level=2. (i.e. we didn't change optimization
>> settings for builds we ship to users.)
>>

Simon Sapin

unread,
Oct 31, 2017, 3:04:55 PM10/31/17
to dev-pl...@lists.mozilla.org
On 31/10/17 19:30, Randell Jesup wrote:
> Note that this means that profiles taken with local builds will be "off"
> compared to Nightly/etc, unless you rebuild with extra options. How
> much off? probably not a lot unless you're super-focused on Rust code,
> but that's just a guess.

This was also the case before this change if you did not use
--enable-release. LTO can make a significant difference. (I’ve seen -10%
time in some microbenchmarks.)

--
Simon Sapin

Gregory Szorc

unread,
Oct 31, 2017, 3:22:34 PM10/31/17
to Jeff Muizelaar, dev-platform, Gregory Szorc
On Tue, Oct 31, 2017 at 12:02 PM, Jeff Muizelaar <jmuiz...@mozilla.com>
wrote:

> As another piece of evidence in support opt-level=1 being the wrong
> default, Glenn also got bitten profiling with the wrong options.
> https://github.com/servo/webrender/issues/1817#issuecomment-340553613


It is "wrong" for the set of "people performing profiling." This set is
different from "people compiling Gecko." Which is different from "people
who actually need to compile Gecko." What I'm trying is the new default is
"not wrong" for a large set of people who aren't "people performing
profiling."

My opinion is that this class of failure is a UX failure revolving around
the question of "am I building the thing that should be used for X?" The
build workflow does a horrible job of asking this critical question. The
closest we have is `mach bootstrap` asking if you are building {Firefox,
Fennec} {with, without} artifact builds. And we don't do a good job of
transitioning between `mach bootstrap` or forcing people to run it at all.
So we essentially have nothing forcing people to answer a question that has
critical implications for their ability to adequately develop Firefox.

I have ideas for solving this problem. But to do it in a way that results
in fewer people falling through "my build configuration is sub-optimal and
I didn't know it" cracks, we need to eliminate the concept of a "default
build configuration" and require people to specify a build configuration
(by creating a mozconfig file, etc). This is because the reality is that
there is no single "default build configuration" that is ideal for
everyone. There are more details and some discussion in bug 1410262. I'd
rather we morph that bug as needed than discuss on this mailing list. I'm
happy to discuss on this mailing list eventually: I just want it to be
after we have general agreement on a concrete proposal.

Kris Maglione

unread,
Oct 31, 2017, 3:35:55 PM10/31/17
to Gregory Szorc, Jeff Muizelaar, dev-platform
On Tue, Oct 31, 2017 at 12:21:42PM -0700, Gregory Szorc wrote:
>On Tue, Oct 31, 2017 at 12:02 PM, Jeff Muizelaar <jmuiz...@mozilla.com>
>wrote:
>
>> As another piece of evidence in support opt-level=1 being the wrong
>> default, Glenn also got bitten profiling with the wrong options.
>> https://github.com/servo/webrender/issues/1817#issuecomment-340553613
>
>It is "wrong" for the set of "people performing profiling." This set is
>different from "people compiling Gecko." Which is different from "people
>who actually need to compile Gecko." What I'm trying is the new default is
>"not wrong" for a large set of people who aren't "people performing
>profiling."

I'll second this. For months I've had Servo and WebRender disabled in
most of my builds because they made my compiles so much slower. I tried
re-enabling them a few weeks ago when I got a new workstation, but gave
up after most of the compile finished in 7 minutes, and then cargo spent
another 8 minutes compiling on a single core. I tried some hacks to bump
the codegen-units setting, but it was heinously difficult, and didn't
help enough, so I gave up.

I suspect I'm not in the minority here.

After this change, I'm considering re-enabling Servo for my development
builds.


I'll also second the notion that we need better build system support for
helping people select the right configuration for the task that they're
working on, but I think this is a better default for the general case.
--
Kris Maglione
Senior Firefox Add-ons Engineer
Mozilla Corporation

If there was anything that depressed him more than his own cynicism,
it was that quite often it still wasn’t as cynical as real life.
--Terry Pratchett

Jeff Muizelaar

unread,
Oct 31, 2017, 3:39:50 PM10/31/17
to Gregory Szorc, dev-platform
On Tue, Oct 31, 2017 at 3:21 PM, Gregory Szorc <g...@mozilla.com> wrote:
> On Tue, Oct 31, 2017 at 12:02 PM, Jeff Muizelaar <jmuiz...@mozilla.com>
> wrote:
>>
>> As another piece of evidence in support opt-level=1 being the wrong
>> default, Glenn also got bitten profiling with the wrong options.
>> https://github.com/servo/webrender/issues/1817#issuecomment-340553613
>
>
> It is "wrong" for the set of "people performing profiling." This set is
> different from "people compiling Gecko." Which is different from "people who
> actually need to compile Gecko." What I'm trying is the new default is "not
> wrong" for a large set of people who aren't "people performing profiling."

I say this a bit tongue-in-cheek, but given our big performance push,
hopefully the set of people who aren't profiling is not that large.

-Jeff

Ben Kelly

unread,
Oct 31, 2017, 3:50:55 PM10/31/17
to dev-platform
Would it be possible to have our profiling tools warn if about:compiler
optimization flags are not in about:buildconfig?

On Tue, Oct 31, 2017 at 3:39 PM, Jeff Muizelaar <jmuiz...@mozilla.com>
wrote:

> On Tue, Oct 31, 2017 at 3:21 PM, Gregory Szorc <g...@mozilla.com> wrote:
> > On Tue, Oct 31, 2017 at 12:02 PM, Jeff Muizelaar <jmuiz...@mozilla.com
> >
> > wrote:
> >>
> >> As another piece of evidence in support opt-level=1 being the wrong
> >> default, Glenn also got bitten profiling with the wrong options.
> >> https://github.com/servo/webrender/issues/1817#issuecomment-340553613
> >
> >
> > It is "wrong" for the set of "people performing profiling." This set is
> > different from "people compiling Gecko." Which is different from "people
> who
> > actually need to compile Gecko." What I'm trying is the new default is
> "not
> > wrong" for a large set of people who aren't "people performing
> profiling."
>
> I say this a bit tongue-in-cheek, but given our big performance push,
> hopefully the set of people who aren't profiling is not that large.
>
> -Jeff

Kris Maglione

unread,
Oct 31, 2017, 3:51:43 PM10/31/17
to Jeff Muizelaar, dev-platform, Gregory Szorc
On Tue, Oct 31, 2017 at 03:39:39PM -0400, Jeff Muizelaar wrote:
>On Tue, Oct 31, 2017 at 3:21 PM, Gregory Szorc <g...@mozilla.com> wrote:
>> It is "wrong" for the set of "people performing profiling." This set is
>> different from "people compiling Gecko." Which is different from "people who
>> actually need to compile Gecko." What I'm trying is the new default is "not
>> wrong" for a large set of people who aren't "people performing profiling."
>
>I say this a bit tongue-in-cheek, but given our big performance push,
>hopefully the set of people who aren't profiling is not that large.

I profile when I need to profile (which, if I had to guess, is probably
more often than the majority of our engineers), but I don't need to
profile the vast majority of my builds. And for the ones that I do
profile, the performance of our Rust code is usually not what's
interesting to me. I expect that the story is different for people who
mostly work in layout code, but that is a very small minority of us.

What matters more most of the time is that my builds finish quickly.
Even if I have to occasionally do a clobber build with a different
configuration, the time and energy saved by most of my builds being
significantly faster easily pays for it.

Boris Zbarsky

unread,
Oct 31, 2017, 6:20:30 PM10/31/17
to
On 10/31/17 3:21 PM, Gregory Szorc wrote:
> It is "wrong" for the set of "people performing profiling."

Just to be clear, that set is "everyone".

Now the set of "people performing profiling on builds they create
themselves" is a separate question....

-Boris

Emilio Cobos Álvarez

unread,
Apr 25, 2018, 11:36:00 AM4/25/18
to dev-pl...@lists.mozilla.org
There's a fair amount of people bitten by this constantly, which see
long style profiling markers and what's really happening is that they're
profiling a local opt build, and thus the Rust code in style has barely
any optimization and is slow.

I know that shouldn't be a thing, and that people should
--enable-release for profiling and all that. But given it happens, could
we consider reverting this change?

-- Emilio

Gregory Szorc

unread,
Apr 25, 2018, 12:11:56 PM4/25/18
to Emilio Cobos Álvarez, dev-pl...@lists.mozilla.org


> On Apr 25, 2018, at 08:35, Emilio Cobos Álvarez <emi...@crisal.io> wrote:
>
> There's a fair amount of people bitten by this constantly, which see long style profiling markers and what's really happening is that they're profiling a local opt build, and thus the Rust code in style has barely any optimization and is slow.
>
> I know that shouldn't be a thing, and that people should --enable-release for profiling and all that. But given it happens, could we consider reverting this change?

I’m going to assert that the set of people who care about fast builds is greater than the set of people who care about profiling the style system on local builds. Do you disagree? Or is the slowness of the style system undermining the ability for the local build to be usable/useful in general?

Regardless, I think the most productive conversation we can have is about how we can make the Firefox development UI guide people towards an optimal build configuration. The reality is that there are several “factions” of developers that each want different things from a build. There is no one-build-config-fits-all.

The build peers have long thought about adding the concept of “build profiles” to the build system. Think of them as in-tree mozconfigs for common, supported scenarios.

But without a forcing function making people choose an initial “profile,” people will continue to get bit by whatever default we choose.

We once printed a fatal message on first invocation of the build system. This was intended to be such a forcing function. But there was popular revolt and this feature was removed. Considering that I managed to upset a lot of people with this feature, I’m reluctant to go down that path again because it would seemingly undermine general support for the build system team and jeopardize our ability to make meaningful changes.

Emilio Cobos Álvarez

unread,
Apr 25, 2018, 12:23:46 PM4/25/18
to dev-pl...@lists.mozilla.org


On 4/25/18 6:11 PM, Gregory Szorc wrote:
>
>
>> On Apr 25, 2018, at 08:35, Emilio Cobos Álvarez <emi...@crisal.io> wrote:
>>
>> There's a fair amount of people bitten by this constantly, which see long style profiling markers and what's really happening is that they're profiling a local opt build, and thus the Rust code in style has barely any optimization and is slow.
>>
>> I know that shouldn't be a thing, and that people should --enable-release for profiling and all that. But given it happens, could we consider reverting this change?
>
> I’m going to assert that the set of people who care about fast builds is greater than the set of people who care about profiling the style system on local builds. Do you disagree? Or is the slowness of the style system undermining the ability for the local build to be usable/useful in general?

This is not about people profiling the style system, I couldn't disagree
with that.

This is about people doing general profiling, or profiling unrelated
stuff, which see that the style system takes half of the time on their
testcase, when in proper release builds it's a minor amount of the total
time.

Bobby Holley

unread,
Apr 25, 2018, 12:26:45 PM4/25/18
to Emilio Cobos Álvarez, dev-platform
Could we instead have the profiler UI throw up a warning if the build was
not compiled with --enable-release?

On Wed, Apr 25, 2018 at 9:23 AM, Emilio Cobos Álvarez <emi...@crisal.io>

Jeff Muizelaar

unread,
Apr 25, 2018, 12:32:45 PM4/25/18
to Emilio Cobos Álvarez, Mozilla
At minimum we should make --enable-profiling build with rust-opt.

-Jeff

On Wed, Apr 25, 2018 at 11:35 AM, Emilio Cobos Álvarez <emi...@crisal.io> wrote:
> There's a fair amount of people bitten by this constantly, which see long
> style profiling markers and what's really happening is that they're
> profiling a local opt build, and thus the Rust code in style has barely any
> optimization and is slow.
>
> I know that shouldn't be a thing, and that people should --enable-release
> for profiling and all that. But given it happens, could we consider
> reverting this change?
>
> -- Emilio

Bobby Holley

unread,
Apr 25, 2018, 12:38:48 PM4/25/18
to Jeff Muizelaar, Emilio Cobos Álvarez, Mozilla
I think that makes sense as a default, with the ability to explicitly
--disable-release if people are profiling something specific, know what
they're doing, and want faster builds.

On Wed, Apr 25, 2018 at 9:32 AM, Jeff Muizelaar <jmuiz...@mozilla.com>
wrote:

Ted Mielczarek

unread,
Apr 25, 2018, 1:22:12 PM4/25/18
to Jeff Muizelaar, Emilio Cobos Álvarez, Mozilla
On Wed, Apr 25, 2018, at 12:32 PM, Jeff Muizelaar wrote:
> At minimum we should make --enable-profiling build with rust-opt.

This sounds reasonable, although the quirk is that we default --enable-profiling on for nightly[1], so anyone building m-c will have it enabled. We could make the build system only do this for "explicitly enabled --enable-profiling", it just might be slightly confusing. (For reference, the rustc opt level is controlled here[2].)

IIRC the slowdown from using opt-level=2 vs. 1 is primarily in LLVM? It would probably be useful if we instrumented the build to record compilation times for all of the crates we build at various optimization levels and see where the biggest issues are. Perhaps we could get someone on the Rust team to make some improvements if we know what the worst offenders are.

-Ted

1. https://dxr.mozilla.org/mozilla-central/rev/26e53729a10976f52e75efa44e17b5e054969fec/js/moz.configure#243
2. https://dxr.mozilla.org/mozilla-central/rev/26e53729a10976f52e75efa44e17b5e054969fec/build/moz.configure/toolchain.configure#1397

Bobby Holley

unread,
Apr 25, 2018, 1:39:05 PM4/25/18
to Ted Mielczarek, Jeff Muizelaar, Emilio Cobos Álvarez, Mozilla
On Wed, Apr 25, 2018 at 10:21 AM, Ted Mielczarek <t...@mielczarek.org> wrote:

> On Wed, Apr 25, 2018, at 12:32 PM, Jeff Muizelaar wrote:
> > At minimum we should make --enable-profiling build with rust-opt.
>
> This sounds reasonable, although the quirk is that we default
> --enable-profiling on for nightly[1], so anyone building m-c will have it
> enabled. We could make the build system only do this for "explicitly
> enabled --enable-profiling", it just might be slightly confusing. (For
> reference, the rustc opt level is controlled here[2].)
>

We could also change this default. In general, Nightly is usually fine for
profiling, so I think the set of people doing local builds for profiling is
small enough that we could reasonably expect them to set the flag in their
mozconfig.


>
> IIRC the slowdown from using opt-level=2 vs. 1 is primarily in LLVM? It
> would probably be useful if we instrumented the build to record compilation
> times for all of the crates we build at various optimization levels and see
> where the biggest issues are. Perhaps we could get someone on the Rust team
> to make some improvements if we know what the worst offenders are.
>

Well, there's the opt level thing, but there's also the much larger bit
that we don't do Rust LTO without --enable-release, which IIUC has a
substantial performance impact.



>
> -Ted
>
> 1. https://dxr.mozilla.org/mozilla-central/rev/
> 26e53729a10976f52e75efa44e17b5e054969fec/js/moz.configure#243
> 2. https://dxr.mozilla.org/mozilla-central/rev/
> 26e53729a10976f52e75efa44e17b5e054969fec/build/moz.
> configure/toolchain.configure#1397

smaug

unread,
Apr 25, 2018, 3:20:00 PM4/25/18
to Bobby Holley, Ted Mielczarek, Jeff Muizelaar, Emilio Cobos Álvarez
On 04/25/2018 08:38 PM, Bobby Holley wrote:
> On Wed, Apr 25, 2018 at 10:21 AM, Ted Mielczarek <t...@mielczarek.org> wrote:
>
>> On Wed, Apr 25, 2018, at 12:32 PM, Jeff Muizelaar wrote:
>>> At minimum we should make --enable-profiling build with rust-opt.
>>
>> This sounds reasonable, although the quirk is that we default
>> --enable-profiling on for nightly[1], so anyone building m-c will have it
>> enabled. We could make the build system only do this for "explicitly
>> enabled --enable-profiling", it just might be slightly confusing. (For
>> reference, the rustc opt level is controlled here[2].)
>>
>
> We could also change this default. In general, Nightly is usually fine for
> profiling, so I think the set of people doing local builds for profiling is
> small enough that we could reasonably expect them to set the flag in their
> mozconfig.


Somewhat off-topic, but we tried hard last year to increase the number of people doing profiling,
and Quantum Flow is still ongoing and performance as important as ever.
Profiling should be part of daily development work.
Because of that it would be great to have local builds profile-able by default.

But still, I kind of like the idea of build profiles. Something which screams in the terminal what kind of build one is about
to get. At the beginning of build process and I guess end too there would be a message hinting whether the build is good for profiling or so.

Henri Sivonen

unread,
Apr 26, 2018, 10:46:16 AM4/26/18
to dev-platform
On Wed, Apr 25, 2018 at 7:11 PM, Gregory Szorc <gsz...@mozilla.com> wrote:
> The build peers have long thought about adding the concept of “build profiles” to the build system. Think of them as in-tree mozconfigs for common, supported scenarios.

This would be good to have. It would also help if such mozconfigs had
comprehensive comments explaining how our release builds differ from
tooling defaults especially now that we have areas of code that are
developed outside a full Firefox build. For example, I A/B tested code
for performance using cargo bench and mistakenly thought that cargo's
"release" mode meant the same thing as Firefox "release" mode. I only
later learned that I had developed with opt-level=3 (cargo's default
for "release") and we ship with opt-level=2.

--
Henri Sivonen
hsiv...@hsivonen.fi
https://hsivonen.fi/

Julien Wajsberg

unread,
May 2, 2018, 9:54:02 AM5/2/18
to dev-pl...@lists.mozilla.org
Le 25/04/2018 à 18:26, Bobby Holley a écrit :
> Could we instead have the profiler UI throw up a warning if the build was
> not compiled with --enable-release?

I filed https://github.com/devtools-html/perf.html/issues/976 to discuss
about it.
Thanks,
--
Julien
Reply all
Reply to author
Forward
0 new messages