-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
mark vec::IntoIter pointers as !nonnull
#114205
Conversation
This comment has been minimized.
This comment has been minimized.
b811e15
to
d65f21c
Compare
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
⌛ Trying commit d65f21c8e91179cbd468cc480ad8401a80cce423 with merge 31998b4119efbe4f8a7a72f7695a35766f54f1e7... |
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
r=me, assuming perf comes back tolerable |
Finished benchmarking commit (31998b4119efbe4f8a7a72f7695a35766f54f1e7): comparison URL. Overall result: ❌ regressions - ACTION NEEDEDBenchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @bors rollup=never Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Bootstrap: 651.072s -> 649.517s (-0.24%) |
Gonna assume that's a no. |
We don't have charts for the runtime benchmarks yet so it's hard to eyeball how significant that is. Plus most of the regressions are in incremental with the dep-graph growing a bit. I don't know what that means, but maybe there's some fat that can be shaved off. |
d65f21c
to
899d960
Compare
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
⌛ Trying commit 899d960 with merge ccce35fd76fdf2a3f9722f31c89a81efbaefba53... |
I'll try to add charts and possibly also some cachegrind diff so that we can examine what causes these runtime benchmark changes. |
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
platform-specific IR naming? Fun... |
b05a234
to
6a1ad55
Compare
@bors r=scottmcm |
🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened. |
mark vec::IntoIter pointers as `!nonnull` This applies the same NonNull optimizations to `vec::IntoIter` as rust-lang#113344 did for `slice::Iter` [Godbolt](https://rust.godbolt.org/z/n1cTea718) showing the test IR on current nightly, note the absence of `!nonnull` on the loads. r? `@scottmcm`
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
☔ The latest upstream changes (presumably #119662) made this pull request unmergeable. Please resolve the merge conflicts. |
6a1ad55
to
4316113
Compare
4316113
to
93b34a5
Compare
@bors r=scottmcm |
☀️ Test successful - checks-actions |
Finished benchmarking commit (9522993): comparison URL. Overall result: ❌✅ regressions and improvements - ACTION NEEDEDNext Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Bootstrap: 667.475s -> 668.625s (0.17%) |
This applies the same NonNull optimizations to
vec::IntoIter
as #113344 did forslice::Iter
Godbolt showing the test IR on current nightly, note the absence of
!nonnull
on the loads.r? @scottmcm