Skip to content

Commit

Permalink
Auto merge of rust-lang#121101 - GnomedDev:dyn-small-c-string, r=Nils…
Browse files Browse the repository at this point in the history 
…trieb

Reduce monomorphisation bloat in small_c_string

This is a code path usually next to an FFI call, so taking the `dyn` slowdown for the 1159 llvm-line (fat lto, codegen-units 1, release build) drop in my testing program [t2fanrd](https://github.com/GnomedDev/t2fanrd) is worth it imo.
  • Loading branch information
@bors
bors committed Feb 18, 2024
2 parents 2bf78d1 + dbb15fb commit 6122397
Show file tree
Hide file tree
Showing 10 changed files with 64 additions and 60 deletions.
29 changes: 16 additions & 13 deletions library/std/src/sys/pal/common/small_c_string.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,28 @@ const NUL_ERR: io::Error =
io::const_io_error!(io::ErrorKind::InvalidInput, "file name contained an unexpected NUL byte");

#[inline]
pub fn run_path_with_cstr<T, F>(path: &Path, f: F) -> io::Result<T>
where
F: FnOnce(&CStr) -> io::Result<T>,
{
pub fn run_path_with_cstr<T>(path: &Path, f: &dyn Fn(&CStr) -> io::Result<T>) -> io::Result<T> {
run_with_cstr(path.as_os_str().as_encoded_bytes(), f)
}

#[inline]
pub fn run_with_cstr<T, F>(bytes: &[u8], f: F) -> io::Result<T>
where
F: FnOnce(&CStr) -> io::Result<T>,
{
pub fn run_with_cstr<T>(bytes: &[u8], f: &dyn Fn(&CStr) -> io::Result<T>) -> io::Result<T> {
// Dispatch and dyn erase the closure type to prevent mono bloat.
// See https://github.com/rust-lang/rust/pull/121101.
if bytes.len() >= MAX_STACK_ALLOCATION {
return run_with_cstr_allocating(bytes, f);
run_with_cstr_allocating(bytes, f)
} else {
unsafe { run_with_cstr_stack(bytes, f) }
}
}

/// # Safety
///
/// `bytes` must have a length less than `MAX_STACK_ALLOCATION`.
unsafe fn run_with_cstr_stack<T>(
bytes: &[u8],
f: &dyn Fn(&CStr) -> io::Result<T>,
) -> io::Result<T> {
let mut buf = MaybeUninit::<[u8; MAX_STACK_ALLOCATION]>::uninit();
let buf_ptr = buf.as_mut_ptr() as *mut u8;

Expand All @@ -47,10 +53,7 @@ where

#[cold]
#[inline(never)]
fn run_with_cstr_allocating<T, F>(bytes: &[u8], f: F) -> io::Result<T>
where
F: FnOnce(&CStr) -> io::Result<T>,
{
fn run_with_cstr_allocating<T>(bytes: &[u8], f: &dyn Fn(&CStr) -> io::Result<T>) -> io::Result<T> {
match CString::new(bytes) {
Ok(s) => f(&s),
Err(_) => Err(NUL_ERR),
Expand Down
12 changes: 6 additions & 6 deletions library/std/src/sys/pal/common/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use core::iter::repeat;
#[test]
fn stack_allocation_works() {
let path = Path::new("abc");
let result = run_path_with_cstr(path, |p| {
let result = run_path_with_cstr(path, &|p| {
assert_eq!(p, &*CString::new(path.as_os_str().as_encoded_bytes()).unwrap());
Ok(42)
});
Expand All @@ -17,14 +17,14 @@ fn stack_allocation_works() {
#[test]
fn stack_allocation_fails() {
let path = Path::new("ab\0");
assert!(run_path_with_cstr::<(), _>(path, |_| unreachable!()).is_err());
assert!(run_path_with_cstr::<()>(path, &|_| unreachable!()).is_err());
}

#[test]
fn heap_allocation_works() {
let path = repeat("a").take(384).collect::<String>();
let path = Path::new(&path);
let result = run_path_with_cstr(path, |p| {
let result = run_path_with_cstr(path, &|p| {
assert_eq!(p, &*CString::new(path.as_os_str().as_encoded_bytes()).unwrap());
Ok(42)
});
Expand All @@ -36,15 +36,15 @@ fn heap_allocation_fails() {
let mut path = repeat("a").take(384).collect::<String>();
path.push('\0');
let path = Path::new(&path);
assert!(run_path_with_cstr::<(), _>(path, |_| unreachable!()).is_err());
assert!(run_path_with_cstr::<()>(path, &|_| unreachable!()).is_err());
}

#[bench]
fn bench_stack_path_alloc(b: &mut test::Bencher) {
let path = repeat("a").take(383).collect::<String>();
let p = Path::new(&path);
b.iter(|| {
run_path_with_cstr(p, |cstr| {
run_path_with_cstr(p, &|cstr| {
black_box(cstr);
Ok(())
})
Expand All @@ -57,7 +57,7 @@ fn bench_heap_path_alloc(b: &mut test::Bencher) {
let path = repeat("a").take(384).collect::<String>();
let p = Path::new(&path);
b.iter(|| {
run_path_with_cstr(p, |cstr| {
run_path_with_cstr(p, &|cstr| {
black_box(cstr);
Ok(())
})
Expand Down
4 changes: 2 additions & 2 deletions library/std/src/sys/pal/hermit/fs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ impl OpenOptions {

impl File {
pub fn open(path: &Path, opts: &OpenOptions) -> io::Result<File> {
run_path_with_cstr(path, |path| File::open_c(&path, opts))
run_path_with_cstr(path, &|path| File::open_c(&path, opts))
}

pub fn open_c(path: &CStr, opts: &OpenOptions) -> io::Result<File> {
Expand Down Expand Up @@ -421,7 +421,7 @@ pub fn readdir(_p: &Path) -> io::Result<ReadDir> {
}

pub fn unlink(path: &Path) -> io::Result<()> {
run_path_with_cstr(path, |path| cvt(unsafe { abi::unlink(path.as_ptr()) }).map(|_| ()))
run_path_with_cstr(path, &|path| cvt(unsafe { abi::unlink(path.as_ptr()) }).map(|_| ()))
}

pub fn rename(_old: &Path, _new: &Path) -> io::Result<()> {
Expand Down
8 changes: 4 additions & 4 deletions library/std/src/sys/pal/solid/os.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ pub fn env() -> Env {
pub fn getenv(k: &OsStr) -> Option<OsString> {
// environment variables with a nul byte can't be set, so their value is
// always None as well
run_with_cstr(k.as_bytes(), |k| {
run_with_cstr(k.as_bytes(), &|k| {
let _guard = env_read_lock();
let v = unsafe { libc::getenv(k.as_ptr()) } as *const libc::c_char;

Expand All @@ -190,16 +190,16 @@ pub fn getenv(k: &OsStr) -> Option<OsString> {
}

pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
run_with_cstr(k.as_bytes(), |k| {
run_with_cstr(v.as_bytes(), |v| {
run_with_cstr(k.as_bytes(), &|k| {
run_with_cstr(v.as_bytes(), &|v| {
let _guard = ENV_LOCK.write();
cvt_env(unsafe { libc::setenv(k.as_ptr(), v.as_ptr(), 1) }).map(drop)
})
})
}

pub fn unsetenv(n: &OsStr) -> io::Result<()> {
run_with_cstr(n.as_bytes(), |nbuf| {
run_with_cstr(n.as_bytes(), &|nbuf| {
let _guard = ENV_LOCK.write();
cvt_env(unsafe { libc::unsetenv(nbuf.as_ptr()) }).map(drop)
})
Expand Down
42 changes: 21 additions & 21 deletions library/std/src/sys/pal/unix/fs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1118,7 +1118,7 @@ impl OpenOptions {

impl File {
pub fn open(path: &Path, opts: &OpenOptions) -> io::Result<File> {
run_path_with_cstr(path, |path| File::open_c(path, opts))
run_path_with_cstr(path, &|path| File::open_c(path, opts))
}

pub fn open_c(path: &CStr, opts: &OpenOptions) -> io::Result<File> {
Expand Down Expand Up @@ -1394,7 +1394,7 @@ impl DirBuilder {
}

pub fn mkdir(&self, p: &Path) -> io::Result<()> {
run_path_with_cstr(p, |p| cvt(unsafe { libc::mkdir(p.as_ptr(), self.mode) }).map(|_| ()))
run_path_with_cstr(p, &|p| cvt(unsafe { libc::mkdir(p.as_ptr(), self.mode) }).map(|_| ()))
}

pub fn set_mode(&mut self, mode: u32) {
Expand Down Expand Up @@ -1575,7 +1575,7 @@ impl fmt::Debug for File {
}

pub fn readdir(path: &Path) -> io::Result<ReadDir> {
let ptr = run_path_with_cstr(path, |p| unsafe { Ok(libc::opendir(p.as_ptr())) })?;
let ptr = run_path_with_cstr(path, &|p| unsafe { Ok(libc::opendir(p.as_ptr())) })?;
if ptr.is_null() {
Err(Error::last_os_error())
} else {
Expand All @@ -1586,27 +1586,27 @@ pub fn readdir(path: &Path) -> io::Result<ReadDir> {
}

pub fn unlink(p: &Path) -> io::Result<()> {
run_path_with_cstr(p, |p| cvt(unsafe { libc::unlink(p.as_ptr()) }).map(|_| ()))
run_path_with_cstr(p, &|p| cvt(unsafe { libc::unlink(p.as_ptr()) }).map(|_| ()))
}

pub fn rename(old: &Path, new: &Path) -> io::Result<()> {
run_path_with_cstr(old, |old| {
run_path_with_cstr(new, |new| {
run_path_with_cstr(old, &|old| {
run_path_with_cstr(new, &|new| {
cvt(unsafe { libc::rename(old.as_ptr(), new.as_ptr()) }).map(|_| ())
})
})
}

pub fn set_perm(p: &Path, perm: FilePermissions) -> io::Result<()> {
run_path_with_cstr(p, |p| cvt_r(|| unsafe { libc::chmod(p.as_ptr(), perm.mode) }).map(|_| ()))
run_path_with_cstr(p, &|p| cvt_r(|| unsafe { libc::chmod(p.as_ptr(), perm.mode) }).map(|_| ()))
}

pub fn rmdir(p: &Path) -> io::Result<()> {
run_path_with_cstr(p, |p| cvt(unsafe { libc::rmdir(p.as_ptr()) }).map(|_| ()))
run_path_with_cstr(p, &|p| cvt(unsafe { libc::rmdir(p.as_ptr()) }).map(|_| ()))
}

pub fn readlink(p: &Path) -> io::Result<PathBuf> {
run_path_with_cstr(p, |c_path| {
run_path_with_cstr(p, &|c_path| {
let p = c_path.as_ptr();

let mut buf = Vec::with_capacity(256);
Expand Down Expand Up @@ -1635,16 +1635,16 @@ pub fn readlink(p: &Path) -> io::Result<PathBuf> {
}

pub fn symlink(original: &Path, link: &Path) -> io::Result<()> {
run_path_with_cstr(original, |original| {
run_path_with_cstr(link, |link| {
run_path_with_cstr(original, &|original| {
run_path_with_cstr(link, &|link| {
cvt(unsafe { libc::symlink(original.as_ptr(), link.as_ptr()) }).map(|_| ())
})
})
}

pub fn link(original: &Path, link: &Path) -> io::Result<()> {
run_path_with_cstr(original, |original| {
run_path_with_cstr(link, |link| {
run_path_with_cstr(original, &|original| {
run_path_with_cstr(link, &|link| {
cfg_if::cfg_if! {
if #[cfg(any(target_os = "vxworks", target_os = "redox", target_os = "android", target_os = "espidf", target_os = "horizon", target_os = "vita"))] {
// VxWorks, Redox and ESP-IDF lack `linkat`, so use `link` instead. POSIX leaves
Expand Down Expand Up @@ -1678,7 +1678,7 @@ pub fn link(original: &Path, link: &Path) -> io::Result<()> {
}

pub fn stat(p: &Path) -> io::Result<FileAttr> {
run_path_with_cstr(p, |p| {
run_path_with_cstr(p, &|p| {
cfg_has_statx! {
if let Some(ret) = unsafe { try_statx(
libc::AT_FDCWD,
Expand All @@ -1697,7 +1697,7 @@ pub fn stat(p: &Path) -> io::Result<FileAttr> {
}

pub fn lstat(p: &Path) -> io::Result<FileAttr> {
run_path_with_cstr(p, |p| {
run_path_with_cstr(p, &|p| {
cfg_has_statx! {
if let Some(ret) = unsafe { try_statx(
libc::AT_FDCWD,
Expand All @@ -1716,7 +1716,7 @@ pub fn lstat(p: &Path) -> io::Result<FileAttr> {
}

pub fn canonicalize(p: &Path) -> io::Result<PathBuf> {
let r = run_path_with_cstr(p, |path| unsafe {
let r = run_path_with_cstr(p, &|path| unsafe {
Ok(libc::realpath(path.as_ptr(), ptr::null_mut()))
})?;
if r.is_null() {
Expand Down Expand Up @@ -1879,7 +1879,7 @@ pub fn copy(from: &Path, to: &Path) -> io::Result<u64> {
// Opportunistically attempt to create a copy-on-write clone of `from`
// using `fclonefileat`.
if HAS_FCLONEFILEAT.load(Ordering::Relaxed) {
let clonefile_result = run_path_with_cstr(to, |to| {
let clonefile_result = run_path_with_cstr(to, &|to| {
cvt(unsafe { fclonefileat(reader.as_raw_fd(), libc::AT_FDCWD, to.as_ptr(), 0) })
});
match clonefile_result {
Expand Down Expand Up @@ -1925,7 +1925,7 @@ pub fn copy(from: &Path, to: &Path) -> io::Result<u64> {
}

pub fn chown(path: &Path, uid: u32, gid: u32) -> io::Result<()> {
run_path_with_cstr(path, |path| {
run_path_with_cstr(path, &|path| {
cvt(unsafe { libc::chown(path.as_ptr(), uid as libc::uid_t, gid as libc::gid_t) })
.map(|_| ())
})
Expand All @@ -1937,15 +1937,15 @@ pub fn fchown(fd: c_int, uid: u32, gid: u32) -> io::Result<()> {
}

pub fn lchown(path: &Path, uid: u32, gid: u32) -> io::Result<()> {
run_path_with_cstr(path, |path| {
run_path_with_cstr(path, &|path| {
cvt(unsafe { libc::lchown(path.as_ptr(), uid as libc::uid_t, gid as libc::gid_t) })
.map(|_| ())
})
}

#[cfg(not(any(target_os = "fuchsia", target_os = "vxworks")))]
pub fn chroot(dir: &Path) -> io::Result<()> {
run_path_with_cstr(dir, |dir| cvt(unsafe { libc::chroot(dir.as_ptr()) }).map(|_| ()))
run_path_with_cstr(dir, &|dir| cvt(unsafe { libc::chroot(dir.as_ptr()) }).map(|_| ()))
}

pub use remove_dir_impl::remove_dir_all;
Expand Down Expand Up @@ -2140,7 +2140,7 @@ mod remove_dir_impl {
if attr.file_type().is_symlink() {
crate::fs::remove_file(p)
} else {
run_path_with_cstr(p, |p| remove_dir_all_recursive(None, &p))
run_path_with_cstr(p, &|p| remove_dir_all_recursive(None, &p))
}
}

Expand Down
10 changes: 5 additions & 5 deletions library/std/src/sys/pal/unix/os.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ pub fn chdir(_p: &path::Path) -> io::Result<()> {

#[cfg(not(target_os = "espidf"))]
pub fn chdir(p: &path::Path) -> io::Result<()> {
let result = run_path_with_cstr(p, |p| unsafe { Ok(libc::chdir(p.as_ptr())) })?;
let result = run_path_with_cstr(p, &|p| unsafe { Ok(libc::chdir(p.as_ptr())) })?;
if result == 0 { Ok(()) } else { Err(io::Error::last_os_error()) }
}

Expand Down Expand Up @@ -643,7 +643,7 @@ pub fn env() -> Env {
pub fn getenv(k: &OsStr) -> Option<OsString> {
// environment variables with a nul byte can't be set, so their value is
// always None as well
run_with_cstr(k.as_bytes(), |k| {
run_with_cstr(k.as_bytes(), &|k| {
let _guard = env_read_lock();
let v = unsafe { libc::getenv(k.as_ptr()) } as *const libc::c_char;

Expand All @@ -661,16 +661,16 @@ pub fn getenv(k: &OsStr) -> Option<OsString> {
}

pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
run_with_cstr(k.as_bytes(), |k| {
run_with_cstr(v.as_bytes(), |v| {
run_with_cstr(k.as_bytes(), &|k| {
run_with_cstr(v.as_bytes(), &|v| {
let _guard = ENV_LOCK.write();
cvt(unsafe { libc::setenv(k.as_ptr(), v.as_ptr(), 1) }).map(drop)
})
})
}

pub fn unsetenv(n: &OsStr) -> io::Result<()> {
run_with_cstr(n.as_bytes(), |nbuf| {
run_with_cstr(n.as_bytes(), &|nbuf| {
let _guard = ENV_LOCK.write();
cvt(unsafe { libc::unsetenv(nbuf.as_ptr()) }).map(drop)
})
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/pal/wasi/fs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,7 @@ fn open_at(fd: &WasiFd, path: &Path, opts: &OpenOptions) -> io::Result<File> {
/// Note that this can fail if `p` doesn't look like it can be opened relative
/// to any pre-opened file descriptor.
fn open_parent(p: &Path) -> io::Result<(ManuallyDrop<WasiFd>, PathBuf)> {
run_path_with_cstr(p, |p| {
run_path_with_cstr(p, &|p| {
let mut buf = Vec::<u8>::with_capacity(512);
loop {
unsafe {
Expand Down
10 changes: 5 additions & 5 deletions library/std/src/sys/pal/wasi/os.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ pub fn getcwd() -> io::Result<PathBuf> {
}

pub fn chdir(p: &path::Path) -> io::Result<()> {
let result = run_path_with_cstr(p, |p| unsafe { Ok(libc::chdir(p.as_ptr())) })?;
let result = run_path_with_cstr(p, &|p| unsafe { Ok(libc::chdir(p.as_ptr())) })?;
match result == (0 as libc::c_int) {
true => Ok(()),
false => Err(io::Error::last_os_error()),
Expand Down Expand Up @@ -227,7 +227,7 @@ pub fn env() -> Env {
pub fn getenv(k: &OsStr) -> Option<OsString> {
// environment variables with a nul byte can't be set, so their value is
// always None as well
run_with_cstr(k.as_bytes(), |k| {
run_with_cstr(k.as_bytes(), &|k| {
let _guard = env_read_lock();
let v = unsafe { libc::getenv(k.as_ptr()) } as *const libc::c_char;

Expand All @@ -245,16 +245,16 @@ pub fn getenv(k: &OsStr) -> Option<OsString> {
}

pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
run_with_cstr(k.as_bytes(), |k| {
run_with_cstr(v.as_bytes(), |v| unsafe {
run_with_cstr(k.as_bytes(), &|k| {
run_with_cstr(v.as_bytes(), &|v| unsafe {
let _guard = env_write_lock();
cvt(libc::setenv(k.as_ptr(), v.as_ptr(), 1)).map(drop)
})
})
}

pub fn unsetenv(n: &OsStr) -> io::Result<()> {
run_with_cstr(n.as_bytes(), |nbuf| unsafe {
run_with_cstr(n.as_bytes(), &|nbuf| unsafe {
let _guard = env_write_lock();
cvt(libc::unsetenv(nbuf.as_ptr())).map(drop)
})
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys_common/net.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ impl<'a> TryFrom<(&'a str, u16)> for LookupHost {
fn try_from((host, port): (&'a str, u16)) -> io::Result<LookupHost> {
init();

run_with_cstr(host.as_bytes(), |c_host| {
run_with_cstr(host.as_bytes(), &|c_host| {
let mut hints: c::addrinfo = unsafe { mem::zeroed() };
hints.ai_socktype = c::SOCK_STREAM;
let mut res = ptr::null_mut();
Expand Down
Loading

0 comments on commit 6122397

Please sign in to comment.