Closed Bug 1389435 (gcc6) Opened 2 years ago Closed 2 years ago

Use GCC 6 to build what we ship on Linux

Categories

(Firefox Build System :: General, enhancement)

enhancement
Not set

Tracking

(firefox57 fixed)

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(3 files)

No description provided.
Depends on: 1389436
Depends on: 1390704
Depends on: 1390752
Depends on: 1390776
Comment on attachment 8897702 [details]
Bug 1389435 - Use GCC 6 to build the Firefox we ship on Linux.

https://reviewboard.mozilla.org/r/169000/#review174474

Well, that was easy.  I like this a lot better than fiddling with tooltool manifests!
Attachment #8897702 - Flags: review?(nfroyd) → review+
Oh, you might have done this already, but do you have talos numbers and/or size comparisons?
Flags: needinfo?(mh+mozilla)
Comment on attachment 8897701 [details]
Bug 1389435 - Explicitly use GCC 4.9 for hazard builds.

https://reviewboard.mozilla.org/r/168998/#review174478

This is kind of unfortunate; we're not that far from moving to C++14 for Gecko, and GCC 4.9's support for C++14, while there, is not nearly as good as it could be.  Do we have any ETA or even a back-of-the-envelope calculation for moving the hazard builds to something newer?
That's a question for sfink. As for C++14, isn't android still blocking us anyways? Then there's also the question whether it's okay that this results in some long term support distros likely building with clang (because they'd already need a backport of clang, and might as well just backport that instead of that *and* gcc), which would mean a slower Firefox.
Flags: needinfo?(mh+mozilla)
(In reply to Nathan Froyd [:froydnj] from comment #5)
> Oh, you might have done this already, but do you have talos numbers and/or
> size comparisons?

https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=d52946df58d2&newProject=try&newRevision=0371912c1766&framework=1&showOnlyImportant=0
(In reply to Mike Hommey [:glandium] from comment #7)
> That's a question for sfink. As for C++14, isn't android still blocking us
> anyways? Then there's also the question whether it's okay that this results
> in some long term support distros likely building with clang (because they'd
> already need a backport of clang, and might as well just backport that
> instead of that *and* gcc), which would mean a slower Firefox.

Android support is basically there; all the patches necessary to build with clang (although not in the most user-friendly way, see bug 1390640, for instance) exist in my local tree.  I'm trying to push forward so we can build with clang in 57.

I have no comment on what the distros do.  I'm not even sure they would perform the experiments necessary to conclude that a clang-built firefox would be bigger/slower/etc. than a GCC-built firefox.
(In reply to Mike Hommey [:glandium] from comment #8)
> (In reply to Nathan Froyd [:froydnj] from comment #5)
> > Oh, you might have done this already, but do you have talos numbers and/or
> > size comparisons?
> 
> https://treeherder.mozilla.org/perf.html#/
> compare?originalProject=try&originalRevision=d52946df58d2&newProject=try&newR
> evision=0371912c1766&framework=1&showOnlyImportant=0

Compiler warnings out the wazoo, but 1% win on installer size and mostly neutral on performance metrics?  I'll take it!
(In reply to Nathan Froyd [:froydnj] from comment #10)
> (In reply to Mike Hommey [:glandium] from comment #8)
> > (In reply to Nathan Froyd [:froydnj] from comment #5)
> > > Oh, you might have done this already, but do you have talos numbers and/or
> > > size comparisons?
> > 
> > https://treeherder.mozilla.org/perf.html#/
> > compare?originalProject=try&originalRevision=d52946df58d2&newProject=try&newR
> > evision=0371912c1766&framework=1&showOnlyImportant=0
> 
> Compiler warnings out the wazoo

Mmm I had missed that because I didn't look at the numbers for pgo builds. They're all -Wcoverage-mismatch warnings, which are actually suspicious. Could be a sccache problem.
Comment on attachment 8897700 [details]
Bug 1389435 - Forbid toolchain dependencies on aliases.

https://reviewboard.mozilla.org/r/168996/#review174516
Attachment #8897700 - Flags: review?(dustin) → review+
Depends on: 1391533
Sfink being out on PTO, I'm going to land this with r=me. The patch is just a trivial formality anyways, that does nothing in practice.
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/78b9a177433e
Forbid toolchain dependencies on aliases. r=dustin
https://hg.mozilla.org/integration/mozilla-inbound/rev/7de11f6fe7cf
Explicitly use GCC 4.9 for hazard builds. r=me
https://hg.mozilla.org/integration/mozilla-inbound/rev/db36870cee64
Use GCC 6 to build the Firefox we ship on Linux. r=froydnj
Depends on: 1392868
These build metrics regressions showed up. Can they be addressed?

== Change summary for alert #8912 (as of August 22 2017 04:49 UTC) ==

Regressions:

 44%  compiler warnings summary linux64-noopt debug      282.00 -> 406.00
 36%  compiler warnings summary linux64 debug            401.00 -> 546.75
 34%  compiler warnings summary linux32 debug            401.00 -> 536.75
 24%  compiler warnings summary linux32 opt              455.00 -> 566.00
 23%  compiler warnings summary linux64 opt valgrind     472.00 -> 581.00
 23%  compiler warnings summary linux64 opt              475.00 -> 582.00

Improvements:

 14%  compiler warnings summary linux64 pgo      1,444.17 -> 1,238.83
 13%  compiler warnings summary linux32 pgo      1,402.25 -> 1,218.17

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=8912
Flags: needinfo?(mh+mozilla)
The PGO warnings improvements are not really expected, but the regressions are pretty much expected: newer versions of compilers find new problems that previous versions didn't know about.

There is nothing more to do here than the global ongoing effort to remove warnings.
Flags: needinfo?(mh+mozilla)
I'm surprised you haven't found talos improvements, though.
As a matter of fact, there are lots of Talos improvements now. I'm triaging them right now.
== Change summary for alert #8931 (as of August 22 2017 04:49 UTC) ==

Improvements:

  3%  tsvgr_opacity summary linux64 opt e10s     372.72 -> 361.68

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=8931
Comment on attachment 8897701 [details]
Bug 1389435 - Explicitly use GCC 4.9 for hazard builds.

https://reviewboard.mozilla.org/r/168998/#review177580

Yeah, no problem with this, and no problem with landing things like this without waiting for review.
Attachment #8897701 - Flags: review?(sphink) → review+
(In reply to Nathan Froyd [:froydnj] from comment #6)
> Comment on attachment 8897701 [details]
> Bug 1389435 - Explicitly use GCC 4.9 for hazard builds.
> 
> https://reviewboard.mozilla.org/r/168998/#review174478
> 
> This is kind of unfortunate; we're not that far from moving to C++14 for
> Gecko, and GCC 4.9's support for C++14, while there, is not nearly as good
> as it could be.  Do we have any ETA or even a back-of-the-envelope
> calculation for moving the hazard builds to something newer?

I have been upgrading as needed (which so far has usually been due to gcc bugs that cause crashes when traversing internal data structures within a plugin). The upgrade path has historically been a PITA because I've had to match compilation and CI execution environments, which has held back a number of improvements because I didn't want to deal with it. With glandium's work, I should be able to convert this to a straightforward toolchain taskcluster dependency, and it will get dramatically easier.

That said, there will definitely be some work to upgrade the plugin to gcc 6; there's been something for every version so far. (Typically, I will have the upgrade running locally for quite a while before biting the bullet and upgrading CI.) I don't know how much work that will be, but I can do it in the context of creating the toolchain taskcluster task.
Product: Core → Firefox Build System
Duplicate of this bug: 1319233
You need to log in before you can comment on or make changes to this bug.