Closed Bug 1253064 Opened 9 years ago Closed 6 years ago

Prefer Clang to GCC in local developer builds

Categories

(Firefox Build System :: Toolchains, defect)

defect
Not set
normal

Tracking

(firefox62 fixed)

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: gps, Assigned: glandium)

References

Details

Attachments

(1 file)

A few weeks ago I was comparing mozilla-central build times on GCC versus Clang and the time difference is not insignificant: Clang is multiple minutes faster than GCC using the latest release of each. And I'm pretty sure Clang has been faster than GCC for several years now. I know our official Linux builds will be using GCC for the immediate future. I know it is important for us to have compiler coverage to ensure the build continues to work as many places as possible. I know we want local builds to resemble automation builds as closely as possible. That being said, waiting a few extra minutes for GCC builds to complete when you could be using Clang feels like avoidable overhead. Unless there is a compelling reason to stick with GCC, I propose we change the build system to prefer Clang over GCC on Linux, at least for local developer builds. I'll try to run the numbers again tonight when I have access to my beefy machine. I encourage others to post their results in addition to make sure we're not missing a scenario where GCC is faster than Clang.
From http://logs.glob.uno/?c=mozilla%23build&s=31+Dec+2015&e=31+Dec+2015&h=clang#c847: 04:52 gps build times for gcc 5.2.1 versus clang 3.6.2 on ubuntu 15.10: gcc - 126:35 CPU time; clang - 89:48 CPU time 04:52 gps wall time was 17:30 versus 12:54 That was likely a default build configuration.
Just for clarification, this bug is about Linux. Clang should be the default on OS X (if it isn't, it should be rare that OS X machines have GCC installed).
Here is some more data I generated tonight. Core i7-6700K (Skylake 4+4HT core @ 4.0GHz). 16 GB RAM. SSD HD. Ubuntu 15.10 64-bit. Using clang and gcc system packages from Canonical. Building changeset 4ea7408b3eef (tip of m-c from today) using `mach build -v` with an empty mozconfig. gold linker. No ccache. Terminal output redirected to a file to minimize impact of terminal buffering slowing down the build (which does happen). Ran each toolchain 2x (1 after the other) and took fastest result. This is an out-of-the-box configuration and likely represents what most people build with (assuming they've run `mach bootstrap`). Here's an example clang invocation: /usr/bin/clang++-3.7 -o Unified_cpp_dom_bindings0.o -c -I/home/gps/src/firefox/objdir/dist/stl_wrappers -I/home/gps/src/firefox/objdir/dist/system_wrappers -include /home/gps/src/firefox/config/gcc_hidden.h -DNDEBUG=1 -DTRIMMED=1 -DOS_POSIX=1 -DOS_LINUX=1 -DHAVE_SIDEBAR -DSTATIC_EXPORTABLE_JS_API -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/home/gps/src/firefox/dom/bindings -I/home/gps/src/firefox/objdir/dom/bindings -I/home/gps/src/firefox/objdir/dist/include/mozilla/dom -I/home/gps/src/firefox/dom/base -I/home/gps/src/firefox/dom/battery -I/home/gps/src/firefox/dom/bluetooth/common/webapi -I/home/gps/src/firefox/dom/camera -I/home/gps/src/firefox/dom/canvas -I/home/gps/src/firefox/dom/geolocation -I/home/gps/src/firefox/dom/html -I/home/gps/src/firefox/dom/indexedDB -I/home/gps/src/firefox/dom/media/webaudio -I/home/gps/src/firefox/dom/media/webspeech/recognition -I/home/gps/src/firefox/dom/svg -I/home/gps/src/firefox/dom/workers -I/home/gps/src/firefox/dom/xbl -I/home/gps/src/firefox/dom/xml -I/home/gps/src/firefox/dom/xslt/base -I/home/gps/src/firefox/dom/xslt/xpath -I/home/gps/src/firefox/dom/xul -I/home/gps/src/firefox/js/xpconnect/src -I/home/gps/src/firefox/js/xpconnect/wrappers -I/home/gps/src/firefox/layout/style -I/home/gps/src/firefox/layout/xul/tree -I/home/gps/src/firefox/media/mtransport -I/home/gps/src/firefox/media/webrtc -I/home/gps/src/firefox/media/webrtc/signaling/src/common/time_profiling -I/home/gps/src/firefox/media/webrtc/signaling/src/peerconnection -I/home/gps/src/firefox/objdir/ipc/ipdl/_ipdlheaders -I/home/gps/src/firefox/ipc/chromium/src -I/home/gps/src/firefox/ipc/glue -I/home/gps/src/firefox/objdir/dist/include -I/home/gps/src/firefox/objdir/dist/include/nspr -I/home/gps/src/firefox/objdir/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /home/gps/src/firefox/objdir/mozilla-config.h -MD -MP -MF .deps/Unified_cpp_dom_bindings0.o.pp -Qunused-arguments -Qunused-arguments -Wall -Wc++11-compat -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wwrite-strings -Wc++11-compat-pedantic -Wc++14-compat -Wc++14-compat-pedantic -Wclass-varargs -Wloop-analysis -Wthread-safety -Wunreachable-code -Wno-invalid-offsetof -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-unknown-warning-option -Wno-return-type-c-linkage -fno-exceptions -fno-strict-aliasing -fno-rtti -fno-exceptions -fno-math-errno -std=gnu++0x -pthread -pipe -g -Os -fomit-frame-pointer /home/gps/src/firefox/objdir/dom/bindings/Unified_cpp_dom_bindings0.cpp And a GCC invocation: /usr/bin/g++-4.9 -o Unified_cpp_dom_bindings0.o -c -I/home/gps/src/firefox/objdir/dist/stl_wrappers -I/home/gps/src/firefox/objdir/dist/system_wrappers -include /home/gps/src/firefox/config/gcc_hidden.h -DNDEBUG=1 -DTRIMMED=1 -DOS_POSIX=1 -DOS_LINUX=1 -DHAVE_SIDEBAR -DSTATIC_EXPORTABLE_JS_API -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/home/gps/src/firefox/dom/bindings -I/home/gps/src/firefox/objdir/dom/bindings -I/home/gps/src/firefox/objdir/dist/include/mozilla/dom -I/home/gps/src/firefox/dom/base -I/home/gps/src/firefox/dom/battery -I/home/gps/src/firefox/dom/bluetooth/common/webapi -I/home/gps/src/firefox/dom/camera -I/home/gps/src/firefox/dom/canvas -I/home/gps/src/firefox/dom/geolocation -I/home/gps/src/firefox/dom/html -I/home/gps/src/firefox/dom/indexedDB -I/home/gps/src/firefox/dom/media/webaudio -I/home/gps/src/firefox/dom/media/webspeech/recognition -I/home/gps/src/firefox/dom/svg -I/home/gps/src/firefox/dom/workers -I/home/gps/src/firefox/dom/xbl -I/home/gps/src/firefox/dom/xml -I/home/gps/src/firefox/dom/xslt/base -I/home/gps/src/firefox/dom/xslt/xpath -I/home/gps/src/firefox/dom/xul -I/home/gps/src/firefox/js/xpconnect/src -I/home/gps/src/firefox/js/xpconnect/wrappers -I/home/gps/src/firefox/layout/style -I/home/gps/src/firefox/layout/xul/tree -I/home/gps/src/firefox/media/mtransport -I/home/gps/src/firefox/media/webrtc -I/home/gps/src/firefox/media/webrtc/signaling/src/common/time_profiling -I/home/gps/src/firefox/media/webrtc/signaling/src/peerconnection -I/home/gps/src/firefox/objdir/ipc/ipdl/_ipdlheaders -I/home/gps/src/firefox/ipc/chromium/src -I/home/gps/src/firefox/ipc/glue -I/home/gps/src/firefox/objdir/dist/include -I/home/gps/src/firefox/objdir/dist/include/nspr -I/home/gps/src/firefox/objdir/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /home/gps/src/firefox/objdir/mozilla-config.h -MD -MP -MF .deps/Unified_cpp_dom_bindings0.o.pp -Wall -Wc++11-compat -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wwrite-strings -Wunreachable-code -Wno-invalid-offsetof -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -fno-exceptions -fno-strict-aliasing -fno-rtti -fno-exceptions -fno-math-errno -std=gnu++0x -pthread -pipe -g -freorder-blocks -Os -fomit-frame-pointer /home/gps/src/firefox/objdir/dom/bindings/Unified_cpp_dom_bindings0.cpp wall (s) cpu (s) clang-3.7.0 665 4807 clang-3.6.2 670 4727 gcc-5.2.1 860 6493 gcc-4.9.3 849 6084 Clang can build Firefox over 3 minutes wall time faster than GCC. And that's on one of the fastest consumer grade CPUs you can buy. The 1000+s CPU time differential is even more impressive IMO. Again, this is only the most basic build configuration. Will need to test things like ccache for a more accurate picture. I'd also like to test GCC 5.3 (latest stable release), but I need to find an appropriate package for that first.
I built gcc-5.3.0 from source and it yielded: wall (s) cpu (s) gcc-5.3.0 860 6161 So similar ballpark as 5.2 and 4.9. That being said, comparing the official Ubuntu packages with something I built locally could introduce variance due to e.g. the compiler optimizations used to build GCC. I could build everything locally. But I don't think any set of compiler flag changes is going to cover a multiple minute spread between Clang. So I'm content with assuming GCC 5.3 performs about the same as 5.2.
Product: Core → Firefox Build System
Since the Clang train appears to be full speed ahead and I've seen emails on dev-platform encouraging people to build with Clang instead of GCC, should we implement the change in the bug summary? Would one of the people needinfo'd be willing to do that?
Component: General → Toolchains
Flags: needinfo?(nfroyd)
Flags: needinfo?(mh+mozilla)
Flags: needinfo?(dmajor)
I am pretty sure this is a matter of "when" and not "if". So, yes, I do think we should do this, but I don't spend enough time on Linux to know whether this is safe to land right away. I'd feel better if some full-time Linux developers weighed in with support or concerns.
Flags: needinfo?(dmajor)
I'd say people that really want to build with gcc probably only have gcc installed. Even if they do have clang installed, using gcc is a CC=gcc away. One possible worry is the fact that configure currently doesn't fallback to an alternative compiler when using the primary compiler fails. As in, systems with *both* gcc and clang installed, but for some reason, clang doesn't work. For those, the build would start failing instead of falling back to gcc. ISTR we've had some complains around something like that in the past. This might not be a large enough problem. One thing, though, is that except if we fix whatever's going wrong, this is a clobber-requiring change (somehow some sanitizer test is still trying to use a (cached?) gcc while the build is trying to actually use clang).
Flags: needinfo?(mh+mozilla)
> (somehow some sanitizer test is still trying to use a (cached?) gcc while the build is trying to actually use clang) yeah, that's it, when we enter old-configure, config.cache is loaded, that still contains settings for gcc.
One concern that some people might have is that since automation builds still use gcc, they might get breakage on automation that they don't get locally, but considering we have CI builds using clang *and* and others using gcc on linux, that's already a problem today, in the opposite direction.
(In reply to Mike Hommey [:glandium] from comment #8) > I'd say people that really want to build with gcc probably only have gcc > installed. Actually that's not true, since we require clang for bindgen. Also, this means that a "simple" switch in toolchain.configure might unexpectedly switch CI builds to clang if they don't explicitly set CC to gcc (and if clang is in the PATH, but I'm not actually sure it is).
I think glandium is the final arbiter of all things Linux toolchain-y. IIUC, the past several comments from glandium can be summed up as "yes, but there are some fine details to work out"? The potential breakage from mis-installed clang cited in comment 8 is a little concerning, but given that there is a fallback (CC=gcc) and seeking out the breakage will force us to improve detection and/or bootstrap, I think that's OK. Redirecting ni? to glandium to double-check. I'm happy to implement, but it will not happen from my end until early next week, I think.
Flags: needinfo?(nfroyd) → needinfo?(mh+mozilla)
The additional concern is comment 11 meaning that downstreams would end up switched to clang if they're not vigilant. So it feels like as long as we don't switch our shipped builds to clang, this should be gated on DEVELOPER_OPTIONS (and CLOBBER should be touched, sadly)
Flags: needinfo?(mh+mozilla)
Assignee: nobody → mh+mozilla
Attachment #8982091 - Flags: review?(core-build-config-reviews) → review?(gps)
Comment on attachment 8982091 [details] Bug 1253064 - Prefer Clang to GCC in local developer builds. https://reviewboard.mozilla.org/r/248130/#review254280 ::: build/moz.configure/toolchain.configure:684 (Diff revision 1) > + if developer_options: > + return ('clang',) + gcc It's past time for r+'ing code, but I'm unsure that this is correct for the local Android case. We don't want to prefer the locally installed clang here for Android builds, because we don't know whether it has support for the architecture (i.e. ARM) we're trying to build for, whereas we do know that the NDK clang, added above, does.
Comment on attachment 8982091 [details] Bug 1253064 - Prefer Clang to GCC in local developer builds. https://reviewboard.mozilla.org/r/248130/#review254278 Would r+ again.
Attachment #8982091 - Flags: review?(gps) → review+
Comment on attachment 8982091 [details] Bug 1253064 - Prefer Clang to GCC in local developer builds. https://reviewboard.mozilla.org/r/248130/#review254280 > It's past time for r+'ing code, but I'm unsure that this is correct for the local Android case. We don't want to prefer the locally installed clang here for Android builds, because we don't know whether it has support for the architecture (i.e. ARM) we're trying to build for, whereas we do know that the NDK clang, added above, does. Good catch. Without wanting to hurt my brain too hard, an easy compromise would be `if developer_options and not android_clang_compiler:`. Then we could deal with Android as a follow-up. I'm very much in favor of making incremental progress towards Clang. But if we can handle Android properly from the start without too much effort, go for it.
Comment on attachment 8982091 [details] Bug 1253064 - Prefer Clang to GCC in local developer builds. https://reviewboard.mozilla.org/r/248130/#review254286
Attachment #8982091 - Flags: review+ → review-
Attachment #8982091 - Flags: review?(core-build-config-reviews)
Comment on attachment 8982091 [details] Bug 1253064 - Prefer Clang to GCC in local developer builds. https://reviewboard.mozilla.org/r/248130/#review254296
Attachment #8982091 - Flags: review+
Pushed by mh@glandium.org: https://hg.mozilla.org/integration/autoland/rev/07db1154d5b9 Prefer Clang to GCC in local developer builds. r=gps
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Can you send mail to dev-platform about this?
Flags: needinfo?(mh+mozilla)
Done.
Flags: needinfo?(mh+mozilla)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: