-
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
Faster Layout::array
#91246
Faster Layout::array
#91246
Conversation
Other, similar methods for `Layout` do likewise, and there's already an `unwrap()` around the result demonstrating the safety.
The current implementation is much more conservative than it needs to be, because it's dealing with the size and alignment of a given `T`, which are more restricted than an arbitrary `Layout`. For example, imagine a struct with a `u32` and a `u4`. You can safely create a `Layout { size_: 5, align_: 4 }` by hand, but `Layout::new::<T>` will give `Layout { size_: 8, align_: 4}`, where the size already has padding that accounts for the alignment. (And the existing `debug_assert_eq!` in `Layout::array` already demonstrates that no additional padding is required.)
Because there's some subtle behaviour specific to zero-sized types and it's currently not well tested.
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit dbfb913 with merge d13e5c93c93bb4fe043c1aee2c2e53101297b702... |
☀️ Try build successful - checks-actions |
Queued d13e5c93c93bb4fe043c1aee2c2e53101297b702 with parent 9adfd9d, future comparison URL. |
Finished benchmarking commit (d13e5c93c93bb4fe043c1aee2c2e53101297b702): comparison url. Summary: This change led to large relevant mixed results 🤷 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. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led 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 |
Instructions mostly show a win, with a few small regressions. Cycles and wall-times show no regressions but fewer wins. We've seen in the past that this r? @Amanieu |
BTW I confirmed with |
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.
Nice!
@bors r+ |
📌 Commit dbfb913 has been approved by |
⌛ Testing commit dbfb913 with merge 54ea5fc76e3b0c1824b39cd4c583badaabc42c67... |
The job Click to see the possible cause of the failure (guessed by this bot)
|
💔 Test failed - checks-actions |
fatal error LNK1201: error writing to program database 'D:\a\rust\rust\build\x86_64-pc-windows-msvc\stage2-tools\x86_64-pc-windows-msvc\tmp\cit\t1300\foo\target\release\deps\test.pdb'; check for insufficient disk space, invalid path, or insufficient privilege @bors retry |
☀️ Test successful - checks-actions |
Finished benchmarking commit (ccce985): comparison url. Summary: This change led to very large relevant mixed results 🤷 in compiler performance.
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression |
Layout::array
is called (indirectly) byVec::push()
, which is typically instantiated many times, and so making it smaller can help with compile times because less LLVM IR is generated.r? @ghost