Skip to content

Commit 6122397

Browse files
borsbors
committed
Auto merge of #121101 - GnomedDev:dyn-small-c-string, r=Nilstrieb
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.
2 parents 2bf78d1 + dbb15fb commit 6122397

File tree

10 files changed

+64
-60
lines changed

10 files changed

+64
-60
lines changed

Diff for: ‎library/std/src/sys/pal/common/small_c_string.rs

+16-13
Original file line numberDiff line numberDiff line change
@@ -15,22 +15,28 @@ const NUL_ERR: io::Error =
1515
io::const_io_error!(io::ErrorKind::InvalidInput, "file name contained an unexpected NUL byte");
1616

1717
#[inline]
18-
pub fn run_path_with_cstr<T, F>(path: &Path, f: F) -> io::Result<T>
19-
where
20-
F: FnOnce(&CStr) -> io::Result<T>,
21-
{
18+
pub fn run_path_with_cstr<T>(path: &Path, f: &dyn Fn(&CStr) -> io::Result<T>) -> io::Result<T> {
2219
run_with_cstr(path.as_os_str().as_encoded_bytes(), f)
2320
}
2421

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

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

@@ -47,10 +53,7 @@ where
4753

4854
#[cold]
4955
#[inline(never)]
50-
fn run_with_cstr_allocating<T, F>(bytes: &[u8], f: F) -> io::Result<T>
51-
where
52-
F: FnOnce(&CStr) -> io::Result<T>,
53-
{
56+
fn run_with_cstr_allocating<T>(bytes: &[u8], f: &dyn Fn(&CStr) -> io::Result<T>) -> io::Result<T> {
5457
match CString::new(bytes) {
5558
Ok(s) => f(&s),
5659
Err(_) => Err(NUL_ERR),

Diff for: ‎library/std/src/sys/pal/common/tests.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use core::iter::repeat;
77
#[test]
88
fn stack_allocation_works() {
99
let path = Path::new("abc");
10-
let result = run_path_with_cstr(path, |p| {
10+
let result = run_path_with_cstr(path, &|p| {
1111
assert_eq!(p, &*CString::new(path.as_os_str().as_encoded_bytes()).unwrap());
1212
Ok(42)
1313
});
@@ -17,14 +17,14 @@ fn stack_allocation_works() {
1717
#[test]
1818
fn stack_allocation_fails() {
1919
let path = Path::new("ab\0");
20-
assert!(run_path_with_cstr::<(), _>(path, |_| unreachable!()).is_err());
20+
assert!(run_path_with_cstr::<()>(path, &|_| unreachable!()).is_err());
2121
}
2222

2323
#[test]
2424
fn heap_allocation_works() {
2525
let path = repeat("a").take(384).collect::<String>();
2626
let path = Path::new(&path);
27-
let result = run_path_with_cstr(path, |p| {
27+
let result = run_path_with_cstr(path, &|p| {
2828
assert_eq!(p, &*CString::new(path.as_os_str().as_encoded_bytes()).unwrap());
2929
Ok(42)
3030
});
@@ -36,15 +36,15 @@ fn heap_allocation_fails() {
3636
let mut path = repeat("a").take(384).collect::<String>();
3737
path.push('\0');
3838
let path = Path::new(&path);
39-
assert!(run_path_with_cstr::<(), _>(path, |_| unreachable!()).is_err());
39+
assert!(run_path_with_cstr::<()>(path, &|_| unreachable!()).is_err());
4040
}
4141

4242
#[bench]
4343
fn bench_stack_path_alloc(b: &mut test::Bencher) {
4444
let path = repeat("a").take(383).collect::<String>();
4545
let p = Path::new(&path);
4646
b.iter(|| {
47-
run_path_with_cstr(p, |cstr| {
47+
run_path_with_cstr(p, &|cstr| {
4848
black_box(cstr);
4949
Ok(())
5050
})
@@ -57,7 +57,7 @@ fn bench_heap_path_alloc(b: &mut test::Bencher) {
5757
let path = repeat("a").take(384).collect::<String>();
5858
let p = Path::new(&path);
5959
b.iter(|| {
60-
run_path_with_cstr(p, |cstr| {
60+
run_path_with_cstr(p, &|cstr| {
6161
black_box(cstr);
6262
Ok(())
6363
})

Diff for: ‎library/std/src/sys/pal/hermit/fs.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ impl OpenOptions {
269269

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

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

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

427427
pub fn rename(_old: &Path, _new: &Path) -> io::Result<()> {

Diff for: ‎library/std/src/sys/pal/solid/os.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ pub fn env() -> Env {
172172
pub fn getenv(k: &OsStr) -> Option<OsString> {
173173
// environment variables with a nul byte can't be set, so their value is
174174
// always None as well
175-
run_with_cstr(k.as_bytes(), |k| {
175+
run_with_cstr(k.as_bytes(), &|k| {
176176
let _guard = env_read_lock();
177177
let v = unsafe { libc::getenv(k.as_ptr()) } as *const libc::c_char;
178178

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

192192
pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
193-
run_with_cstr(k.as_bytes(), |k| {
194-
run_with_cstr(v.as_bytes(), |v| {
193+
run_with_cstr(k.as_bytes(), &|k| {
194+
run_with_cstr(v.as_bytes(), &|v| {
195195
let _guard = ENV_LOCK.write();
196196
cvt_env(unsafe { libc::setenv(k.as_ptr(), v.as_ptr(), 1) }).map(drop)
197197
})
198198
})
199199
}
200200

201201
pub fn unsetenv(n: &OsStr) -> io::Result<()> {
202-
run_with_cstr(n.as_bytes(), |nbuf| {
202+
run_with_cstr(n.as_bytes(), &|nbuf| {
203203
let _guard = ENV_LOCK.write();
204204
cvt_env(unsafe { libc::unsetenv(nbuf.as_ptr()) }).map(drop)
205205
})

Diff for: ‎library/std/src/sys/pal/unix/fs.rs

+21-21
Original file line numberDiff line numberDiff line change
@@ -1118,7 +1118,7 @@ impl OpenOptions {
11181118

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

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

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

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

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

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

15921592
pub fn rename(old: &Path, new: &Path) -> io::Result<()> {
1593-
run_path_with_cstr(old, |old| {
1594-
run_path_with_cstr(new, |new| {
1593+
run_path_with_cstr(old, &|old| {
1594+
run_path_with_cstr(new, &|new| {
15951595
cvt(unsafe { libc::rename(old.as_ptr(), new.as_ptr()) }).map(|_| ())
15961596
})
15971597
})
15981598
}
15991599

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

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

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

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

16371637
pub fn symlink(original: &Path, link: &Path) -> io::Result<()> {
1638-
run_path_with_cstr(original, |original| {
1639-
run_path_with_cstr(link, |link| {
1638+
run_path_with_cstr(original, &|original| {
1639+
run_path_with_cstr(link, &|link| {
16401640
cvt(unsafe { libc::symlink(original.as_ptr(), link.as_ptr()) }).map(|_| ())
16411641
})
16421642
})
16431643
}
16441644

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

16801680
pub fn stat(p: &Path) -> io::Result<FileAttr> {
1681-
run_path_with_cstr(p, |p| {
1681+
run_path_with_cstr(p, &|p| {
16821682
cfg_has_statx! {
16831683
if let Some(ret) = unsafe { try_statx(
16841684
libc::AT_FDCWD,
@@ -1697,7 +1697,7 @@ pub fn stat(p: &Path) -> io::Result<FileAttr> {
16971697
}
16981698

16991699
pub fn lstat(p: &Path) -> io::Result<FileAttr> {
1700-
run_path_with_cstr(p, |p| {
1700+
run_path_with_cstr(p, &|p| {
17011701
cfg_has_statx! {
17021702
if let Some(ret) = unsafe { try_statx(
17031703
libc::AT_FDCWD,
@@ -1716,7 +1716,7 @@ pub fn lstat(p: &Path) -> io::Result<FileAttr> {
17161716
}
17171717

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

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

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

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

19511951
pub use remove_dir_impl::remove_dir_all;
@@ -2140,7 +2140,7 @@ mod remove_dir_impl {
21402140
if attr.file_type().is_symlink() {
21412141
crate::fs::remove_file(p)
21422142
} else {
2143-
run_path_with_cstr(p, |p| remove_dir_all_recursive(None, &p))
2143+
run_path_with_cstr(p, &|p| remove_dir_all_recursive(None, &p))
21442144
}
21452145
}
21462146

Diff for: ‎library/std/src/sys/pal/unix/os.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ pub fn chdir(_p: &path::Path) -> io::Result<()> {
186186

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

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

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

663663
pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
664-
run_with_cstr(k.as_bytes(), |k| {
665-
run_with_cstr(v.as_bytes(), |v| {
664+
run_with_cstr(k.as_bytes(), &|k| {
665+
run_with_cstr(v.as_bytes(), &|v| {
666666
let _guard = ENV_LOCK.write();
667667
cvt(unsafe { libc::setenv(k.as_ptr(), v.as_ptr(), 1) }).map(drop)
668668
})
669669
})
670670
}
671671

672672
pub fn unsetenv(n: &OsStr) -> io::Result<()> {
673-
run_with_cstr(n.as_bytes(), |nbuf| {
673+
run_with_cstr(n.as_bytes(), &|nbuf| {
674674
let _guard = ENV_LOCK.write();
675675
cvt(unsafe { libc::unsetenv(nbuf.as_ptr()) }).map(drop)
676676
})

Diff for: ‎library/std/src/sys/pal/wasi/fs.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -698,7 +698,7 @@ fn open_at(fd: &WasiFd, path: &Path, opts: &OpenOptions) -> io::Result<File> {
698698
/// Note that this can fail if `p` doesn't look like it can be opened relative
699699
/// to any pre-opened file descriptor.
700700
fn open_parent(p: &Path) -> io::Result<(ManuallyDrop<WasiFd>, PathBuf)> {
701-
run_path_with_cstr(p, |p| {
701+
run_path_with_cstr(p, &|p| {
702702
let mut buf = Vec::<u8>::with_capacity(512);
703703
loop {
704704
unsafe {

Diff for: ‎library/std/src/sys/pal/wasi/os.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ pub fn getcwd() -> io::Result<PathBuf> {
9595
}
9696

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

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

247247
pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
248-
run_with_cstr(k.as_bytes(), |k| {
249-
run_with_cstr(v.as_bytes(), |v| unsafe {
248+
run_with_cstr(k.as_bytes(), &|k| {
249+
run_with_cstr(v.as_bytes(), &|v| unsafe {
250250
let _guard = env_write_lock();
251251
cvt(libc::setenv(k.as_ptr(), v.as_ptr(), 1)).map(drop)
252252
})
253253
})
254254
}
255255

256256
pub fn unsetenv(n: &OsStr) -> io::Result<()> {
257-
run_with_cstr(n.as_bytes(), |nbuf| unsafe {
257+
run_with_cstr(n.as_bytes(), &|nbuf| unsafe {
258258
let _guard = env_write_lock();
259259
cvt(libc::unsetenv(nbuf.as_ptr())).map(drop)
260260
})

Diff for: ‎library/std/src/sys_common/net.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ impl<'a> TryFrom<(&'a str, u16)> for LookupHost {
199199
fn try_from((host, port): (&'a str, u16)) -> io::Result<LookupHost> {
200200
init();
201201

202-
run_with_cstr(host.as_bytes(), |c_host| {
202+
run_with_cstr(host.as_bytes(), &|c_host| {
203203
let mut hints: c::addrinfo = unsafe { mem::zeroed() };
204204
hints.ai_socktype = c::SOCK_STREAM;
205205
let mut res = ptr::null_mut();

0 commit comments

Comments
 (0)