Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance regression in nightly-2019-12-14 #67331

Closed
jbg opened this issue Dec 15, 2019 · 6 comments
Closed

Performance regression in nightly-2019-12-14 #67331

jbg opened this issue Dec 15, 2019 · 6 comments
Labels
C-bug Category: This is a bug. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example I-compiletime Issue: Problems and improvements with respect to compile times. regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jbg
Copy link

jbg commented Dec 15, 2019

nightly-2019-12-14 times out building a crate here after 60m.

nightly-2019-12-13 builds it in 2m.

cargo-bisect-rustc gives:

bisecting ci builds
starting at 3eeb8d4f2, ending at ff15e9670
fetching commits from 3eeb8d4f2 to ff15e9670
opening existing repository at "rust.git"
refreshing repository
looking up first commit
looking up second commit
checking that commits are by bors and thus have ci artifacts...
finding bors merge commits
found 6 bors merge commits in the specified range
opening existing repository at "rust.git"
refreshing repository
validated commits found, specifying toolchains
testing commits
verifying the start of the range does not reproduce the regression
installing 3eeb8d4f2fbae0bb1c587d00b5abeaf938da47f4
testing 3eeb8d4f2fbae0bb1c587d00b5abeaf938da47f4
tested 3eeb8d4f2fbae0bb1c587d00b5abeaf938da47f4, got No
uninstalling 3eeb8d4f2fbae0bb1c587d00b5abeaf938da47f4
confirmed the start of the range does not reproduce the regression
verifying the end of the range reproduces the regression
installing ff15e9670843f8bd6b54ab1b042d2095b4c0aa6d
testing ff15e9670843f8bd6b54ab1b042d2095b4c0aa6d
tested ff15e9670843f8bd6b54ab1b042d2095b4c0aa6d, got Yes
uninstalling ff15e9670843f8bd6b54ab1b042d2095b4c0aa6d
confirmed the end of the range reproduces the regression
installing 9409c208a9a70eb6194c7502979843f42d6fed1a
testing 9409c208a9a70eb6194c7502979843f42d6fed1a
tested 9409c208a9a70eb6194c7502979843f42d6fed1a, got Yes
uninstalling 9409c208a9a70eb6194c7502979843f42d6fed1a
installing cf7e019b42cd523d91cb350ab49acbda1b11e571
testing cf7e019b42cd523d91cb350ab49acbda1b11e571
tested cf7e019b42cd523d91cb350ab49acbda1b11e571, got No
uninstalling cf7e019b42cd523d91cb350ab49acbda1b11e571
searched toolchains 3eeb8d4f2fbae0bb1c587d00b5abeaf938da47f4 through ff15e9670843f8bd6b54ab1b042d2095b4c0aa6d
regression in 9409c208a9a70eb6194c7502979843f42d6fed1a

Which relates to PR #66405

I'm working on a minimal repro but it may be difficult - the crate is a complex web API backend using a lot of async/await. Commenting out the entire route dispatcher brings us back to 2m compile time, bringing back routes one at a time indicates that routes which use database transactions (passing a &mut Transaction<'_> from deadpool-postgres to various async fns) trigger the regression, but they could also have something else in common.

@jonas-schievink jonas-schievink added C-bug Category: This is a bug. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example I-compiletime Issue: Problems and improvements with respect to compile times. I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 15, 2019
@jonas-schievink
Copy link
Contributor

cc @nnethercote

@nnethercote
Copy link
Contributor

@jonas-schievink: Is the crate publically available? It would be very helpful if I could build it myself and do some measurements, both to confirm that #66405 is at fault and to understand how it might have caused the regression. Although a minimal reproducer would be helpful, don't take too much effort producing one, because even a large test case would still be useful for me. Thanks.

@jbg
Copy link
Author

jbg commented Dec 15, 2019

@nnethercote (I think you meant to tag me)

Unfortunately this crate can't be shared, but I'm trying to produce something that I'm allowed to share. It's good to know it doesn't have to be minimal; as soon as I have something that's not sensitive I'll post it here even if I think it can be reduced further.

@pnkfelix
Copy link
Member

triage: for now, leaving unprioritized and removing nomination. Please re-nominate for prioritization after you come up with an example we can use to demonstrate the problem.

@jonas-schievink jonas-schievink added the regression-from-stable-to-beta Performance or correctness regression from stable to beta. label Dec 20, 2019
@nnethercote
Copy link
Contributor

#67454 has a reproducer for this regression.

@jbg
Copy link
Author

jbg commented Dec 20, 2019

Thank goodness, I spent a few hours trying to figure out what was triggering it here but didn't get anywhere. I'll close this as a dupe of #67454 since there's nothing here that isn't also on the other issue, and the other issue has much more useful detail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example I-compiletime Issue: Problems and improvements with respect to compile times. regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants