std: Improve downstream codegen in Command::env

This commit rejiggers the generics used in the implementation of `Command::env` with the purpose of reducing the amount of codegen that needs to happen in consumer crates, instead preferring to generate code into libstd. This was found when profiling the compile times of the `cc` crate where the binary rlib produced had a lot of `BTreeMap` code compiled into it but the crate doesn't actually use `BTreeMap`. It turns out that `Command::env` is generic enough to codegen the entire implementation in calling crates, but in this case there's no performance concern so it's fine to compile the code into the standard library. This change is done by removing the generic on the `CommandEnv` map which is intended to handle case-insensitive variables on Windows. Instead now a generic isn't used but rather a `use` statement defined per-platform is used. With this commit a debug build of `Command::new("foo").env("a", "b")` drops from 21k lines of LLVM IR to 10k.
rust-lang · Sep 5, 2019 · 0b7ba6e · 0b7ba6e
1 parent 6187684
commit 0b7ba6e
Show file tree

Hide file tree

Showing 9 changed files with 56 additions and 70 deletions.
diff --git a/src/libstd/sys/cloudabi/shims/process.rs b/src/libstd/sys/cloudabi/shims/process.rs
@@ -4,14 +4,16 @@ use crate::io;
 use crate::sys::fs::File;
 use crate::sys::pipe::AnonPipe;
 use crate::sys::{unsupported, Void};
-use crate::sys_common::process::{CommandEnv, DefaultEnvKey};
+use crate::sys_common::process::CommandEnv;
+
+pub use crate::ffi::OsString as EnvKey;
 
 ////////////////////////////////////////////////////////////////////////////////
 // Command
 ////////////////////////////////////////////////////////////////////////////////
 
 pub struct Command {
-    env: CommandEnv<DefaultEnvKey>,
+    env: CommandEnv,
 }
 
 // passed back to std::process with the pipes connected to the child, if any
