Closed Bug 1368649 Opened 7 years ago Closed 7 years ago

ubuntu dev ppa built without optimizations (-Os)

Categories

(Firefox Build System :: General, defect)

Product:
Firefox Build System
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox55 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: jtaylor.debian, Assigned: glandium)

Details

Attachments

(1 file)

Bug 1368649 - Default to -O2 when building on Linux without PGO.
59 bytes, text/x-review-board-request
gps
: review+
Details 
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:53.0) Gecko/20100101 Firefox/53.0
Build ID: 20170524174214

Steps to reproduce:

https://launchpad.net/~ubuntu-mozilla-daily/+archive/ubuntu/ppa/+build/12650133/+files/buildlog_ubuntu-artful-amd64.firefox-trunk_55.0~a1~hg20170528r361062-0ubuntu0.17.10.1~umd1_BUILDING.txt.gz


Actual results:

The firefox daily build ppa for ubuntu (https://launchpad.net/~ubuntu-mozilla-daily/+archive/ubuntu/ppa) is building firefox mostly with optimization level Os which is size optimization.
This makes firefox very slow, in particularly libyuv suffers greatly due to lacking inlining and vectorization on older cpus without ssse3.
The main debian archives are also affected, see https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=863672


Expected results:

Please build the firefox packages with better compiler optimizations enabled (O2 or O3).
Actually it appears that ppa is maintained by ubuntu itself, in that case this bug can probably be closed.
Though using Os does not appear to be a ubuntu/debian modification, maybe the defaults can be tuned better for third party distributions.
Component: Untriaged → Build Config
Product: Firefox → Core
Correct, Mozilla does not maintain this PPA.

I suspect the PPA maintainers aren't performing a fully optimal build because of resource constraints: it takes a lot of CPU to crank up the optimization. That's why it isn't enabled by default.
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Resolution: --- → INVALID
I asked around a bit, it is not due to resource constraints, it is because it is the default in your build system, in particular the one used by ubuntu/debian (the old-configure without using pgo).

This also affects Fedora and probably most other distributions. It would probably be a good idea to change the default so more people get a decently performing firefox.
It sounds like we may need to reach out to package maintainers for distros then. Thanks for the heads up.
No, we should change the default in configure. I did a try run, and the difference between -Os and -O2 is enormous.
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: INVALID → ---
Assignee: nobody → mh+mozilla
This is -Os vs. -O2:
https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=522eae797ae2&newProject=try&newRevision=d99889734dda&framework=1&showOnlyImportant=0

The only regressions are installer size because libxul ends up larger, and RSS, for the same reason.
And FWIW, -O2 vs. PGO:
https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=d99889734dda&newProject=try&newRevision=b327dc51d0d7&framework=1&showOnlyImportant=0

Now, for a little bit of history:
- bug 494095 replaced the default of -O2 with -O3 for OSX builds (where we never PGOed)
- bug 590181 changed the default of -Os to -O3 on Linux builds
- bug 655003 partially reverted that back to -Os on Linux non-PGO builds for two reasons: size and correctness (back then GCC would blow some things up with -O3).

-O2 makes things larger than -Os, but not as much as -O3 does. I think the trade-off is well worth it considering the talos results in comment 6.
Thanks for catching this. I should have known to not trust our defaults.

Will switching to -O2 impact local developer builds? e.g. will more people need debug-related flags in their mozconfig that could get away without them before due to fewer code optimizations? (Read: do we need to make an announcement when landing this?)
Comment on attachment 8873659 [details]
Bug 1368649 - Default to -O2 when building on Linux without PGO.

https://reviewboard.mozilla.org/r/145048/#review149000
Attachment #8873659 - Flags: review?(gps) → review+
I'll send a note to dev-platform, because it /can/ have an impact on debugging, although in practice -Os vs. -O2 should not matter that much. People tend to use --disable-optimize when they want something that is easier to debug. I'll take on the occasion to remind them that recent GCC supports -Og (in fact, all supported versions of GCC, also clang >= 4.0), and that --enable-optimize=-Og would probably be better than --disable-optimize for them.
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/d956ff86aaa6
Default to -O2 when building on Linux without PGO. r=gps
Attachment #8873659 - Flags: review+ → review?(gps)
Comment on attachment 8873659 [details]
Bug 1368649 - Default to -O2 when building on Linux without PGO.

https://reviewboard.mozilla.org/r/145048/#review149072
Attachment #8873659 - Flags: review?(gps) → review+
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8fdb9e30b6a7
Default to -O2 when building on Linux without PGO. r=gps
https://hg.mozilla.org/mozilla-central/rev/8fdb9e30b6a7
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
wow, huge perf improvements according to talos:
== Change summary for alert #7009 (as of June 02 2017 04:49 UTC) ==

Regressions:

  3%  tp5o_webext Main_RSS linux64 opt e10s     188,555,429.98 -> 193,448,888.04
  2%  tp5o Main_RSS linux64 opt e10s            187,202,557.19 -> 191,397,209.08

Improvements:

 43%  bloom_basic_ref summary linux64 opt e10s     562.38 -> 319.37
 43%  bloom_basic summary linux64 opt e10s         562.47 -> 319.44
 23%  a11yr summary linux64 opt e10s               766.30 -> 588.48
 23%  sessionrestore linux64 opt e10s              843.50 -> 653.25
 22%  sessionrestore_no_auto_restore linux64 opt e10s870.50 -> 682.67
 21%  dromaeo_dom summary linux64 opt e10s         1,636.04 -> 1,983.72
 19%  tsvgx summary linux64 opt e10s               430.37 -> 349.92
 16%  ts_paint linux64 opt e10s                    1,296.08 -> 1,090.50
 15%  tcanvasmark summary linux64 opt e10s         9,129.83 -> 10,498.00
 15%  tp5o_webext summary linux64 opt e10s         362.77 -> 308.67
 14%  tart summary linux64 opt e10s                6.70 -> 5.74
 14%  tp5o summary linux64 opt e10s                363.80 -> 313.17
 14%  tsvgr_opacity summary linux64 opt e10s       438.69 -> 377.84
 12%  cart summary linux64 opt e10s                29.15 -> 25.52
 12%  damp summary linux64 opt e10s                300.61 -> 265.60
 11%  dromaeo_css summary linux64 opt e10s         6,864.84 -> 7,653.53
  8%  tpaint linux64 opt e10s                      278.70 -> 255.57
  8%  tsvg_static summary linux64 opt e10s         95.92 -> 88.34
  7%  tresize linux64 opt e10s                     24.82 -> 23.15
  7%  tps summary linux64 opt e10s                 28.73 -> 26.80
  6%  ts_paint_webext linux64 opt e10s             1,443.08 -> 1,350.50

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=7009
Yeah, but that's not the build we ship. :)
oh well, a false sense of achievement
I guess this should also make most of our Linux test runs faster, which is nice.
It also cut ~500 compiler warnings from various builds: https://treeherder.mozilla.org/perf.html#/alerts?id=7001
(In reply to Gregory Szorc [:gps] from comment #22)
> It also cut ~500 compiler warnings from various builds:
> https://treeherder.mozilla.org/perf.html#/alerts?id=7001

That's not necessarily a good thing.
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: