auto merge of #9352 : erickt/rust/master, r=huonw

One of the downsides with `c_str` is that it always forces an allocation, and so this could add unnecessary overhead to various calls. This PR implements one of the suggestions @graydon made in #8296 for `vec.with_c_str` in that for a short string can use a small stack array instead of a malloced array for our temporary c string. This ends up being twice as fast for small strings.

There are two things to consider before landing this commit though. First off, I arbitrarily picked the stack array as 32 bytes, and I'm not sure if this a reasonable amount or not. Second, there is a risk that someone can keep a reference to the interior stack pointer, which could cause mayhem if someone were to dereference the pointer. Since we also can easily grab and return interior pointers to vecs though, I don't think this is that much of an issue.

src/librustc/middle/trans/base.rs

@@ -524,13 +524,13 @@ pub fn set_always_inline(f: ValueRef) {
 }
 
 pub fn set_fixed_stack_segment(f: ValueRef) {
-    do "fixed-stack-segment".to_c_str().with_ref |buf| {
+    do "fixed-stack-segment".with_c_str |buf| {
         unsafe { llvm::LLVMAddFunctionAttrString(f, buf); }
     }
 }
 
 pub fn set_no_split_stack(f: ValueRef) {
-    do "no-split-stack".to_c_str().with_ref |buf| {
+    do "no-split-stack".with_c_str |buf| {
         unsafe { llvm::LLVMAddFunctionAttrString(f, buf); }
     }
 }

src/librustc/middle/trans/debuginfo.rs

@@ -781,7 +781,7 @@ pub fn create_function_debug_context(cx: &mut CrateContext,
 
                 let ident = special_idents::type_self;
 
-                let param_metadata = do token::ident_to_str(&ident).to_c_str().with_ref |name| {
+                let param_metadata = do token::ident_to_str(&ident).with_c_str |name| {
                     unsafe {
                         llvm::LLVMDIBuilderCreateTemplateTypeParameter(
                             DIB(cx),
@@ -819,7 +819,7 @@ pub fn create_function_debug_context(cx: &mut CrateContext,
             // Again, only create type information if extra_debuginfo is enabled
             if cx.sess.opts.extra_debuginfo {
                 let actual_type_metadata = type_metadata(cx, actual_type, codemap::dummy_sp());
-                let param_metadata = do token::ident_to_str(&ident).to_c_str().with_ref |name| {
+                let param_metadata = do token::ident_to_str(&ident).with_c_str |name| {
                     unsafe {
                         llvm::LLVMDIBuilderCreateTemplateTypeParameter(
                             DIB(cx),

src/librustc/middle/trans/foreign.rs

@@ -465,7 +465,7 @@ pub fn trans_rust_fn_with_foreign_abi(ccx: @mut CrateContext,
         //     }
 
         let the_block =
-            "the block".to_c_str().with_ref(
+            "the block".with_c_str(
                 |s| llvm::LLVMAppendBasicBlockInContext(ccx.llcx, llwrapfn, s));
 
         let builder = ccx.builder.B;
@@ -519,7 +519,7 @@ pub fn trans_rust_fn_with_foreign_abi(ccx: @mut CrateContext,
 
                 None => {
                     let slot = {
-                        "return_alloca".to_c_str().with_ref(
+                        "return_alloca".with_c_str(
                             |s| llvm::LLVMBuildAlloca(builder,
                                                       llrust_ret_ty.to_ref(),
                                                       s))

src/libstd/c_str.rs

@@ -61,16 +61,18 @@ do my_string.with_c_str |c_buffer| {
 */
 
 use cast;
+use container::Container;
 use iter::{Iterator, range};
 use libc;
 use ops::Drop;
 use option::{Option, Some, None};
 use ptr::RawPtr;
 use ptr;
-use str;
 use str::StrSlice;
-use vec::{ImmutableVector, CopyableVector};
-use container::Container;
+use str;
+use vec::{CopyableVector, ImmutableVector, MutableVector};
+use vec;
+use unstable::intrinsics;
 
 /// Resolution options for the `null_byte` condition
 pub enum NullByteResolution {
@@ -152,8 +154,7 @@ impl CString {
     pub fn as_bytes<'a>(&'a self) -> &'a [u8] {
         if self.buf.is_null() { fail!("CString is null!"); }
         unsafe {
-            let len = ptr::position(self.buf, |c| *c == 0);
-            cast::transmute((self.buf, len + 1))
+            cast::transmute((self.buf, self.len() + 1))
         }
     }
 
@@ -187,6 +188,15 @@ impl Drop for CString {
     }
 }
 
+impl Container for CString {
+    #[inline]
+    fn len(&self) -> uint {
+        unsafe {
+            ptr::position(self.buf, |c| *c == 0)
+        }
+    }
+}
+
 /// A generic trait for converting a value to a CString.
 pub trait ToCStr {
     /// Copy the receiver into a CString.
@@ -233,24 +243,27 @@ impl<'self> ToCStr for &'self str {
     unsafe fn to_c_str_unchecked(&self) -> CString {
         self.as_bytes().to_c_str_unchecked()
     }
+
+    #[inline]
+    fn with_c_str<T>(&self, f: &fn(*libc::c_char) -> T) -> T {
+        self.as_bytes().with_c_str(f)
+    }
+
+    #[inline]
+    unsafe fn with_c_str_unchecked<T>(&self, f: &fn(*libc::c_char) -> T) -> T {
+        self.as_bytes().with_c_str_unchecked(f)
+    }
 }
 
+// The length of the stack allocated buffer for `vec.with_c_str()`
+static BUF_LEN: uint = 128;
+
 impl<'self> ToCStr for &'self [u8] {
     fn to_c_str(&self) -> CString {
         #[fixed_stack_segment]; #[inline(never)];
         let mut cs = unsafe { self.to_c_str_unchecked() };
         do cs.with_mut_ref |buf| {
-            for i in range(0, self.len()) {
-                unsafe {
-                    let p = buf.offset(i as int);
-                    if *p == 0 {
-                        match null_byte::cond.raise(self.to_owned()) {
-                            Truncate => break,
-                            ReplaceWith(c) => *p = c
-                        }
-                    }
-                }
-            }
+            check_for_null(*self, buf);
         }
         cs
     }
@@ -269,6 +282,50 @@ impl<'self> ToCStr for &'self [u8] {
             CString::new(buf as *libc::c_char, true)
         }
     }
+
+    fn with_c_str<T>(&self, f: &fn(*libc::c_char) -> T) -> T {
+        unsafe { with_c_str(*self, true, f) }
+    }
+
+    unsafe fn with_c_str_unchecked<T>(&self, f: &fn(*libc::c_char) -> T) -> T {
+        with_c_str(*self, false, f)
+    }
+}
+
+// Unsafe function that handles possibly copying the &[u8] into a stack array.
+unsafe fn with_c_str<T>(v: &[u8], checked: bool, f: &fn(*libc::c_char) -> T) -> T {
+    if v.len() < BUF_LEN {
+        let mut buf: [u8, .. BUF_LEN] = intrinsics::uninit();
+        vec::bytes::copy_memory(buf, v, v.len());
+        buf[v.len()] = 0;
+
+        do buf.as_mut_buf |buf, _| {
+            if checked {
+                check_for_null(v, buf as *mut libc::c_char);
+            }
+
+            f(buf as *libc::c_char)
+        }
+    } else if checked {
+        v.to_c_str().with_ref(f)
+    } else {
+        v.to_c_str_unchecked().with_ref(f)
+    }
+}
+
+#[inline]
+fn check_for_null(v: &[u8], buf: *mut libc::c_char) {
+    for i in range(0, v.len()) {
+        unsafe {
+            let p = buf.offset(i as int);
+            if *p == 0 {
+                match null_byte::cond.raise(v.to_owned()) {
+                    Truncate => break,
+                    ReplaceWith(c) => *p = c
+                }
+            }
+        }
+    }
 }
 
 /// External iterator for a CString's bytes.
@@ -471,3 +528,131 @@ mod tests {
         assert_eq!(c_str.as_str(), None);
     }
 }
+
+#[cfg(test)]
+mod bench {
+    use iter::range;
+    use libc;
+    use option::Some;
+    use ptr;
+    use extra::test::BenchHarness;
+
+    #[inline]
+    fn check(s: &str, c_str: *libc::c_char) {
+        do s.as_imm_buf |s_buf, s_len| {
+            for i in range(0, s_len) {
+                unsafe {
+                    assert_eq!(
+                        *ptr::offset(s_buf, i as int) as libc::c_char,
+                        *ptr::offset(c_str, i as int));
+                }
+            }
+        }
+    }
+
+    static s_short: &'static str = "Mary";
+    static s_medium: &'static str = "Mary had a little lamb";
+    static s_long: &'static str = "\
+        Mary had a little lamb, Little lamb
+        Mary had a little lamb, Little lamb
+        Mary had a little lamb, Little lamb
+        Mary had a little lamb, Little lamb
+        Mary had a little lamb, Little lamb
+        Mary had a little lamb, Little lamb";
+
+    fn bench_to_str(bh: &mut BenchHarness, s: &str) {
+        do bh.iter {
+            let c_str = s.to_c_str();
+            do c_str.with_ref |c_str_buf| {
+                check(s, c_str_buf)
+            }
+        }
+    }
+
+    #[bench]
+    fn bench_to_c_str_short(bh: &mut BenchHarness) {
+        bench_to_str(bh, s_short)
+    }
+
+    #[bench]
+    fn bench_to_c_str_medium(bh: &mut BenchHarness) {
+        bench_to_str(bh, s_medium)
+    }
+
+    #[bench]
+    fn bench_to_c_str_long(bh: &mut BenchHarness) {
+        bench_to_str(bh, s_long)
+    }
+
+    fn bench_to_c_str_unchecked(bh: &mut BenchHarness, s: &str) {
+        do bh.iter {
+            let c_str = unsafe { s.to_c_str_unchecked() };
+            do c_str.with_ref |c_str_buf| {
+                check(s, c_str_buf)
+            }
+        }
+    }
+
+    #[bench]
+    fn bench_to_c_str_unchecked_short(bh: &mut BenchHarness) {
+        bench_to_c_str_unchecked(bh, s_short)
+    }
+
+    #[bench]
+    fn bench_to_c_str_unchecked_medium(bh: &mut BenchHarness) {
+        bench_to_c_str_unchecked(bh, s_medium)
+    }
+
+    #[bench]
+    fn bench_to_c_str_unchecked_long(bh: &mut BenchHarness) {
+        bench_to_c_str_unchecked(bh, s_long)
+    }
+
+    fn bench_with_c_str(bh: &mut BenchHarness, s: &str) {
+        do bh.iter {
+            do s.with_c_str |c_str_buf| {
+                check(s, c_str_buf)
+            }
+        }
+    }
+
+    #[bench]
+    fn bench_with_c_str_short(bh: &mut BenchHarness) {
+        bench_with_c_str(bh, s_short)
+    }
+
+    #[bench]
+    fn bench_with_c_str_medium(bh: &mut BenchHarness) {
+        bench_with_c_str(bh, s_medium)
+    }
+
+    #[bench]
+    fn bench_with_c_str_long(bh: &mut BenchHarness) {
+        bench_with_c_str(bh, s_long)
+    }
+
+    fn bench_with_c_str_unchecked(bh: &mut BenchHarness, s: &str) {
+        do bh.iter {
+            unsafe {
+                do s.with_c_str_unchecked |c_str_buf| {
+                    check(s, c_str_buf)
+                }
+            }
+        }
+    }
+
+    #[bench]
+    fn bench_with_c_str_unchecked_short(bh: &mut BenchHarness) {
+        bench_with_c_str_unchecked(bh, s_short)
+    }
+
+    #[bench]
+    fn bench_with_c_str_unchecked_medium(bh: &mut BenchHarness) {
+        bench_with_c_str_unchecked(bh, s_medium)
+    }
+
+    #[bench]
+    fn bench_with_c_str_unchecked_long(bh: &mut BenchHarness) {
+        bench_with_c_str_unchecked(bh, s_long)
+    }
+}

src/libstd/rt/crate_map.rs

@@ -209,7 +209,7 @@ fn iter_crate_map_follow_children() {
         let child_crate1 = CrateMapT2 {
             version: 1,
             entries: vec::raw::to_ptr([
-                ModEntry { name: "t::f1".to_c_str().with_ref(|buf| buf), log_level: &mut 1},
+                ModEntry { name: "t::f1".with_c_str(|buf| buf), log_level: &mut 1},
                 ModEntry { name: ptr::null(), log_level: ptr::mut_null()}
             ]),
             children: [&child_crate2 as *CrateMap, ptr::null()]
@@ -219,7 +219,7 @@ fn iter_crate_map_follow_children() {
         let root_crate = CrateMapT2 {
             version: 1,
             entries: vec::raw::to_ptr([
-                ModEntry { name: "t::f1".to_c_str().with_ref(|buf| buf), log_level: &mut 0},
+                ModEntry { name: "t::f1".with_c_str(|buf| buf), log_level: &mut 0},
                 ModEntry { name: ptr::null(), log_level: ptr::mut_null()}
             ]),
             children: [child_crate1_ptr, ptr::null()]

src/libstd/rt/logging.rs

@@ -294,7 +294,7 @@ fn update_entry_match_full_path() {
                  LogDirective {name: Some(~"crate2"), level: 3}];
     let level = &mut 0;
     unsafe {
-        do "crate1::mod1".to_c_str().with_ref |ptr| {
+        do "crate1::mod1".with_c_str |ptr| {
             let entry= &ModEntry {name: ptr, log_level: level};
             let m = update_entry(dirs, transmute(entry));
             assert!(*entry.log_level == 2);
@@ -310,7 +310,7 @@ fn update_entry_no_match() {
                  LogDirective {name: Some(~"crate2"), level: 3}];
     let level = &mut 0;
     unsafe {
-        do "crate3::mod1".to_c_str().with_ref |ptr| {
+        do "crate3::mod1".with_c_str |ptr| {
             let entry= &ModEntry {name: ptr, log_level: level};
             let m = update_entry(dirs, transmute(entry));
             assert!(*entry.log_level == DEFAULT_LOG_LEVEL);
@@ -326,7 +326,7 @@ fn update_entry_match_beginning() {
                  LogDirective {name: Some(~"crate2"), level: 3}];
     let level = &mut 0;
     unsafe {
-        do "crate2::mod1".to_c_str().with_ref |ptr| {
+        do "crate2::mod1".with_c_str |ptr| {
             let entry= &ModEntry {name: ptr, log_level: level};
             let m = update_entry(dirs, transmute(entry));
             assert!(*entry.log_level == 3);
@@ -343,7 +343,7 @@ fn update_entry_match_beginning_longest_match() {
                  LogDirective {name: Some(~"crate2::mod"), level: 4}];
     let level = &mut 0;
     unsafe {
-        do "crate2::mod1".to_c_str().with_ref |ptr| {
+        do "crate2::mod1".with_c_str |ptr| {
             let entry = &ModEntry {name: ptr, log_level: level};
             let m = update_entry(dirs, transmute(entry));
             assert!(*entry.log_level == 4);
@@ -360,13 +360,13 @@ fn update_entry_match_default() {
                 ];
     let level = &mut 0;
     unsafe {
-        do "crate1::mod1".to_c_str().with_ref |ptr| {
+        do "crate1::mod1".with_c_str |ptr| {
             let entry= &ModEntry {name: ptr, log_level: level};
             let m = update_entry(dirs, transmute(entry));
             assert!(*entry.log_level == 2);
             assert!(m == 1);
         }
-        do "crate2::mod2".to_c_str().with_ref |ptr| {
+        do "crate2::mod2".with_c_str |ptr| {
             let entry= &ModEntry {name: ptr, log_level: level};
             let m = update_entry(dirs, transmute(entry));
             assert!(*entry.log_level == 3);

src/libstd/rt/uv/file.rs

@@ -43,7 +43,7 @@ impl FsRequest {
             me.req_boilerplate(Some(cb))
         };
         path.path_as_str(|p| {
-            p.to_c_str().with_ref(|p| unsafe {
+            p.with_c_str(|p| unsafe {
             uvll::fs_open(loop_.native_handle(),
                           self.native_handle(), p, flags, mode, complete_cb_ptr)
             })
@@ -57,7 +57,7 @@ impl FsRequest {
             me.req_boilerplate(None)
         };
         let result = path.path_as_str(|p| {
-            p.to_c_str().with_ref(|p| unsafe {
+            p.with_c_str(|p| unsafe {
             uvll::fs_open(loop_.native_handle(),
                     self.native_handle(), p, flags, mode, complete_cb_ptr)
             })
@@ -71,7 +71,7 @@ impl FsRequest {
             me.req_boilerplate(Some(cb))
         };
         path.path_as_str(|p| {
-            p.to_c_str().with_ref(|p| unsafe {
+            p.with_c_str(|p| unsafe {
                 uvll::fs_unlink(loop_.native_handle(),
                               self.native_handle(), p, complete_cb_ptr)
             })
@@ -85,7 +85,7 @@ impl FsRequest {
             me.req_boilerplate(None)
         };
         let result = path.path_as_str(|p| {
-            p.to_c_str().with_ref(|p| unsafe {
+            p.with_c_str(|p| unsafe {
                 uvll::fs_unlink(loop_.native_handle(),
                               self.native_handle(), p, complete_cb_ptr)
             })
@@ -99,7 +99,7 @@ impl FsRequest {
             me.req_boilerplate(Some(cb))
         };
         path.path_as_str(|p| {
-            p.to_c_str().with_ref(|p| unsafe {
+            p.with_c_str(|p| unsafe {
                 uvll::fs_stat(loop_.native_handle(),
                               self.native_handle(), p, complete_cb_ptr)
             })
@@ -192,7 +192,7 @@ impl FsRequest {
             me.req_boilerplate(Some(cb))
         };
         path.path_as_str(|p| {
-            p.to_c_str().with_ref(|p| unsafe {
+            p.with_c_str(|p| unsafe {
             uvll::fs_mkdir(loop_.native_handle(),
                           self.native_handle(), p, mode, complete_cb_ptr)
             })
@@ -205,7 +205,7 @@ impl FsRequest {
             me.req_boilerplate(Some(cb))
         };
         path.path_as_str(|p| {
-            p.to_c_str().with_ref(|p| unsafe {
+            p.with_c_str(|p| unsafe {
             uvll::fs_rmdir(loop_.native_handle(),
                           self.native_handle(), p, complete_cb_ptr)
             })
@@ -219,7 +219,7 @@ impl FsRequest {
             me.req_boilerplate(Some(cb))
         };
         path.path_as_str(|p| {
-            p.to_c_str().with_ref(|p| unsafe {
+            p.with_c_str(|p| unsafe {
             uvll::fs_readdir(loop_.native_handle(),
                           self.native_handle(), p, flags, complete_cb_ptr)
             })
@@ -370,8 +370,7 @@ mod test {
     use unstable::run_in_bare_thread;
     use path::Path;
     use rt::uv::{Loop, Buf, slice_to_uv_buf};
-    use libc::{c_int, O_CREAT, O_RDWR, O_RDONLY,
-               S_IWUSR, S_IRUSR};
+    use libc::{O_CREAT, O_RDWR, O_RDONLY, S_IWUSR, S_IRUSR};
 
     #[test]
     fn file_test_full_simple() {
@@ -603,7 +602,7 @@ mod test {
                         assert!(uverr.is_none());
                         let loop_ = req.get_loop();
                         let stat_req = FsRequest::new();
-                        do stat_req.stat(&loop_, &path) |req, uverr| {
+                        do stat_req.stat(&loop_, &path) |_req, uverr| {
                             assert!(uverr.is_some());
                         }
                     }
@@ -628,11 +627,11 @@ mod test {
                 do mkdir_req.mkdir(&loop_, &path, mode as int) |req,uverr| {
                     assert!(uverr.is_some());
                     let loop_ = req.get_loop();
-                    let stat = req.get_stat();
+                    let _stat = req.get_stat();
                     let rmdir_req = FsRequest::new();
                     do rmdir_req.rmdir(&loop_, &path) |req,uverr| {
                         assert!(uverr.is_none());
-                        let loop_ = req.get_loop();
+                        let _loop = req.get_loop();
                     }
                 }
             }
@@ -646,7 +645,7 @@ mod test {
             let mut loop_ = Loop::new();
             let path = "./tmp/never_existed_dir";
             let rmdir_req = FsRequest::new();
-            do rmdir_req.rmdir(&loop_, &path) |req,uverr| {
+            do rmdir_req.rmdir(&loop_, &path) |_req, uverr| {
                 assert!(uverr.is_some());
             }
             loop_.run();

src/libstd/str.rs

@@ -1222,7 +1222,7 @@ pub mod raw {
         unsafe {
             let input = bytes!("zero", "\x00", "one", "\x00", "\x00");
             let ptr = vec::raw::to_ptr(input);
-            let mut result = from_c_multistring(ptr as *libc::c_char, None);
+            let result = from_c_multistring(ptr as *libc::c_char, None);
             assert!(result.len() == 2);
             let mut ctr = 0;
             for x in result.iter() {