Skip to content

Commit

Permalink
Add split-debuginfo profile option
Browse files Browse the repository at this point in the history
This commit adds a new `split-debuginfo` option to Cargo compilation
profiles which gets forwarded to the `-Csplit-debuginfo` codegen option
in rustc. This commit also sets the default, only on macOS, to be
`-Csplit-debuginfo=unpacked`. The purpose of this change is to leverage
rust-lang/rust#79570 to avoid running `dsymutil` on incremental builds
while also preserving a pleasant debugging experience by default. This
should lead to much faster incremental build times on macOS since
`dsymutil` isn't exactly the speediest tool in the world.

This is technically a breaking change in Cargo because we're no longer
by-default producing the `*.dSYM` folders on macOS. If those are still
desired, however, authors can always run `dsymutil` themselves or
otherwise configure `split-debuginfo = 'packed'` in their
manifest/profile configuration.
  • Loading branch information
alexcrichton committed Feb 1, 2021
1 parent 8a3361c commit ed4568e
Show file tree
Hide file tree
Showing 15 changed files with 212 additions and 119 deletions.
10 changes: 10 additions & 0 deletions crates/cargo-test-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -722,6 +722,16 @@ impl Execs {
self
}

pub fn enable_mac_dsym(&mut self) -> &mut Self {
if cfg!(target_os = "macos") {
self.env("CARGO_PROFILE_DEV_SPLIT_DEBUGINFO", "packed")
.env("CARGO_PROFILE_TEST_SPLIT_DEBUGINFO", "packed")
.env("CARGO_PROFILE_RELEASE_SPLIT_DEBUGINFO", "packed")
.env("CARGO_PROFILE_BENCH_SPLIT_DEBUGINFO", "packed");
}
self
}

pub fn run(&mut self) {
self.ran = true;
let p = (&self.process_builder).clone().unwrap();
Expand Down
6 changes: 6 additions & 0 deletions src/cargo/core/compiler/build_context/target_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ pub struct TargetInfo {
pub rustflags: Vec<String>,
/// Extra flags to pass to `rustdoc`, see `env_args`.
pub rustdocflags: Vec<String>,
/// Whether or not rustc supports the `-Csplit-debuginfo` flag.
pub supports_split_debuginfo: bool,
}

/// Kind of each file generated by a Unit, part of `FileType`.
Expand Down Expand Up @@ -157,6 +159,9 @@ impl TargetInfo {
for crate_type in KNOWN_CRATE_TYPES.iter() {
process.arg("--crate-type").arg(crate_type.as_str());
}
let supports_split_debuginfo = rustc
.cached_output(process.clone().arg("-Csplit-debuginfo=packed"))
.is_ok();

process.arg("--print=sysroot");
process.arg("--print=cfg");
Expand Down Expand Up @@ -231,6 +236,7 @@ impl TargetInfo {
"RUSTDOCFLAGS",
)?,
cfg,
supports_split_debuginfo,
})
}

