Skip to content

Commit

Permalink
Auto merge of #6867 - alexcrichton:improvements, r=ehuss
Browse files Browse the repository at this point in the history 
Backend refactorings and perf improvements

This PR is extracted from #6864 and then also additionally adds some commits related to performance optimizations that I noticed while profiling #6864. Each commit is in theory standalone and should pass all the tests, as well as being descriptive about what it's doing.
  • Loading branch information
@bors
bors committed Apr 25, 2019
2 parents a507ae4 + 32269f4 commit 6562942
Show file tree
Hide file tree
Showing 15 changed files with 548 additions and 482 deletions.
153 changes: 10 additions & 143 deletions src/cargo/core/compiler/build_context/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use std::collections::HashMap;
use std::env;
use std::path::{Path, PathBuf};
use std::str;

Expand All @@ -9,9 +8,9 @@ use crate::core::profiles::Profiles;
use crate::core::{Dependency, Workspace};
use crate::core::{PackageId, PackageSet, Resolve};
use crate::util::errors::CargoResult;
use crate::util::{profile, Cfg, CfgExpr, Config, Rustc};

use super::{BuildConfig, BuildOutput, Kind, Unit};
use crate::util::{profile, Cfg, Config, Rustc};
use crate::core::compiler::{Unit, Kind, BuildConfig, BuildOutput};
use crate::core::compiler::unit::UnitInterner;

mod target_info;
pub use self::target_info::{FileFlavor, TargetInfo};
Expand All @@ -38,6 +37,7 @@ pub struct BuildContext<'a, 'cfg: 'a> {
pub target_config: TargetConfig,
pub target_info: TargetInfo,
pub host_info: TargetInfo,
pub units: &'a UnitInterner<'a>,
}

impl<'a, 'cfg> BuildContext<'a, 'cfg> {
Expand All @@ -48,6 +48,7 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
config: &'cfg Config,
build_config: &'a BuildConfig,
profiles: &'a Profiles,
units: &'a UnitInterner<'a>,
extra_compiler_args: HashMap<Unit<'a>, Vec<String>>,
) -> CargoResult<BuildContext<'a, 'cfg>> {
let mut rustc = config.load_global_rustc(Some(ws))?;
Expand Down Expand Up @@ -83,6 +84,7 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
build_config,
profiles,
extra_compiler_args,
units,
})
}

Expand Down Expand Up @@ -157,26 +159,12 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
self.build_config.jobs
}

