1389435 - (gcc6) Use GCC 6 to build what we ship on Linux

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Comment 1

[Mike Hommey [:glandium]]

Attachment: Bug 1389435 - Forbid toolchain dependencies on aliases.

This extends bug 1386920 in a more general way on toolchain jobs.

Review commit: https://reviewboard.mozilla.org/r/168996/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/168996/

Comment 2

[Mike Hommey [:glandium]]

Attachment: Bug 1389435 - Explicitly use GCC 4.9 for hazard builds.

The GCC plugin used for hazard builds was built for GCC 4.9. Until a new
plugin is built by a toolchain job for the "default" version of GCC we
build Firefox with, the builds using the plugin need to stay with the
version the plugin was built for.

Review commit: https://reviewboard.mozilla.org/r/168998/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/168998/

Comment 3

[Mike Hommey [:glandium]]

Attachment: Bug 1389435 - Use GCC 6 to build the Firefox we ship on Linux.

Review commit: https://reviewboard.mozilla.org/r/169000/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/169000/

Comment 4

[Nathan Froyd [:froydnj]]

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!

Comment 5

[Nathan Froyd [:froydnj]]

Oh, you might have done this already, but do you have talos numbers and/or size comparisons?

Comment 6

[Nathan Froyd [:froydnj]]

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?

Comment 7

[Mike Hommey [:glandium]]

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.

Comment 8

[Mike Hommey [:glandium]]

(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

Comment 9

[Nathan Froyd [:froydnj]]

(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.

Comment 10

[Nathan Froyd [:froydnj]]

(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!

Comment 11

[Mike Hommey [:glandium]]

(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 12

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8897700 [details]
Bug 1389435 - Forbid toolchain dependencies on aliases.

https://reviewboard.mozilla.org/r/168996/#review174516

Comment 13

[Mike Hommey [:glandium]]

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.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
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

Comment 15

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/78b9a177433e
https://hg.mozilla.org/mozilla-central/rev/7de11f6fe7cf
https://hg.mozilla.org/mozilla-central/rev/db36870cee64

Comment 16

[Ionuț Goldan [:igoldan]]

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

Comment 17

[Mike Hommey [:glandium]]

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.

Comment 18

[Mike Hommey [:glandium]]

I'm surprised you haven't found talos improvements, though.

Comment 19

[Ionuț Goldan [:igoldan]]

As a matter of fact, there are lots of Talos improvements now. I'm triaging them right now.

Comment 20

[Joel Maher ( :jmaher ) (UTC -8)]

== 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 21

[Steve Fink [:sfink] [:s:]]

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.

Comment 22

[Steve Fink [:sfink] [:s:]]

(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.