Expand Down
10 changes: 10 additions & 0 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -773,6 +773,7 @@ fn build_base_args(
codegen_units,
debuginfo,
debug_assertions,
split_debuginfo,
overflow_checks,
rpath,
ref panic,
Expand Down Expand Up @@ -825,6 +826,15 @@ fn build_base_args(

cmd.args(&lto_args(cx, unit));

// This is generally just an optimization on build time so if we don't pass
// it then it's ok. As of the time of this writing it's a very new flag, so
// we need to dynamically check if it's available.
if cx.bcx.target_data.info(unit.kind).supports_split_debuginfo {
if let Some(split) = split_debuginfo {
cmd.arg("-C").arg(format!("split-debuginfo={}", split));
}
}

if let Some(n) = codegen_units {
cmd.arg("-C").arg(&format!("codegen-units={}", n));
}
Expand Down
15 changes: 8 additions & 7 deletions src/cargo/core/compiler/standard_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,16 +162,17 @@ pub fn generate_std_roots(
// in time is minimal, and the difference in caching is
// significant.
let mode = CompileMode::Build;
let profile = profiles.get_profile(
pkg.package_id(),
/*is_member*/ false,
/*is_local*/ false,
unit_for,
mode,
);
let features = std_features.activated_features(pkg.package_id(), FeaturesFor::NormalOrDev);

for kind in kinds {
let profile = profiles.get_profile(
pkg.package_id(),
/*is_member*/ false,
/*is_local*/ false,
unit_for,
mode,
*kind,
);
let list = ret.entry(*kind).or_insert_with(Vec::new);
list.push(interner.intern(
pkg,
Expand Down
1 change: 1 addition & 0 deletions src/cargo/core/compiler/unit_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,7 @@ fn new_unit_dep(
is_local,
unit_for,
mode,
kind,
);
new_unit_dep_with_profile(state, parent, pkg, target, unit_for, kind, mode, profile)
}
Expand Down
86 changes: 48 additions & 38 deletions src/cargo/core/profiles.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
use crate::core::compiler::{CompileMode, Unit};
use crate::core::compiler::{CompileKind, CompileMode, Unit};
use crate::core::resolver::features::FeaturesFor;
use crate::core::{Feature, Features, PackageId, PackageIdSpec, Resolve, Shell, Target};
use crate::core::{Feature, PackageId, PackageIdSpec, Resolve, Shell, Target, Workspace};
use crate::util::errors::CargoResultExt;
use crate::util::interning::InternedString;
use crate::util::toml::{ProfilePackageSpec, StringOrBool, TomlProfile, TomlProfiles, U32OrBool};
use crate::util::{closest_msg, config, CargoResult, Config};
use anyhow::bail;
use std::collections::{BTreeMap, HashMap, HashSet};
use std::hash::Hash;
use std::{cmp, env, fmt, hash};

/// Collection of all profiles.
Expand All @@ -24,28 +25,28 @@ pub struct Profiles {
named_profiles_enabled: bool,
/// The profile the user requested to use.
requested_profile: InternedString,
/// The host target for rustc being used by this `Profiles`.
rustc_host: InternedString,
}

impl Profiles {
pub fn new(
profiles: Option<&TomlProfiles>,
config: &Config,
requested_profile: InternedString,
features: &Features,
) -> CargoResult<Profiles> {
pub fn new(ws: &Workspace<'_>, requested_profile: InternedString) -> CargoResult<Profiles> {
let config = ws.config();
let incremental = match env::var_os("CARGO_INCREMENTAL") {
Some(v) => Some(v == "1"),
None => config.build_config()?.incremental,
};
let mut profiles = merge_config_profiles(profiles, config, requested_profile, features)?;
let mut profiles = merge_config_profiles(ws, requested_profile)?;
let rustc_host = ws.config().load_global_rustc(Some(ws))?.host;

if !features.is_enabled(Feature::named_profiles()) {
if !ws.unstable_features().is_enabled(Feature::named_profiles()) {
let mut profile_makers = Profiles {
incremental,
named_profiles_enabled: false,
dir_names: Self::predefined_dir_names(),
by_name: HashMap::new(),
requested_profile,
rustc_host,
};

profile_makers.by_name.insert(
Expand Down Expand Up @@ -98,6 +99,7 @@ impl Profiles {
dir_names: Self::predefined_dir_names(),
by_name: HashMap::new(),
requested_profile,
rustc_host,
};

Self::add_root_profiles(&mut profile_makers, &profiles);
Expand Down Expand Up @@ -289,6 +291,7 @@ impl Profiles {
is_local: bool,
unit_for: UnitFor,
mode: CompileMode,
kind: CompileKind,
) -> Profile {
let (profile_name, inherits) = if !self.named_profiles_enabled {
// With the feature disabled, we degrade `--profile` back to the
Expand Down Expand Up @@ -344,10 +347,28 @@ impl Profiles {
}
}

// Default macOS debug information to being stored in the "packed"
// split-debuginfo format. At the time of this writing that's the only
// platform which has a stable `-Csplit-debuginfo` option for rustc,
// and it's typically much faster than running `dsymutil` on all builds
// in incremental cases.
if let Some(debug) = profile.debuginfo {
if profile.split_debuginfo.is_none() && debug > 0 {
let target = match &kind {
CompileKind::Host => self.rustc_host.as_str(),
CompileKind::Target(target) => target.short_name(),
};
if target.contains("-apple-") {
profile.split_debuginfo = Some(InternedString::new("unpacked"));
}
}
}

// Incremental can be globally overridden.
if let Some(v) = self.incremental {
profile.incremental = v;
}

// Only enable incremental compilation for sources the user can
// modify (aka path sources). For things that change infrequently,
// non-incremental builds yield better performance in the compiler
Expand Down Expand Up @@ -567,6 +588,9 @@ fn merge_profile(profile: &mut Profile, toml: &TomlProfile) {
if let Some(debug_assertions) = toml.debug_assertions {
profile.debug_assertions = debug_assertions;
}
if let Some(split_debuginfo) = &toml.split_debuginfo {
profile.split_debuginfo = Some(InternedString::new(split_debuginfo));
}
if let Some(rpath) = toml.rpath {
profile.rpath = rpath;
}
Expand Down Expand Up @@ -612,6 +636,7 @@ pub struct Profile {
// `None` means use rustc default.
pub codegen_units: Option<u32>,
pub debuginfo: Option<u32>,
pub split_debuginfo: Option<InternedString>,
pub debug_assertions: bool,
pub overflow_checks: bool,
pub rpath: bool,
Expand All @@ -630,6 +655,7 @@ impl Default for Profile {
codegen_units: None,
debuginfo: None,
debug_assertions: false,
split_debuginfo: None,
overflow_checks: false,
rpath: false,
incremental: false,
Expand All @@ -654,6 +680,7 @@ compact_debug! {
root
codegen_units
debuginfo
split_debuginfo
debug_assertions
overflow_checks
rpath
Expand Down Expand Up @@ -734,25 +761,13 @@ impl Profile {
/// Compares all fields except `name`, which doesn't affect compilation.
/// This is necessary for `Unit` deduplication for things like "test" and
/// "dev" which are essentially the same.
fn comparable(
&self,
) -> (
InternedString,
Lto,
Option<u32>,
Option<u32>,
bool,
bool,
bool,
bool,
PanicStrategy,
Strip,
) {
fn comparable(&self) -> impl Hash + Eq {
(
self.opt_level,
self.lto,
self.codegen_units,
self.debuginfo,
self.split_debuginfo,
self.debug_assertions,
self.overflow_checks,
self.rpath,
Expand Down Expand Up @@ -1073,12 +1088,10 @@ impl UnitFor {
///
/// Returns a new copy of the profile map with all the mergers complete.
fn merge_config_profiles(
profiles: Option<&TomlProfiles>,
config: &Config,
ws: &Workspace<'_>,
requested_profile: InternedString,
features: &Features,
) -> CargoResult<BTreeMap<InternedString, TomlProfile>> {
let mut profiles = match profiles {
let mut profiles = match ws.profiles() {
Some(profiles) => profiles.get_all().clone(),
None => BTreeMap::new(),
};
Expand All @@ -1087,7 +1100,7 @@ fn merge_config_profiles(
check_to_add.insert(requested_profile);
// Merge config onto manifest profiles.
for (name, profile) in &mut profiles {
if let Some(config_profile) = get_config_profile(name, config, features)? {
if let Some(config_profile) = get_config_profile(ws, name)? {
profile.merge(&config_profile);
}
if let Some(inherits) = &profile.inherits {
Expand All @@ -1106,7 +1119,7 @@ fn merge_config_profiles(
std::mem::swap(&mut current, &mut check_to_add);
for name in current.drain() {
if !profiles.contains_key(&name) {
if let Some(config_profile) = get_config_profile(&name, config, features)? {
if let Some(config_profile) = get_config_profile(ws, &name)? {
if let Some(inherits) = &config_profile.inherits {
check_to_add.insert(*inherits);
}
Expand All @@ -1119,20 +1132,17 @@ fn merge_config_profiles(
}

/// Helper for fetching a profile from config.
fn get_config_profile(
name: &str,
config: &Config,
features: &Features,
) -> CargoResult<Option<TomlProfile>> {
let profile: Option<config::Value<TomlProfile>> = config.get(&format!("profile.{}", name))?;
fn get_config_profile(ws: &Workspace<'_>, name: &str) -> CargoResult<Option<TomlProfile>> {
let profile: Option<config::Value<TomlProfile>> =
ws.config().get(&format!("profile.{}", name))?;
let profile = match profile {
Some(profile) => profile,
None => return Ok(None),
};
let mut warnings = Vec::new();
profile
.val
.validate(name, features, &mut warnings)
.validate(name, ws.unstable_features(), &mut warnings)
.chain_err(|| {
anyhow::format_err!(
"config profile `{}` is not valid (defined in `{}`)",
Expand All @@ -1141,7 +1151,7 @@ fn get_config_profile(
)
})?;
for warning in warnings {
config.shell().warn(warning)?;
ws.config().shell().warn(warning)?;
}
Ok(Some(profile.val))
}
Expand Down
14 changes: 7 additions & 7 deletions src/cargo/ops/cargo_clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,7 @@ pub fn clean(ws: &Workspace<'_>, opts: &CleanOptions<'_>) -> CargoResult<()> {
return rm_rf(&target_dir.into_path_unlocked(), config);
}

let profiles = Profiles::new(
ws.profiles(),
config,
opts.requested_profile,
ws.unstable_features(),
)?;
let profiles = Profiles::new(ws, opts.requested_profile)?;

if opts.profile_specified {
// After parsing profiles we know the dir-name of the profile, if a profile
Expand Down Expand Up @@ -181,9 +176,14 @@ pub fn clean(ws: &Workspace<'_>, opts: &CleanOptions<'_>) -> CargoResult<()> {
// Remove dep-info file generated by rustc. It is not tracked in
// file_types. It does not have a prefix.
let hashed_dep_info = dir.join(format!("{}-*.d", crate_name));
let unhashed_dep_info = dir.join(format!("{}.d", crate_name));
rm_rf_glob(&hashed_dep_info, config)?;
let unhashed_dep_info = dir.join(format!("{}.d", crate_name));
rm_rf(&unhashed_dep_info, config)?;
// Remove split-debuginfo files generated by rustc.
let split_debuginfo_obj = dir.join(format!("{}.*.o", crate_name));
rm_rf_glob(&split_debuginfo_obj, config)?;
let split_debuginfo_dwo = dir.join(format!("{}.*.dwo", crate_name));
rm_rf_glob(&split_debuginfo_dwo, config)?;

// Remove the uplifted copy.
if let Some(uplift_dir) = uplift_dir {
Expand Down
22 changes: 9 additions & 13 deletions src/cargo/ops/cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -429,12 +429,7 @@ pub fn create_bcx<'a, 'cfg>(
);
}

let profiles = Profiles::new(
ws.profiles(),
config,
build_config.requested_profile,
ws.unstable_features(),
)?;
let profiles = Profiles::new(ws, build_config.requested_profile)?;
profiles.validate_packages(
ws.profiles(),
&mut config.shell(),
Expand Down Expand Up @@ -887,19 +882,20 @@ fn generate_targets(
};

let is_local = pkg.package_id().source_id().is_path();
let profile = profiles.get_profile(
pkg.package_id(),
ws.is_member(pkg),
is_local,
unit_for,
target_mode,
);

// No need to worry about build-dependencies, roots are never build dependencies.
let features_for = FeaturesFor::from_for_host(target.proc_macro());
let features = resolved_features.activated_features(pkg.package_id(), features_for);

for kind in requested_kinds {
let profile = profiles.get_profile(
pkg.package_id(),
ws.is_member(pkg),
is_local,
unit_for,
target_mode,
kind.for_target(target),
);
let unit = interner.intern(
pkg,
target,
Expand Down
Loading

0 comments on commit ed4568e

Please sign in to comment.