pub fn rustflags_args(&self, unit: &Unit<'_>) -> CargoResult<Vec<String>> {
env_args(
self.config,
&self.build_config.requested_target,
self.host_triple(),
self.info(unit.kind).cfg(),
unit.kind,
"RUSTFLAGS",
)
pub fn rustflags_args(&self, unit: &Unit<'_>) -> &[String] {
&self.info(unit.kind).rustflags
}

pub fn rustdocflags_args(&self, unit: &Unit<'_>) -> CargoResult<Vec<String>> {
env_args(
self.config,
&self.build_config.requested_target,
self.host_triple(),
self.info(unit.kind).cfg(),
unit.kind,
"RUSTDOCFLAGS",
)
pub fn rustdocflags_args(&self, unit: &Unit<'_>) -> &[String] {
&self.info(unit.kind).rustdocflags
}

pub fn show_warnings(&self, pkg: PackageId) -> bool {
Expand Down Expand Up @@ -292,124 +280,3 @@ impl TargetConfig {
Ok(ret)
}
}

/// Acquire extra flags to pass to the compiler from various locations.
///
/// The locations are:
///
/// - the `RUSTFLAGS` environment variable
///
/// then if this was not found
///
/// - `target.*.rustflags` from the manifest (Cargo.toml)
/// - `target.cfg(..).rustflags` from the manifest
///
/// then if neither of these were found
///
/// - `build.rustflags` from the manifest
///
/// Note that if a `target` is specified, no args will be passed to host code (plugins, build
/// scripts, ...), even if it is the same as the target.
fn env_args(
config: &Config,
requested_target: &Option<String>,
host_triple: &str,
target_cfg: Option<&[Cfg]>,
kind: Kind,
name: &str,
) -> CargoResult<Vec<String>> {
// We *want* to apply RUSTFLAGS only to builds for the
// requested target architecture, and not to things like build
// scripts and plugins, which may be for an entirely different
// architecture. Cargo's present architecture makes it quite
// hard to only apply flags to things that are not build
// scripts and plugins though, so we do something more hacky
// instead to avoid applying the same RUSTFLAGS to multiple targets
// arches:
//
// 1) If --target is not specified we just apply RUSTFLAGS to
// all builds; they are all going to have the same target.
//
// 2) If --target *is* specified then we only apply RUSTFLAGS
// to compilation units with the Target kind, which indicates
// it was chosen by the --target flag.
//
// This means that, e.g., even if the specified --target is the
// same as the host, build scripts in plugins won't get
// RUSTFLAGS.
let compiling_with_target = requested_target.is_some();
let is_target_kind = kind == Kind::Target;

if compiling_with_target && !is_target_kind {
// This is probably a build script or plugin and we're
// compiling with --target. In this scenario there are
// no rustflags we can apply.
return Ok(Vec::new());
}

// First try RUSTFLAGS from the environment
if let Ok(a) = env::var(name) {
let args = a
.split(' ')
.map(str::trim)
.filter(|s| !s.is_empty())
.map(str::to_string);
return Ok(args.collect());
}

let mut rustflags = Vec::new();

let name = name
.chars()
.flat_map(|c| c.to_lowercase())
.collect::<String>();
// Then the target.*.rustflags value...
let target = requested_target
.as_ref()
.map(|s| s.as_str())
.unwrap_or(host_triple);
let key = format!("target.{}.{}", target, name);
if let Some(args) = config.get_list_or_split_string(&key)? {
let args = args.val.into_iter();
rustflags.extend(args);
}
// ...including target.'cfg(...)'.rustflags
if let Some(target_cfg) = target_cfg {
if let Some(table) = config.get_table("target")? {
let cfgs = table
.val
.keys()
.filter(|key| CfgExpr::matches_key(key, target_cfg));

// Note that we may have multiple matching `[target]` sections and
// because we're passing flags to the compiler this can affect
// cargo's caching and whether it rebuilds. Ensure a deterministic
// ordering through sorting for now. We may perhaps one day wish to
// ensure a deterministic ordering via the order keys were defined
// in files perhaps.
let mut cfgs = cfgs.collect::<Vec<_>>();
cfgs.sort();

for n in cfgs {
let key = format!("target.{}.{}", n, name);
if let Some(args) = config.get_list_or_split_string(&key)? {
let args = args.val.into_iter();
rustflags.extend(args);
}
}
}
}

if !rustflags.is_empty() {
return Ok(rustflags);
}

// Then the `build.rustflags` value.
let key = format!("build.{}", name);
if let Some(args) = config.get_list_or_split_string(&key)? {
let args = args.val.into_iter();
return Ok(args.collect());
}

Ok(Vec::new())
}
150 changes: 146 additions & 4 deletions src/cargo/core/compiler/build_context/target_info.rs
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use std::cell::RefCell;
use std::collections::hash_map::{Entry, HashMap};
use std::env;
use std::path::PathBuf;
use std::str::{self, FromStr};

use super::env_args;
use super::Kind;
use crate::core::compiler::Kind;
use crate::core::TargetKind;
use crate::util::CfgExpr;
use crate::util::{CargoResult, CargoResultExt, Cfg, Config, ProcessBuilder, Rustc};

#[derive(Clone)]
Expand All @@ -14,6 +15,8 @@ pub struct TargetInfo {
crate_types: RefCell<HashMap<String, Option<(String, String)>>>,
cfg: Option<Vec<Cfg>>,
pub sysroot_libdir: Option<PathBuf>,
pub rustflags: Vec<String>,
pub rustdocflags: Vec<String>,
}

/// Type of each file generated by a Unit.
Expand Down Expand Up @@ -136,16 +139,34 @@ impl TargetInfo {
}

let cfg = if has_cfg_and_sysroot {
Some(lines.map(Cfg::from_str).collect::<CargoResult<_>>()?)
Some(lines.map(Cfg::from_str).collect::<CargoResult<Vec<_>>>()?)
} else {
None
};

Ok(TargetInfo {
crate_type_process: Some(crate_type_process),
crate_types: RefCell::new(map),
cfg,
sysroot_libdir,
// recalculate `rustflags` from above now that we have `cfg`
// information
rustflags: env_args(
config,
requested_target,
&rustc.host,
cfg.as_ref().map(|v| v.as_ref()),
kind,
"RUSTFLAGS",
)?,
rustdocflags: env_args(
config,
requested_target,
&rustc.host,
cfg.as_ref().map(|v| v.as_ref()),
kind,
"RUSTDOCFLAGS",
)?,
cfg,
})
}

Expand Down Expand Up @@ -289,3 +310,124 @@ fn parse_crate_type(

Ok(Some((prefix.to_string(), suffix.to_string())))
}

/// Acquire extra flags to pass to the compiler from various locations.
///
/// The locations are:
///
/// - the `RUSTFLAGS` environment variable
///
/// then if this was not found
///
/// - `target.*.rustflags` from the manifest (Cargo.toml)
/// - `target.cfg(..).rustflags` from the manifest
///
/// then if neither of these were found
///
/// - `build.rustflags` from the manifest
///
/// Note that if a `target` is specified, no args will be passed to host code (plugins, build
/// scripts, ...), even if it is the same as the target.
fn env_args(
config: &Config,
requested_target: &Option<String>,
host_triple: &str,
target_cfg: Option<&[Cfg]>,
kind: Kind,
name: &str,
) -> CargoResult<Vec<String>> {
// We *want* to apply RUSTFLAGS only to builds for the
// requested target architecture, and not to things like build
// scripts and plugins, which may be for an entirely different
// architecture. Cargo's present architecture makes it quite
// hard to only apply flags to things that are not build
// scripts and plugins though, so we do something more hacky
// instead to avoid applying the same RUSTFLAGS to multiple targets
// arches:
//
// 1) If --target is not specified we just apply RUSTFLAGS to
// all builds; they are all going to have the same target.
//
// 2) If --target *is* specified then we only apply RUSTFLAGS
// to compilation units with the Target kind, which indicates
// it was chosen by the --target flag.
//
// This means that, e.g., even if the specified --target is the
// same as the host, build scripts in plugins won't get
// RUSTFLAGS.
let compiling_with_target = requested_target.is_some();
let is_target_kind = kind == Kind::Target;

if compiling_with_target && !is_target_kind {
// This is probably a build script or plugin and we're
// compiling with --target. In this scenario there are
// no rustflags we can apply.
return Ok(Vec::new());
}

// First try RUSTFLAGS from the environment
if let Ok(a) = env::var(name) {
let args = a
.split(' ')
.map(str::trim)
.filter(|s| !s.is_empty())
.map(str::to_string);
return Ok(args.collect());
}

let mut rustflags = Vec::new();

let name = name
.chars()
.flat_map(|c| c.to_lowercase())
.collect::<String>();
// Then the target.*.rustflags value...
let target = requested_target
.as_ref()
.map(|s| s.as_str())
.unwrap_or(host_triple);
let key = format!("target.{}.{}", target, name);
if let Some(args) = config.get_list_or_split_string(&key)? {
let args = args.val.into_iter();
rustflags.extend(args);
}
// ...including target.'cfg(...)'.rustflags
if let Some(target_cfg) = target_cfg {
if let Some(table) = config.get_table("target")? {
let cfgs = table
.val
.keys()
.filter(|key| CfgExpr::matches_key(key, target_cfg));

// Note that we may have multiple matching `[target]` sections and
// because we're passing flags to the compiler this can affect
// cargo's caching and whether it rebuilds. Ensure a deterministic
// ordering through sorting for now. We may perhaps one day wish to
// ensure a deterministic ordering via the order keys were defined
// in files perhaps.
let mut cfgs = cfgs.collect::<Vec<_>>();
cfgs.sort();

for n in cfgs {
let key = format!("target.{}.{}", n, name);
if let Some(args) = config.get_list_or_split_string(&key)? {
let args = args.val.into_iter();
rustflags.extend(args);
}
}
}
}

if !rustflags.is_empty() {
return Ok(rustflags);
}

// Then the `build.rustflags` value.
let key = format!("build.{}", name);
if let Some(args) = config.get_list_or_split_string(&key)? {
let args = args.val.into_iter();
return Ok(args.collect());
}

Ok(Vec::new())
}
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/build_plan.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ impl BuildPlan {
}
}

pub fn add(&mut self, cx: &Context<'_, '_>, unit: &Unit<'_>) -> CargoResult<()> {
pub fn add<'a>(&mut self, cx: &Context<'a, '_>, unit: &Unit<'a>) -> CargoResult<()> {
let id = self.plan.invocations.len();
self.invocation_map.insert(unit.buildkey(), id);
let deps = cx
Expand Down
Loading

0 comments on commit 6562942

Please sign in to comment.