-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Inline simple Cursor write calls #33921
Conversation
Implementing the Write trait for Cursors over slices is so light-weight that under some circumstances multiple writes can be fused into a single instruction. In general I think inlining these functions is a good idea because most of the code can be constant-folded and copy-propagated away. Closes issue rust-lang#33916.
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
⌛ Testing commit 80d7333 with merge 6b930d1... |
💔 Test failed - auto-linux-64-cross-netbsd |
@bors: retry On Mon, May 30, 2016 at 11:09 PM, bors [email protected] wrote:
|
⌛ Testing commit 80d7333 with merge 2319d2f... |
💔 Test failed - auto-linux-64-cross-armhf |
@bors: retry |
Inline simple Cursor write calls Implementing the Write trait for Cursors over slices is so light-weight that under some circumstances multiple writes can be fused into a single instruction. In general I think inlining these functions is a good idea because most of the code can be constant-folded and copy-propagated away. Closes issue rust-lang#33916. r? @alexcrichton
(Partially) brings back rust-lang#33921
…lice, r=sfackler Inline some Cursor calls for slices (Partially) brings back rust-lang#33921 I've noticed in some serialization code I was writing that writes to slices produce much, much, worse code than you'd expect even with optimizations turned on. For example, you'd expect something like this to be zero cost: ``` use std::io::{self, Cursor, Write}; pub fn serialize((a, b): (u64, u64)) -> [u8;8+8] { let mut r = [0u8;16]; { let mut w = Cursor::new(&mut r[..]); w.write(&a.to_le_bytes()).unwrap(); w.write(&b.to_le_bytes()).unwrap(); } r } ``` ...but it compiles down to [dozens of instructions](https://rust.godbolt.org/z/bdwDzb) because the `slice_write()` calls aren't inlined, which in turn means `unwrap()` can't be optimized away, and so on. To be clear, this pull-req isn't sufficient by itself: if we want to go down that path we also need to add `#[inline]`'s to the default implementations for functions like `write_all()` in the `Write` trait and so on, or implement them separately in the `Cursor` impls. But I figured I'd start a conversation about what tradeoffs we're expecting here.
Implementing the Write trait for Cursors over slices is so light-weight that under some circumstances multiple writes can be fused into a single instruction. In general I think inlining these functions is a good idea because most of the code can be constant-folded and copy-propagated away.
Closes issue #33916.
r? @alexcrichton