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

Add a new const eval perf problem to the test suite #349

Merged
merged 2 commits into from
Feb 21, 2019

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Feb 19, 2019

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.

@Mark-Simulacrum
Copy link
Member

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?

@oli-obk
Copy link
Contributor Author

oli-obk commented Feb 19, 2019

could we rename the benchmark? so the old name just stops having data and the new one starts fresh?

@Mark-Simulacrum
Copy link
Member

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).

@nnethercote
Copy link
Contributor

I agree that renaming is a good idea. Simply adding a '-2' suffix might suffice?

@oli-obk
Copy link
Contributor Author

oli-obk commented Feb 21, 2019

I've renamed the benchmark

@Mark-Simulacrum
Copy link
Member

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];
Copy link
Member

@RalfJung RalfJung Oct 12, 2019

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 ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants