-
Notifications
You must be signed in to change notification settings - Fork 152
Add a new const eval perf problem to the test suite #349
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
Conversation
2 seconds seems fine; we try to stay under a hard limit of 30. I'm not so sure about modifying an existing benchmark though - it'll show up as a regression despite no changes to the compiler. @nnethercote, what do you think? |
could we rename the benchmark? so the old name just stops having data and the new one starts fresh? |
Yeah, thinking on this more, that seems like the best option. It might be that we want to long-term add some form of versioning to benchmarks (so we can update whenever and graphs are properly disjoint). |
I agree that renaming is a good idea. Simply adding a '-2' suffix might suffice? |
I've renamed the benchmark |
I will try to deploy this later today, thanks! |
// copying all these zeros and the corresponding definedness bits can be expensive and is probably | ||
// prone to regressions. | ||
// 24 is an exponent that makes the repeat expression take less than two seconds to compute | ||
const FOO: [i32; 1 << 24] = [0; 1 << 24]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried hard to give more or less reasonable names to the constants above; I think we can do better than FOO
;)
This takes around two seconds on my machine, but with a few optimizations I've been able to push it below one second (rust-lang/rust#58556) and I still see many more avenues to optimize it. We can lower the exponent to decrease the compilation time further.
Since this entire test used to take around 16 seconds, I hope the additional 2 seconds are ok for now.