Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upRegression in compile times after LLVM was upgraded #31435
Comments
This comment has been minimized.
This comment has been minimized.
|
Nominating for triage as well cc @rust-lang/compiler |
This comment has been minimized.
This comment has been minimized.
before? Most of the time-intensive passes are universally slower. There’s more passes being run in the new LLVM, but all the new ones are pretty fast (timings around 0.000s). |
This comment has been minimized.
This comment has been minimized.
|
@nagisa ah yeah oops that was a typo (corrected above) Also I just realized that nightly compilers have LLVM assertions enabled whereas my local compilers did not, that would explain the discrepancy between those timings. |
This comment has been minimized.
This comment has been minimized.
|
Looks like something to ping the LLVM guys about, at least to get some insight. Loop-invariant code motion seems like it's taken a significant hit, with the top 3 LICM passes all taking around twice the time now. |
This comment has been minimized.
This comment has been minimized.
|
This is at least partially caused by the new AA handling which ends up being quadratic in the number of used passes. I've filed an LLVM bug at https://llvm.org/bugs/show_bug.cgi?id=26564 |
This comment has been minimized.
This comment has been minimized.
|
triage: P-medium Not much to do here besides try to keep an eye on how LLVM progresses in fixes this issue, right? (Rolling back LLVM doesn't really seem like a viable plan.) |
rust-highfive
added
P-medium
and removed
I-nominated
labels
Feb 11, 2016
This comment has been minimized.
This comment has been minimized.
|
At the very least we could apply a temporary local patch if it's a quick hack vs. a longer proper fix. |
This comment has been minimized.
This comment has been minimized.
|
We could perhaps disable that pass? I'm not sure if it is a pass that's causing the problem. |
This comment has been minimized.
This comment has been minimized.
|
We could remove TBAA which does nothing for us. That saves a few percent.
|
This comment has been minimized.
This comment has been minimized.
|
We've got a pretty long period of time before this reaches stable, so I'd be willing to give the LLVM bug report some more time to get some analysis on behalf of the LLVM folks. If this gets close to stable, however, backporting a change to just disable TBAA seems like it should definitely be worth it! In other words, ideally, there'd be an upstream patch to fix this regressions, we'd backport it to our branch, but failing that we can just locally disable the regression temporarily. |
This comment has been minimized.
This comment has been minimized.
|
This is perhaps a bit off-topic, but why are we running passes that don't do anything for us? Just to keep us close to the C/C++ pass pipeline? |
This comment has been minimized.
This comment has been minimized.
|
Yes, as far as I'm aware that's it. The reason for that is that the default pass setup works reasonablly well so far, and having your own pass setup means more maintenance to keep compatible with different LLVM versions and that you don't automatically profit from improvements made to the default pass setup (obviously). That said, there have been various people who wanted to look into getting a more rust specific pass setup, it's just a ton of work and nothing has been completed (started?) yet. Also, for this specific analysis, the extra time spent on TBAA was negligible, shouldn't have been much more than just checking for TBAA metadata, seeing that there is none and handing things over to the next AA analysis. |
sanxiyn
added
the
I-compiletime
label
Mar 18, 2016
This comment has been minimized.
This comment has been minimized.
|
AA fix landed upstream in LLVM r262490. |
dotdash
added a commit
to dotdash/rust
that referenced
this issue
Mar 18, 2016
dotdash
referenced this issue
Mar 18, 2016
Merged
Update LLVM to include a backport to restore AA performance #32337
eddyb
added a commit
to eddyb/rust
that referenced
this issue
Mar 19, 2016
This comment has been minimized.
This comment has been minimized.
|
I'm not 100% sure what the status is here -- but it seems like we merged the fix from upstream, so going to close this as fixed. Feel free to re-open. |
alexcrichton commentedFeb 5, 2016
LLVM was recently upgraded in #30448 which appears to have caused a regression
in compile times. First up, let's talk numbers. The compiler revisions in
question are:
The
nightly-2016-01-30toolchain conveniently corresponds exactly to303892e. The benchmarks here will all be
compiling libsyntax at these two revision in optimized mode. Note that
libsyntax did not change at all between these two revisions. For local
compilations I configured with:
and the C++ compiler version is:
Nightly compiler (
nightly-2016-01-30)Summary: 2:11.18s compile time, full output of time-passes
Locally compiled after LLVM upgrade (303892e)
Summary: 1:37.48s compile time, full output of time-passes
Locally compiled before LLVM upgrade (074f49a)
Summary: 1:22.69s compile time, full output of time-passes
Summary
So there are some fascinating data points here:
The nightlies we're shipping are 35% slower than a locally compiled copy-- Edit: nightly has LLVM assertions enabled where my local compilers did notOk, so something looks clearly suspicious with LLVM! Let's try and take a closer
look. First, some data dumps of
-Z time-llvm-passesnightly-2016-01-30compilerUnfortunately I don't have much of a takeaway from this. Maybe others might
thought?
Next steps
can include in our local LLVM fork temporarily?
here?