-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Derive Copy
for VarianceDiagInfo
#86670
Conversation
r? @davidtwco (rust-highfive has picked a reviewer for you, use r? to override) |
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit 4be38d2 with merge 242d9f0318ecad35ad4e410f82bae32fb62fe3d0... |
Copy
for `VarianceDiagInfo1Copy
for VarianceDiagInfo
☀️ Try build successful - checks-actions |
Queued 242d9f0318ecad35ad4e410f82bae32fb62fe3d0 with parent 543ab99, future comparison URL. |
Finished benchmarking try commit (242d9f0318ecad35ad4e410f82bae32fb62fe3d0): comparison url. Summary: This change led to significant improvements 🎉 in compiler performance.
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up. @bors rollup=never |
Error: Label perf-regression can only be set by Rust team members Please let |
The performance improvements suggest that it might be worth trying to revive #72409 in some form - the compiler shouldn't need a |
Improvement in one test, but max-rss is red in most cases (given that max-rss bench results accuracy not high and can be scattered up and down). |
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.
LGTM, r=me if this is something you wanted to land and not just an experiment to check #72409's viability.
This is intended to fix the performance regression caused by adding the variance diag info. @bors r=davidtwco |
📌 Commit 4be38d2 has been approved by |
⌛ Testing commit 4be38d2 with merge 71a97b28ab15a7ff758e748e9518088f6adfa076... |
💔 Test failed - checks-actions |
@bors retry |
☀️ Test successful - checks-actions |
Thanks @Aaron1011 ! End results look good to me. (Original regressing results here for reference.) |
No description provided.