Stop building lld with LLVM_ENABLE_THREADS=OFF |
||
Issue descriptionBoth the Android and Windows CFI bots are failing to finish compiling due to an output timeout. https://build.chromium.org/p/chromium.clang/builders/ToTAndroidCFI/builds/276 https://build.chromium.org/p/chromium.clang/builders/ToTWinCFI/builds/120 This is because the lld that we build during ToT builds is built with LLVM_ENABLE_THREADS=OFF. We should stop doing that. The simplest solution would be to start building the entire toolchain with threading enabled. This on its own should not affect how many threads clang uses, so we will continue to use process-level parallelism as before. But we will need to do a few things before we can do that: - make sure it is compatible with goma - make sure perf is acceptable
,
Oct 30 2017
> We build lld separately from clang on linux. Let's do the same on windows. Note that we only do that in bootstrap builds, and only if --lto-lld is enabled (which is why we're still single threaded on the Android ToT bot). > I'd strongly prefer if we could keep building clang with threads off. I don't understand why.
,
Oct 30 2017
> I don't understand why. Because it compile-time-ensures that compiles are single threaded. If someone wanted to change that, that has ramifications for interaction with the build system, so it's imho important that we don't accidentally end up in a world with multithreaded compiles. (See internal Oct 23 thread "Compiler performance difference" for how we might end up there if we're not careful.)
,
Oct 30 2017
(re other point: That sounds just like a bug to me that we could fix: Always build lld separately.)
,
Oct 30 2017
The rest of the world uses process-level parallelism for compiles as well, so I'd imagine that upstream would be resistant to doing that by default. But if that doesn't change your mind, I'll look at building lld separately.
,
Oct 30 2017
I've been looking into this. TLDR: Performance of single-threaded builds with thread support is well within one percent of performance of single-threaded builds without thread support. For ThinLTO, multithreaded builds do *much* better. I think we should go ahead and enable thread support, and make sure Clang supports single-threaded builds even when LLVM is compiled with thread support. As for the concerns raised: > make sure it is compatible with goma How do we test this? Do we have reason to believe that enabling thread support would affect goma compatibility? > make sure perf is acceptable On the tests I've ran, the difference in time or cycles consumed is pretty small. For example, on Windows I measured a 0.5% difference in time to build unit_tests, and on Linux 0.1%. For builds that do use threads, the wins from using multiple threads can be huge. E.g. on Linux I had a ThinLTO build of unit_tests go from over an hour down to about 38 minutes. Since ThinLTO is now enabled by default on Linux, I'd say we should ship with thread support enabled. As for the larger picture discussion about process-based concurrency controlled by the build system: This is indeed the long-established practice and I agree we should be aware of the ramifications of changing that. Having worked on build systems in the past, this is an issue I've given a fair amount of thought. My ideal would be that the build system coordinate with other work that's going on on the system, including but not limited to the compiler and linker. The single-threaded compiler, build system controls number of processes model is an approximation of this that has served well for many years, but we can do better. Particularly if we can schedule small units of work (after all, a single compiler invocation may take 20 minutes), we can shorten the critical path for build times. Having this coordinated by a system with a more global view than what the build system has not only makes it ok for the compiler and linker to use multiple threads, but also allows the build system to play nice with other activity on the system. Ideal scenarios aside, I think at least for the foreseeable future there is a use case for explicitly limiting the number of threads that the compiler will actually use, in particular limiting it to a single thread. I think this is something we should explicitly support, and not just have depend on whether or not LLVM is compiled with *support* for multithreading.
,
Oct 30 2017
inglorion: To be clear, nobody is arguing against using threads in lld. The question is just how to hold our llvm build to get that.
,
Oct 30 2017
thakis: Understood, and thanks for the clarification. I'm making the case for "just compile without -DLLVM_ENABLE_THREADS=OFF".
,
Nov 1 2017
pcc's patch is here: https://chromium-review.googlesource.com/c/chromium/src/+/745525 > I'm making the case for "just compile without -DLLVM_ENABLE_THREADS=OFF". To make that case I think one would need to explain why we want to build Clang with threading enabled. Since we don't want to do multi-threaded compilation, all that would do is add the overhead from linking in the threading primitives and doing locking, etc. By using -DLLVM_ENABLE_THREADS=OFF we avoid all that, make sure no multi-threading sneaks into the compiles accidentally, and make sure that this build mode keeps working. The only arugment I see in favour of dropping -DLLVM_ENABLE_THREADS=OFF is to simplify our Clang build. That's a very valid argument, but I'm not sure it's enough justification to do it.
,
Nov 1 2017
> > I'm making the case for "just compile without -DLLVM_ENABLE_THREADS=OFF". > To make that case I think one would need to explain why we want to build Clang with threading enabled. Well, the way I looked at it is more "Why would we want to do two builds of LLVM, one with thread support disabled and one with thread support enabled?" I don't want to build a multi-threaded Clang (at least not at this point), rather I was curious why we build with thread *support* explicitly disabled. I got some answers to that question, and I'm happy to shelve the discussion for now given that we have a way forward that at least will reduce link times, which was my impetus for looking at this.
,
Nov 2 2017
Has anyone successfully replicated the result that "LLVM_ENABLE_THREADS=OFF" actually matters for single-threaded performance in the last year? My recollection is that there was some locking or fencing in LLVM's ManagedStatic, which is used by the pass manager, and the pass registry, and it matters... sometimes. Someone might have fixed that, though. I feel like we need new measurements to justify the ongoing complexity of this build customization. Right now it seems like we're afraid to touch this setting because of historical data.
,
Nov 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cd776f006a0fe2e35e179944fa475c97457069ae commit cd776f006a0fe2e35e179944fa475c97457069ae Author: Peter Collingbourne <pcc@chromium.org> Date: Fri Nov 03 17:10:16 2017 update.py: Build lld with threading support unconditionally. This removes the condition that we need to be doing a bootstrap build. With this change we will get an lld built with threads on Windows as well as Linux ToT builds. Bug: 779639 Change-Id: I961920d0f04f4aa2607edeab44a02e3857a2fd52 Reviewed-on: https://chromium-review.googlesource.com/745525 Commit-Queue: Hans Wennborg <hans@chromium.org> Reviewed-by: Hans Wennborg <hans@chromium.org> Cr-Commit-Position: refs/heads/master@{#513810} [modify] https://crrev.com/cd776f006a0fe2e35e179944fa475c97457069ae/tools/clang/scripts/update.py
,
Nov 3 2017
> Has anyone successfully replicated the result that "LLVM_ENABLE_THREADS=OFF" actually matters for single-threaded performance in the last year? It makes building unit_tests about 0.5% slower on Windows, 0.1% on Linux. I measured cycles on Linux, too, which was also increased by a small amount, but I don't have those numbers handy.
,
Mar 9 2018
I think this is done (see comment 12). Please reopen if that's wrong. |
||
►
Sign in to add a comment |
||
Comment 1 by thakis@chromium.org
, Oct 30 2017