Skip to content

Commit

Permalink
Use posix_spawn for absolute paths on macOS
Browse files Browse the repository at this point in the history
Currently, on macOS, Rust never uses the fast posix_spawn path if a
directory change is requested due to a bug in Apple's libc. However, the
bug is only triggered if the program is a relative path.

This PR makes it so that the fast path continues to work if the program
is an absolute path or a lone filename.

This was an alternative proposed in
#80537 (comment), and it
makes a measurable performance difference in some of my code that spawns
thousands of processes.
  • Loading branch information
sunshowers committed Aug 29, 2022
1 parent 1ea4efd commit bd8b4b9
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 1 deletion.
33 changes: 33 additions & 0 deletions library/std/src/sys/unix/process/process_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ pub struct Command {
argv: Argv,
env: CommandEnv,

program_kind: ProgramKind,
cwd: Option<CString>,
uid: Option<uid_t>,
gid: Option<gid_t>,
Expand Down Expand Up @@ -148,15 +149,40 @@ pub enum Stdio {
Fd(FileDesc),
}

#[derive(Copy, Clone, Debug, Eq, PartialEq)]
pub enum ProgramKind {
/// A program that would be looked up on the PATH (e.g. `ls`)
PathLookup,
/// A relative path (e.g. `my-dir/foo`, `../foo`, `./foo`)
Relative,
/// An absolute path.
Absolute,
}

impl ProgramKind {
fn new(program: &OsStr) -> Self {
if program.bytes().starts_with(b"/") {
Self::Absolute
} else if program.bytes().contains(&b'/') {
// If the program has more than one component in it, it is a relative path.
Self::Relative
} else {
Self::PathLookup
}
}
}

impl Command {
#[cfg(not(target_os = "linux"))]
pub fn new(program: &OsStr) -> Command {
let mut saw_nul = false;
let program_kind = ProgramKind::new(program.as_ref());
let program = os2c(program, &mut saw_nul);
Command {
argv: Argv(vec![program.as_ptr(), ptr::null()]),
args: vec![program.clone()],
program,
program_kind,
env: Default::default(),
cwd: None,
uid: None,
Expand All @@ -174,11 +200,13 @@ impl Command {
#[cfg(target_os = "linux")]
pub fn new(program: &OsStr) -> Command {
let mut saw_nul = false;
let program_kind = ProgramKind::new(program.as_ref());
let program = os2c(program, &mut saw_nul);
Command {
argv: Argv(vec![program.as_ptr(), ptr::null()]),
args: vec![program.clone()],
program,
program_kind,
env: Default::default(),
cwd: None,
uid: None,
Expand Down Expand Up @@ -254,6 +282,11 @@ impl Command {
OsStr::from_bytes(self.program.as_bytes())
}

#[allow(dead_code)]
pub fn get_program_kind(&self) -> ProgramKind {
self.program_kind
}

pub fn get_args(&self) -> CommandArgs<'_> {
let mut iter = self.args.iter();
iter.next();
Expand Down
24 changes: 24 additions & 0 deletions library/std/src/sys/unix/process/process_common/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,3 +122,27 @@ fn test_process_group_no_posix_spawn() {
t!(cat.wait());
}
}

#[test]
fn test_program_kind() {
let vectors = &[
("foo", ProgramKind::PathLookup),
("foo.out", ProgramKind::PathLookup),
("./foo", ProgramKind::Relative),
("../foo", ProgramKind::Relative),
("dir/foo", ProgramKind::Relative),
// Note that paths on Unix can't contain / in them, so this is actually the directory "fo\\"
// followed by the file "o".
("fo\\/o", ProgramKind::Relative),
("/foo", ProgramKind::Absolute),
("/dir/../foo", ProgramKind::Absolute),
];

for (program, expected_kind) in vectors {
assert_eq!(
ProgramKind::new(program.as_ref()),
*expected_kind,
"actual != expected program kind for input {program}",
);
}
}
4 changes: 3 additions & 1 deletion library/std/src/sys/unix/process/process_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,9 @@ impl Command {
// successfully launch the program, but erroneously return
// ENOENT when used with posix_spawn_file_actions_addchdir_np
// which was introduced in macOS 10.15.
return Ok(None);
if self.get_program_kind() == ProgramKind::Relative {
return Ok(None);
}
}
match posix_spawn_file_actions_addchdir_np.get() {
Some(f) => Some((f, cwd)),
Expand Down

0 comments on commit bd8b4b9

Please sign in to comment.