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

30-60% perf regression in ctfe-stress #68967

Closed
jonas-schievink opened this issue Feb 8, 2020 · 3 comments
Closed

30-60% perf regression in ctfe-stress #68967

jonas-schievink opened this issue Feb 8, 2020 · 3 comments
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) C-bug Category: This is a bug. 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

@jonas-schievink
Copy link
Contributor

jonas-schievink commented Feb 8, 2020

This happened on 2020-01-11: https://perf.rust-lang.org/compare.html?start=bfd04876b93ad5c013d90bc46937e28b6ee1a3f4&end=1389494ac145a84dba025ff65969f7ab150c3f02&stat=instructions:u

Caused by #67000

cc @oli-obk @spastorino

(I could've sworn we already tracked this regression, but apparently not)

screenshot-2020-02-09-00:03:41

@jonas-schievink jonas-schievink added I-nominated I-compiletime Issue: Problems and improvements with respect to compile times. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. C-bug Category: This is a bug. A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) labels Feb 8, 2020
@oli-obk
Copy link
Contributor

oli-obk commented Feb 10, 2020

We knew about this regression in the PR, maybe you remember that discussion. In #67000 (comment) I finally came to the conclusion that the reason we regressed perf here is that before that PR we didn't actually evaluate the promoteds, we just created pointers to them. #67000 causes all promoteds to get evaluated immediately when their use site is evaluated. Basically any real program would already have done this work as collect would have evaluated the constant. Only if you have loads and loads of dead promoteds can you see the difference.

@jonas-schievink
Copy link
Contributor Author

Okay, that sounds much less alarming then. Thanks for the clarification!

@nikomatsakis
Copy link
Contributor

Visited during compiler-team triage. Based on @oli-obk's comment, going to close as "won't fix". Thanks @jonas-schievink!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) C-bug Category: This is a bug. 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

3 participants