@@ -37,7 +39,7 @@ impl Command {
 
     pub fn arg(&mut self, _arg: &OsStr) {}
 
-    pub fn env_mut(&mut self) -> &mut CommandEnv<DefaultEnvKey> {
+    pub fn env_mut(&mut self) -> &mut CommandEnv {
         &mut self.env
     }
 

diff --git a/src/libstd/sys/sgx/process.rs b/src/libstd/sys/sgx/process.rs
@@ -4,14 +4,16 @@ use crate::io;
 use crate::sys::fs::File;
 use crate::sys::pipe::AnonPipe;
 use crate::sys::{unsupported, Void};
-use crate::sys_common::process::{CommandEnv, DefaultEnvKey};
+use crate::sys_common::process::CommandEnv;
+
+pub use crate::ffi::OsString as EnvKey;
 
 ////////////////////////////////////////////////////////////////////////////////
 // Command
 ////////////////////////////////////////////////////////////////////////////////
 
 pub struct Command {
-    env: CommandEnv<DefaultEnvKey>
+    env: CommandEnv,
 }
 
 // passed back to std::process with the pipes connected to the child, if any
@@ -38,7 +40,7 @@ impl Command {
     pub fn arg(&mut self, _arg: &OsStr) {
     }
 
-    pub fn env_mut(&mut self) -> &mut CommandEnv<DefaultEnvKey> {
+    pub fn env_mut(&mut self) -> &mut CommandEnv {
         &mut self.env
     }
 

diff --git a/src/libstd/sys/unix/process/mod.rs b/src/libstd/sys/unix/process/mod.rs
@@ -1,5 +1,6 @@
 pub use self::process_common::{Command, ExitStatus, ExitCode, Stdio, StdioPipes};
 pub use self::process_inner::Process;
+pub use crate::ffi::OsString as EnvKey;
 
 mod process_common;
 #[cfg(not(target_os = "fuchsia"))]

diff --git a/src/libstd/sys/unix/process/process_common.rs b/src/libstd/sys/unix/process/process_common.rs
@@ -7,7 +7,7 @@ use crate::ptr;
 use crate::sys::fd::FileDesc;
 use crate::sys::fs::{File, OpenOptions};
 use crate::sys::pipe::{self, AnonPipe};
-use crate::sys_common::process::{CommandEnv, DefaultEnvKey};
+use crate::sys_common::process::CommandEnv;
 use crate::collections::BTreeMap;
 
 use libc::{c_int, gid_t, uid_t, c_char, EXIT_SUCCESS, EXIT_FAILURE};
@@ -69,7 +69,7 @@ pub struct Command {
     program: CString,
     args: Vec<CString>,
     argv: Argv,
-    env: CommandEnv<DefaultEnvKey>,
+    env: CommandEnv,
 
     cwd: Option<CString>,
     uid: Option<uid_t>,
@@ -201,7 +201,7 @@ impl Command {
         self.stderr = Some(stderr);
     }
 
-    pub fn env_mut(&mut self) -> &mut CommandEnv<DefaultEnvKey> {
+    pub fn env_mut(&mut self) -> &mut CommandEnv {
         &mut self.env
     }
 
@@ -271,7 +271,7 @@ impl CStringArray {
     }
 }
 
-fn construct_envp(env: BTreeMap<DefaultEnvKey, OsString>, saw_nul: &mut bool) -> CStringArray {
+fn construct_envp(env: BTreeMap<OsString, OsString>, saw_nul: &mut bool) -> CStringArray {
     let mut result = CStringArray::with_capacity(env.len());
     for (k, v) in env {
         let mut k: OsString = k.into();

diff --git a/src/libstd/sys/vxworks/process/process_common.rs b/src/libstd/sys/vxworks/process/process_common.rs
@@ -7,11 +7,13 @@ use crate::ptr;
 use crate::sys::fd::FileDesc;
 use crate::sys::fs::{File, OpenOptions};
 use crate::sys::pipe::{self, AnonPipe};
-use crate::sys_common::process::{CommandEnv, DefaultEnvKey};
+use crate::sys_common::process::CommandEnv;
 use crate::collections::BTreeMap;
 
 use libc::{c_int, gid_t, uid_t, c_char, EXIT_SUCCESS, EXIT_FAILURE};
 
+pub use crate::ffi::OsString as EnvKey;
+
 ////////////////////////////////////////////////////////////////////////////////
 // Command
 ////////////////////////////////////////////////////////////////////////////////
@@ -37,7 +39,7 @@ pub struct Command {
     program: CString,
     args: Vec<CString>,
     argv: Argv,
-    env: CommandEnv<DefaultEnvKey>,
+    env: CommandEnv,
 
     cwd: Option<CString>,
     uid: Option<uid_t>,
@@ -170,7 +172,7 @@ impl Command {
         self.stderr = Some(stderr);
     }
 
-    pub fn env_mut(&mut self) -> &mut CommandEnv<DefaultEnvKey> {
+    pub fn env_mut(&mut self) -> &mut CommandEnv {
         &mut self.env
     }
 
@@ -240,7 +242,7 @@ impl CStringArray {
     }
 }
 
-fn construct_envp(env: BTreeMap<DefaultEnvKey, OsString>, saw_nul: &mut bool) -> CStringArray {
+fn construct_envp(env: BTreeMap<OsString, OsString>, saw_nul: &mut bool) -> CStringArray {
     let mut result = CStringArray::with_capacity(env.len());
     for (k, v) in env {
         let mut k: OsString = k.into();

diff --git a/src/libstd/sys/wasi/process.rs b/src/libstd/sys/wasi/process.rs
@@ -4,14 +4,16 @@ use crate::io;
 use crate::sys::fs::File;
 use crate::sys::pipe::AnonPipe;
 use crate::sys::{unsupported, Void};
-use crate::sys_common::process::{CommandEnv, DefaultEnvKey};
+use crate::sys_common::process::CommandEnv;
+
+pub use crate::ffi::OsString as EnvKey;
 
 ////////////////////////////////////////////////////////////////////////////////
 // Command
 ////////////////////////////////////////////////////////////////////////////////
 
 pub struct Command {
-    env: CommandEnv<DefaultEnvKey>
+    env: CommandEnv
 }
 
 // passed back to std::process with the pipes connected to the child, if any
@@ -38,7 +40,7 @@ impl Command {
     pub fn arg(&mut self, _arg: &OsStr) {
     }
 
-    pub fn env_mut(&mut self) -> &mut CommandEnv<DefaultEnvKey> {
+    pub fn env_mut(&mut self) -> &mut CommandEnv {
         &mut self.env
     }
 

diff --git a/src/libstd/sys/wasm/process.rs b/src/libstd/sys/wasm/process.rs
@@ -4,14 +4,16 @@ use crate::io;
 use crate::sys::fs::File;
 use crate::sys::pipe::AnonPipe;
 use crate::sys::{unsupported, Void};
-use crate::sys_common::process::{CommandEnv, DefaultEnvKey};
+use crate::sys_common::process::CommandEnv;
+
+pub use crate::ffi::OsString as EnvKey;
 
 ////////////////////////////////////////////////////////////////////////////////
 // Command
 ////////////////////////////////////////////////////////////////////////////////
 
 pub struct Command {
-    env: CommandEnv<DefaultEnvKey>
+    env: CommandEnv,
 }
 
 // passed back to std::process with the pipes connected to the child, if any
@@ -38,7 +40,7 @@ impl Command {
     pub fn arg(&mut self, _arg: &OsStr) {
     }
 
-    pub fn env_mut(&mut self) -> &mut CommandEnv<DefaultEnvKey> {
+    pub fn env_mut(&mut self) -> &mut CommandEnv {
         &mut self.env
     }
 

diff --git a/src/libstd/sys/windows/process.rs b/src/libstd/sys/windows/process.rs
@@ -19,7 +19,7 @@ use crate::sys::pipe::{self, AnonPipe};
 use crate::sys::stdio;
 use crate::sys::cvt;
 use crate::sys_common::{AsInner, FromInner, IntoInner};
-use crate::sys_common::process::{CommandEnv, EnvKey};
+use crate::sys_common::process::CommandEnv;
 use crate::borrow::Borrow;
 
 use libc::{c_void, EXIT_SUCCESS, EXIT_FAILURE};
@@ -30,30 +30,28 @@ use libc::{c_void, EXIT_SUCCESS, EXIT_FAILURE};
 
 #[derive(Clone, Debug, Eq, PartialEq, Ord, PartialOrd)]
 #[doc(hidden)]
-pub struct WindowsEnvKey(OsString);
+pub struct EnvKey(OsString);
 
-impl From<OsString> for WindowsEnvKey {
+impl From<OsString> for EnvKey {
     fn from(k: OsString) -> Self {
         let mut buf = k.into_inner().into_inner();
         buf.make_ascii_uppercase();
-        WindowsEnvKey(FromInner::from_inner(FromInner::from_inner(buf)))
+        EnvKey(FromInner::from_inner(FromInner::from_inner(buf)))
     }
 }
 
-impl From<WindowsEnvKey> for OsString {
-    fn from(k: WindowsEnvKey) -> Self { k.0 }
+impl From<EnvKey> for OsString {
+    fn from(k: EnvKey) -> Self { k.0 }
 }
 
-impl Borrow<OsStr> for WindowsEnvKey {
+impl Borrow<OsStr> for EnvKey {
     fn borrow(&self) -> &OsStr { &self.0 }
 }
 
-impl AsRef<OsStr> for WindowsEnvKey {
+impl AsRef<OsStr> for EnvKey {
     fn as_ref(&self) -> &OsStr { &self.0 }
 }
 
-impl EnvKey for WindowsEnvKey {}
-
 
 fn ensure_no_nuls<T: AsRef<OsStr>>(str: T) -> io::Result<T> {
     if str.as_ref().encode_wide().any(|b| b == 0) {
@@ -66,7 +64,7 @@ fn ensure_no_nuls<T: AsRef<OsStr>>(str: T) -> io::Result<T> {
 pub struct Command {
     program: OsString,
     args: Vec<OsString>,
-    env: CommandEnv<WindowsEnvKey>,
+    env: CommandEnv,
     cwd: Option<OsString>,
     flags: u32,
     detach: bool, // not currently exposed in std::process
@@ -110,7 +108,7 @@ impl Command {
     pub fn arg(&mut self, arg: &OsStr) {
         self.args.push(arg.to_os_string())
     }
-    pub fn env_mut(&mut self) -> &mut CommandEnv<WindowsEnvKey> {
+    pub fn env_mut(&mut self) -> &mut CommandEnv {
         &mut self.env
     }
     pub fn cwd(&mut self, dir: &OsStr) {
@@ -498,7 +496,7 @@ fn make_command_line(prog: &OsStr, args: &[OsString]) -> io::Result<Vec<u16>> {
     }
 }
 
-fn make_envp(maybe_env: Option<BTreeMap<WindowsEnvKey, OsString>>)
+fn make_envp(maybe_env: Option<BTreeMap<EnvKey, OsString>>)
              -> io::Result<(*mut c_void, Vec<u16>)> {
     // On Windows we pass an "environment block" which is not a char**, but
     // rather a concatenation of null-terminated k=v\0 sequences, with a final

diff --git a/src/libstd/sys_common/process.rs b/src/libstd/sys_common/process.rs
@@ -1,47 +1,20 @@
 #![allow(dead_code)]
 #![unstable(feature = "process_internals", issue = "0")]
 
-use crate::ffi::{OsStr, OsString};
-use crate::env;
 use crate::collections::BTreeMap;
-use crate::borrow::Borrow;
-
-pub trait EnvKey:
-    From<OsString> + Into<OsString> +
-    Borrow<OsStr> + Borrow<Self> + AsRef<OsStr> +
-    Ord + Clone {}
-
-// Implement a case-sensitive environment variable key
-#[derive(Clone, Debug, Eq, PartialEq, Ord, PartialOrd)]
-pub struct DefaultEnvKey(OsString);
-
-impl From<OsString> for DefaultEnvKey {
-    fn from(k: OsString) -> Self { DefaultEnvKey(k) }
-}
-
-impl From<DefaultEnvKey> for OsString {
-    fn from(k: DefaultEnvKey) -> Self { k.0 }
-}
-
-impl Borrow<OsStr> for DefaultEnvKey {
-    fn borrow(&self) -> &OsStr { &self.0 }
-}
-
-impl AsRef<OsStr> for DefaultEnvKey {
-    fn as_ref(&self) -> &OsStr { &self.0 }
-}
-
-impl EnvKey for DefaultEnvKey {}
+use crate::env;
+use crate::ffi::{OsStr, OsString};
+use crate::sys::process::EnvKey;
 
 // Stores a set of changes to an environment
 #[derive(Clone, Debug)]
-pub struct CommandEnv<K> {
+pub struct CommandEnv {
     clear: bool,
     saw_path: bool,
-    vars: BTreeMap<K, Option<OsString>>
+    vars: BTreeMap<EnvKey, Option<OsString>>
 }
 
-impl<K: EnvKey> Default for CommandEnv<K> {
+impl Default for CommandEnv {
     fn default() -> Self {
         CommandEnv {
             clear: false,
@@ -51,10 +24,10 @@ impl<K: EnvKey> Default for CommandEnv<K> {
     }
 }
 
-impl<K: EnvKey> CommandEnv<K> {
+impl CommandEnv {
     // Capture the current environment with these changes applied
-    pub fn capture(&self) -> BTreeMap<K, OsString> {
-        let mut result = BTreeMap::<K, OsString>::new();
+    pub fn capture(&self) -> BTreeMap<EnvKey, OsString> {
+        let mut result = BTreeMap::<EnvKey, OsString>::new();
         if !self.clear {
             for (k, v) in env::vars_os() {
                 result.insert(k.into(), v);
@@ -90,7 +63,7 @@ impl<K: EnvKey> CommandEnv<K> {
         !self.clear && self.vars.is_empty()
     }
 
-    pub fn capture_if_changed(&self) -> Option<BTreeMap<K, OsString>> {
+    pub fn capture_if_changed(&self) -> Option<BTreeMap<EnvKey, OsString>> {
         if self.is_unchanged() {
             None
         } else {
@@ -103,6 +76,7 @@ impl<K: EnvKey> CommandEnv<K> {
         self.maybe_saw_path(&key);
         self.vars.insert(key.to_owned().into(), Some(value.to_owned()));
     }
+
     pub fn remove(&mut self, key: &OsStr) {
         self.maybe_saw_path(&key);
         if self.clear {
@@ -111,13 +85,16 @@ impl<K: EnvKey> CommandEnv<K> {
             self.vars.insert(key.to_owned().into(), None);
         }
     }
+
     pub fn clear(&mut self) {
         self.clear = true;
         self.vars.clear();
     }
+
     pub fn have_changed_path(&self) -> bool {
         self.saw_path || self.clear
     }
+
     fn maybe_saw_path(&mut self, key: &OsStr) {
         if !self.saw_path && key == "PATH" {
             self.saw_path = true;