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

Avoid some string allocations. #778

Merged

Conversation

SimonSapin
Copy link
Contributor

No description provided.

let written_len = buffer.len() - remaining_len;
let written = &buffer[..written_len];

// write! only provides std::fmt::Formatter to Display implementations,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would strongly prefer to implement this safely if possible. Would it be possible to implement fmt::Write and use that in the call to write!?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementing fmt::Write how?

It’s possible to remove unsafe completely, at some performance cost:

diff --git a/serde/src/macros.rs b/serde/src/macros.rs
index 126c7ec..cc8775b 100644
--- a/serde/src/macros.rs
+++ b/serde/src/macros.rs
@@ -200,7 +200,7 @@ macro_rules! serialize_display_bounded_length {
     ($value: expr, $MAX_LEN: expr, $serializer: expr) => {
         {
             use std::io::Write;
-            let mut buffer: [u8; $MAX_LEN] = unsafe { ::std::mem::uninitialized() };
+            let mut buffer = [0; $MAX_LEN];
             let remaining_len;
             {
                 let mut remaining = &mut buffer[..];
@@ -209,13 +209,7 @@ macro_rules! serialize_display_bounded_length {
             }
             let written_len = buffer.len() - remaining_len;
             let written = &buffer[..written_len];
-
-            // write! only provides std::fmt::Formatter to Display implementations,
-            // which has methods write_str and write_char but no method to write arbitrary bytes.
-            // Therefore, `written` is well-formed in UTF-8.
-            let written_str = unsafe {
-                ::std::str::from_utf8_unchecked(written)
-            };
+            let written_str = ::std::str::from_utf8(written).unwrap();
             $serializer.serialize_str(written_str)
         }
     }

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right that doesn't help. Let's go with your way.

@dtolnay
Copy link
Member

dtolnay commented Feb 20, 2017

Benchmark:

test fast_safe   ... bench:         110 ns/iter (+/- 3)
test fast_unsafe ... bench:         103 ns/iter (+/- 3)
test string      ... bench:         189 ns/iter (+/- 6)
#![feature(test)]

extern crate test;
use test::Bencher;

use std::io::Write;
use std::mem;
use std::net::*;
use std::str;

macro_rules! addr {
    () => {
        Ipv4Addr::new(127, 0, 0, 1)
        //SocketAddrV6::new(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 1), 8080, 0, 0)
        //Ipv6Addr::new(0, 0, 0, 0, 0, 0xffff, 0xc00a, 0x2ff)
    }
}

#[bench]
fn string(b: &mut Bencher) {
    let v = addr!();
    b.iter(|| {
        test::black_box(v.to_string());
    });
}

#[bench]
fn fast_unsafe(b: &mut Bencher) {
    let v = addr!();
    b.iter(|| {
        /// "[1000:1002:1003:1004:1005:1006:1007:1008]:65000".len()
        const MAX_LEN: usize = 47;

        let mut buffer: [u8; MAX_LEN] = unsafe { mem::uninitialized() };
        let remaining_len;
        {
            let mut remaining = &mut buffer[..];
            write!(remaining, "{}", v).unwrap();
            remaining_len = remaining.len()
        }
        let written_len = buffer.len() - remaining_len;
        let written = &buffer[..written_len];

        // write! only provides std::fmt::Formatter to Display implementations,
        // which has methods write_str and write_char but no method to write arbitrary bytes.
        // Therefore, `written` is well-formed in UTF-8.
        let written_str = unsafe {
            str::from_utf8_unchecked(written)
        };
        test::black_box(written_str);
    });
}

#[bench]
fn fast_safe(b: &mut Bencher) {
    let v = addr!();
    b.iter(|| {
        /// "[1000:1002:1003:1004:1005:1006:1007:1008]:65000".len()
        const MAX_LEN: usize = 47;

        let mut buffer = [0; MAX_LEN];
        let remaining_len;
        {
            let mut remaining = &mut buffer[..];
            write!(remaining, "{}", v).unwrap();
            remaining_len = remaining.len()
        }
        let written_len = buffer.len() - remaining_len;
        let written = &buffer[..written_len];

        let written_str = str::from_utf8(written).unwrap();
        test::black_box(written_str);
    });
}

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

Successfully merging this pull request may close these issues.

2 participants