From 4834661c66658196f5f4dd60b751aae2d0daf0ec Mon Sep 17 00:00:00 2001 From: Erick Tryzelaar Date: Wed, 18 Sep 2013 10:54:43 -0700 Subject: [PATCH 1/9] std and rustc: Convert users of c_str to use .with_c_str --- src/librustc/middle/trans/base.rs | 4 ++-- src/librustc/middle/trans/debuginfo.rs | 4 ++-- src/librustc/middle/trans/foreign.rs | 4 ++-- src/libstd/rt/crate_map.rs | 4 ++-- src/libstd/rt/logging.rs | 12 ++++++------ src/libstd/rt/uv/file.rs | 16 ++++++++-------- 6 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index 479f10a3b4a..bb793dd2155 100644 --- a/src/librustc/middle/trans/base.rs +++ b/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); } } } diff --git a/src/librustc/middle/trans/debuginfo.rs b/src/librustc/middle/trans/debuginfo.rs index 91f6169b419..c36d427a06c 100644 --- a/src/librustc/middle/trans/debuginfo.rs +++ b/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), diff --git a/src/librustc/middle/trans/foreign.rs b/src/librustc/middle/trans/foreign.rs index b00d77d72dd..f28f5449e00 100644 --- a/src/librustc/middle/trans/foreign.rs +++ b/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)) diff --git a/src/libstd/rt/crate_map.rs b/src/libstd/rt/crate_map.rs index 57abb7560a1..94d37f67ba1 100644 --- a/src/libstd/rt/crate_map.rs +++ b/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()] diff --git a/src/libstd/rt/logging.rs b/src/libstd/rt/logging.rs index cfbc53ad34e..abbcd27e8ca 100644 --- a/src/libstd/rt/logging.rs +++ b/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); diff --git a/src/libstd/rt/uv/file.rs b/src/libstd/rt/uv/file.rs index 54cb40c9873..fcd8800fadd 100644 --- a/src/libstd/rt/uv/file.rs +++ b/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) }) From 0bdc99d81f038fe0e480eeb87b380b11b8e72ba7 Mon Sep 17 00:00:00 2001 From: Erick Tryzelaar Date: Wed, 18 Sep 2013 11:45:17 -0700 Subject: [PATCH 2/9] std: removed some warnings in tests. --- src/libstd/rt/uv/file.rs | 11 +++++------ src/libstd/str.rs | 2 +- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/libstd/rt/uv/file.rs b/src/libstd/rt/uv/file.rs index fcd8800fadd..f37402d944e 100644 --- a/src/libstd/rt/uv/file.rs +++ b/src/libstd/rt/uv/file.rs @@ -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(); diff --git a/src/libstd/str.rs b/src/libstd/str.rs index 9562346c70d..505b1d20d01 100644 --- a/src/libstd/str.rs +++ b/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() { From 2a9e7633043a79388fe3ed40c61a3961c2aedbcb Mon Sep 17 00:00:00 2001 From: Erick Tryzelaar Date: Wed, 18 Sep 2013 12:21:30 -0700 Subject: [PATCH 3/9] std: implement Container for CString --- src/libstd/c_str.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/libstd/c_str.rs b/src/libstd/c_str.rs index 823cc0db4b9..82726627bc8 100644 --- a/src/libstd/c_str.rs +++ b/src/libstd/c_str.rs @@ -152,8 +152,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 +186,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. From 410a96cc79df4fbb3c2f30bdb2682b867df41a7e Mon Sep 17 00:00:00 2001 From: Erick Tryzelaar Date: Wed, 18 Sep 2013 12:32:45 -0700 Subject: [PATCH 4/9] std: Add benchmarks to c_str --- src/libstd/c_str.rs | 103 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/src/libstd/c_str.rs b/src/libstd/c_str.rs index 82726627bc8..3506faadd3e 100644 --- a/src/libstd/c_str.rs +++ b/src/libstd/c_str.rs @@ -479,3 +479,106 @@ 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) + } +} From e02d1eb1717fee3c513baa0cb5fc39b8fa630335 Mon Sep 17 00:00:00 2001 From: Erick Tryzelaar Date: Wed, 18 Sep 2013 12:32:35 -0700 Subject: [PATCH 5/9] std: Micro-optimize vec.with_c_str for short vectors This now makes it unsafe to save the pointer returned by .with_c_str as that pointer now may be pointing at a stack allocated array. I arbitrarily chose 32 bytes as the length of the stack vector, and so it might not be the most optimal size. before: test c_str::bench::bench_with_c_str_long ... bench: 539 ns/iter (+/- 91) test c_str::bench::bench_with_c_str_medium ... bench: 97 ns/iter (+/- 2) test c_str::bench::bench_with_c_str_short ... bench: 70 ns/iter (+/- 5) after: test c_str::bench::bench_with_c_str_long ... bench: 542 ns/iter (+/- 13) test c_str::bench::bench_with_c_str_medium ... bench: 53 ns/iter (+/- 6) test c_str::bench::bench_with_c_str_short ... bench: 19 ns/iter (+/- 0) --- src/libstd/c_str.rs | 66 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 52 insertions(+), 14 deletions(-) diff --git a/src/libstd/c_str.rs b/src/libstd/c_str.rs index 3506faadd3e..8b5b780012b 100644 --- a/src/libstd/c_str.rs +++ b/src/libstd/c_str.rs @@ -61,16 +61,17 @@ 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 unstable::intrinsics; /// Resolution options for the `null_byte` condition pub enum NullByteResolution { @@ -241,24 +242,22 @@ 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(&self, f: &fn(*libc::c_char) -> T) -> T { + self.as_bytes().with_c_str(f) + } } +// The length of the stack allocated buffer for `vec.with_c_str()` +static BUF_LEN: uint = 32; + 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 } @@ -277,6 +276,45 @@ impl<'self> ToCStr for &'self [u8] { CString::new(buf as *libc::c_char, true) } } + + /// WARNING: This function uses an optimization to only malloc a temporary + /// CString when the source string is small. Do not save a reference to + /// the `*libc::c_char` as it may be invalid after this function call. + fn with_c_str(&self, f: &fn(*libc::c_char) -> T) -> T { + if self.len() < BUF_LEN { + do self.as_imm_buf |self_buf, self_len| { + unsafe { + let mut buf: [u8, .. BUF_LEN] = intrinsics::uninit(); + + do buf.as_mut_buf |buf, _| { + ptr::copy_memory(buf, self_buf, self_len); + *ptr::mut_offset(buf, self_len as int) = 0; + + check_for_null(*self, buf as *mut libc::c_char); + + f(buf as *libc::c_char) + } + } + } + } else { + self.to_c_str().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. From ca66b8128391a2423123a873f800139a55e194bf Mon Sep 17 00:00:00 2001 From: Erick Tryzelaar Date: Fri, 20 Sep 2013 13:24:23 -0700 Subject: [PATCH 6/9] std: Remove an unnecessary comment from c_str The documentation for `.with_c_str()` already says that the pointer will be deallocated before returning from the function. --- src/libstd/c_str.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/libstd/c_str.rs b/src/libstd/c_str.rs index 8b5b780012b..49db9e4f3b0 100644 --- a/src/libstd/c_str.rs +++ b/src/libstd/c_str.rs @@ -277,9 +277,6 @@ impl<'self> ToCStr for &'self [u8] { } } - /// WARNING: This function uses an optimization to only malloc a temporary - /// CString when the source string is small. Do not save a reference to - /// the `*libc::c_char` as it may be invalid after this function call. fn with_c_str(&self, f: &fn(*libc::c_char) -> T) -> T { if self.len() < BUF_LEN { do self.as_imm_buf |self_buf, self_len| { From 4868273d97958d832fc7d0681be096bb265ae6ba Mon Sep 17 00:00:00 2001 From: Erick Tryzelaar Date: Fri, 20 Sep 2013 13:25:02 -0700 Subject: [PATCH 7/9] std: Up vec.with_c_str's stack buffer to 128 This matches @graydon's recommendation. --- src/libstd/c_str.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstd/c_str.rs b/src/libstd/c_str.rs index 49db9e4f3b0..913a0cff47f 100644 --- a/src/libstd/c_str.rs +++ b/src/libstd/c_str.rs @@ -250,7 +250,7 @@ impl<'self> ToCStr for &'self str { } // The length of the stack allocated buffer for `vec.with_c_str()` -static BUF_LEN: uint = 32; +static BUF_LEN: uint = 128; impl<'self> ToCStr for &'self [u8] { fn to_c_str(&self) -> CString { From 2d878033fd77f13a41bb9142ba2a4e9b976d0089 Mon Sep 17 00:00:00 2001 From: Erick Tryzelaar Date: Fri, 20 Sep 2013 16:48:07 -0700 Subject: [PATCH 8/9] std: add micro optimization to vec.with_c_str_unchecked before: test c_str::bench::bench_with_c_str_unchecked_long ... bench: 361 ns/iter (+/- 9) test c_str::bench::bench_with_c_str_unchecked_medium ... bench: 75 ns/iter (+/- 2) test c_str::bench::bench_with_c_str_unchecked_short ... bench: 60 ns/iter (+/- 9) after: test c_str::bench::bench_with_c_str_unchecked_long ... bench: 362 ns/iter (+/- test c_str::bench::bench_with_c_str_unchecked_medium ... bench: 30 ns/iter (+/- 7) test c_str::bench::bench_with_c_str_unchecked_short ... bench: 12 ns/iter (+/- 4) --- src/libstd/c_str.rs | 47 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/src/libstd/c_str.rs b/src/libstd/c_str.rs index 913a0cff47f..e789e9a3bf1 100644 --- a/src/libstd/c_str.rs +++ b/src/libstd/c_str.rs @@ -247,6 +247,11 @@ impl<'self> ToCStr for &'self str { fn with_c_str(&self, f: &fn(*libc::c_char) -> T) -> T { self.as_bytes().with_c_str(f) } + + #[inline] + unsafe fn with_c_str_unchecked(&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()` @@ -297,6 +302,23 @@ impl<'self> ToCStr for &'self [u8] { self.to_c_str().with_ref(f) } } + + unsafe fn with_c_str_unchecked(&self, f: &fn(*libc::c_char) -> T) -> T { + if self.len() < BUF_LEN { + do self.as_imm_buf |self_buf, self_len| { + let mut buf: [u8, .. BUF_LEN] = intrinsics::uninit(); + + do buf.as_mut_buf |buf, _| { + ptr::copy_memory(buf, self_buf, self_len); + *ptr::mut_offset(buf, self_len as int) = 0; + + f(buf as *libc::c_char) + } + } + } else { + self.to_c_str().with_ref(f) + } + } } #[inline] @@ -616,4 +638,29 @@ mod 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) + } } From b1ee87f402f929689fc028010c450184f661db3b Mon Sep 17 00:00:00 2001 From: Erick Tryzelaar Date: Thu, 26 Sep 2013 22:49:10 -0700 Subject: [PATCH 9/9] std: simplify vec.with_c_str This also fixes a bug in `vec.with_c_str_unchecked` where we were not calling `.to_c_str_unchecked()` for long strings. --- src/libstd/c_str.rs | 48 +++++++++++++++++++-------------------------- 1 file changed, 20 insertions(+), 28 deletions(-) diff --git a/src/libstd/c_str.rs b/src/libstd/c_str.rs index e789e9a3bf1..acfa02a4def 100644 --- a/src/libstd/c_str.rs +++ b/src/libstd/c_str.rs @@ -71,6 +71,7 @@ use ptr; use str::StrSlice; use str; use vec::{CopyableVector, ImmutableVector, MutableVector}; +use vec; use unstable::intrinsics; /// Resolution options for the `null_byte` condition @@ -283,41 +284,32 @@ impl<'self> ToCStr for &'self [u8] { } fn with_c_str(&self, f: &fn(*libc::c_char) -> T) -> T { - if self.len() < BUF_LEN { - do self.as_imm_buf |self_buf, self_len| { - unsafe { - let mut buf: [u8, .. BUF_LEN] = intrinsics::uninit(); - - do buf.as_mut_buf |buf, _| { - ptr::copy_memory(buf, self_buf, self_len); - *ptr::mut_offset(buf, self_len as int) = 0; - - check_for_null(*self, buf as *mut libc::c_char); - - f(buf as *libc::c_char) - } - } - } - } else { - self.to_c_str().with_ref(f) - } + unsafe { with_c_str(*self, true, f) } } unsafe fn with_c_str_unchecked(&self, f: &fn(*libc::c_char) -> T) -> T { - if self.len() < BUF_LEN { - do self.as_imm_buf |self_buf, self_len| { - let mut buf: [u8, .. BUF_LEN] = intrinsics::uninit(); + with_c_str(*self, false, f) + } +} - do buf.as_mut_buf |buf, _| { - ptr::copy_memory(buf, self_buf, self_len); - *ptr::mut_offset(buf, self_len as int) = 0; +// Unsafe function that handles possibly copying the &[u8] into a stack array. +unsafe fn with_c_str(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; - f(buf as *libc::c_char) - } + do buf.as_mut_buf |buf, _| { + if checked { + check_for_null(v, buf as *mut libc::c_char); } - } else { - self.to_c_str().with_ref(f) + + 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) } }