Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Turn off debuginfo for build dependencies v2 #11252

Merged
merged 17 commits into from
Feb 2, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
introduce dedicated DebugInfo enum in Profiles
This enum will be used to model the current Option<u32> value in
profiles, as the explicitly set value, and also allow to model a
deferred value: one that can be ignored for optimization purposes,
and used in all other cases.

This allows to have a default debuginfo for build dependencies, that can
change:
- if a dependency is shared between the build and runtime subgraphs, the
  default value will be used, to that re-use between units will
  continue.
- otherwise, a build dependency is only used in a context where
  debuginfo is not very useful. We can optimize build times in this
  situation by not asking rustc to emit debuginfo.
  • Loading branch information
lqd committed Jan 31, 2023
commit 79dd5117dac51ab56622a94ca984db50b08625ac
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/custom_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Job> {
// carried over.
let to_exec = to_exec.into_os_string();
let mut cmd = cx.compilation.host_process(to_exec, &unit.pkg)?;
let debug = unit.profile.debuginfo.unwrap_or(0) != 0;
let debug = unit.profile.debuginfo.is_turned_on();
cmd.env("OUT_DIR", &script_out_dir)
.env("CARGO_MANIFEST_DIR", unit.pkg.root())
.env("NUM_JOBS", &bcx.jobs().to_string())
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/job_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -985,7 +985,7 @@ impl<'cfg> DrainState<'cfg> {
} else {
"optimized"
});
if profile.debuginfo.unwrap_or(0) != 0 {
if profile.debuginfo.is_turned_on() {
opt_type += " + debuginfo";
}

Expand Down
4 changes: 2 additions & 2 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ fn link_targets(cx: &mut Context<'_, '_>, unit: &Unit, fresh: bool) -> CargoResu
if json_messages {
let art_profile = machine_message::ArtifactProfile {
opt_level: profile.opt_level.as_str(),
debuginfo: profile.debuginfo,
debuginfo: profile.debuginfo.to_option(),
debug_assertions: profile.debug_assertions,
overflow_checks: profile.overflow_checks,
test: unit_mode.is_any_test(),
Expand Down Expand Up @@ -1003,7 +1003,7 @@ fn build_base_args(
cmd.arg("-C").arg(&format!("codegen-units={}", n));
}

if let Some(debuginfo) = debuginfo {
if let Some(debuginfo) = debuginfo.to_option() {
cmd.arg("-C").arg(format!("debuginfo={}", debuginfo));
}

Expand Down
94 changes: 87 additions & 7 deletions src/cargo/core/profiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ impl Profiles {
// 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 let Some(debug) = profile.debuginfo.to_option() {
if profile.split_debuginfo.is_none() && debug > 0 {
let target = match &kind {
CompileKind::Host => self.rustc_host.as_str(),
Expand Down Expand Up @@ -515,9 +515,9 @@ fn merge_profile(profile: &mut Profile, toml: &TomlProfile) {
profile.codegen_units = toml.codegen_units;
}
match toml.debug {
Some(U32OrBool::U32(debug)) => profile.debuginfo = Some(debug),
Some(U32OrBool::Bool(true)) => profile.debuginfo = Some(2),
Some(U32OrBool::Bool(false)) => profile.debuginfo = None,
Some(U32OrBool::U32(debug)) => profile.debuginfo = DebugInfo::Explicit(debug),
Some(U32OrBool::Bool(true)) => profile.debuginfo = DebugInfo::Explicit(2),
Some(U32OrBool::Bool(false)) => profile.debuginfo = DebugInfo::None,
None => {}
}
if let Some(debug_assertions) = toml.debug_assertions {
Expand Down Expand Up @@ -578,7 +578,7 @@ pub struct Profile {
pub codegen_backend: Option<InternedString>,
// `None` means use rustc default.
pub codegen_units: Option<u32>,
pub debuginfo: Option<u32>,
pub debuginfo: DebugInfo,
pub split_debuginfo: Option<InternedString>,
pub debug_assertions: bool,
pub overflow_checks: bool,
Expand All @@ -600,7 +600,7 @@ impl Default for Profile {
lto: Lto::Bool(false),
codegen_backend: None,
codegen_units: None,
debuginfo: None,
debuginfo: DebugInfo::None,
debug_assertions: false,
split_debuginfo: None,
overflow_checks: false,
Expand Down Expand Up @@ -669,7 +669,7 @@ impl Profile {
Profile {
name: InternedString::new("dev"),
root: ProfileRoot::Debug,
debuginfo: Some(2),
debuginfo: DebugInfo::Explicit(2),
debug_assertions: true,
overflow_checks: true,
incremental: true,
Expand Down Expand Up @@ -708,6 +708,86 @@ impl Profile {
}
}

/// The debuginfo level in a given profile. This is semantically an
/// `Option<u32>`, and should be used as so via the `to_option` method for all
/// intents and purposes:
/// - `DebugInfo::None` corresponds to `None`
/// - `DebugInfo::Explicit(u32)` and `DebugInfo::Deferred` correspond to
/// `Option<u32>::Some`
///
/// Internally, it's used to model a debuginfo level whose value can be deferred
/// for optimization purposes: host dependencies usually don't need the same
/// level as target dependencies. For dependencies that are shared between the
/// two however, that value also affects reuse: different debuginfo levels would
/// cause to build a unit twice. By deferring the choice until we know
/// whether to choose the optimized value or the default value, we can make sure
/// the unit is only built once and the unit graph is still optimized.
#[derive(Debug, Copy, Clone)]
pub enum DebugInfo {
/// No debuginfo
None,
/// A debuginfo level that is explicitly set.
Explicit(u32),
/// For internal purposes: a deferred debuginfo level that can be optimized
/// away but has a default value otherwise.
/// Behaves like `Explicit` in all situations except when dependencies are
/// shared between build dependencies and runtime dependencies. The former
/// case can be optimized, in all other situations this level value will be
/// the one to use.
Deferred(u32),
}

impl DebugInfo {
/// The main way to interact with this debuginfo level, turning it into an Option.
pub fn to_option(&self) -> Option<u32> {
match self {
DebugInfo::None => None,
DebugInfo::Explicit(v) | DebugInfo::Deferred(v) => Some(*v),
}
}

/// Returns true if the debuginfo level is high enough (at least 1). Helper
/// for a common operation on the usual `Option` representation.
pub(crate) fn is_turned_on(&self) -> bool {
self.to_option().unwrap_or(0) != 0
}
}

impl serde::Serialize for DebugInfo {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
weihanglo marked this conversation as resolved.
Show resolved Hide resolved
where
S: serde::Serializer,
{
self.to_option().serialize(serializer)
}
}

impl PartialEq for DebugInfo {
fn eq(&self, other: &DebugInfo) -> bool {
self.to_option().eq(&other.to_option())
}
}

impl Eq for DebugInfo {}

impl Hash for DebugInfo {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
self.to_option().hash(state);
}
}

impl PartialOrd for DebugInfo {
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
self.to_option().partial_cmp(&other.to_option())
}
}

impl Ord for DebugInfo {
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
self.to_option().cmp(&other.to_option())
}
}

/// The link-time-optimization setting.
#[derive(Clone, Copy, PartialEq, Eq, Debug, Hash, PartialOrd, Ord)]
pub enum Lto {
Expand Down
6 changes: 3 additions & 3 deletions tests/testsuite/profile_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ fn named_config_profile() {
assert_eq!(p.name, "foo");
assert_eq!(p.codegen_units, Some(2)); // "foo" from config
assert_eq!(p.opt_level, "1"); // "middle" from manifest
assert_eq!(p.debuginfo, Some(1)); // "bar" from config
assert_eq!(p.debuginfo.to_option(), Some(1)); // "bar" from config
assert_eq!(p.debug_assertions, true); // "dev" built-in (ignore build-override)
assert_eq!(p.overflow_checks, true); // "dev" built-in (ignore package override)

Expand All @@ -445,7 +445,7 @@ fn named_config_profile() {
assert_eq!(bo.name, "foo");
assert_eq!(bo.codegen_units, Some(6)); // "foo" build override from config
assert_eq!(bo.opt_level, "0"); // default to zero
assert_eq!(bo.debuginfo, Some(1)); // SAME as normal
assert_eq!(bo.debuginfo.to_option(), Some(1)); // SAME as normal
assert_eq!(bo.debug_assertions, false); // "foo" build override from manifest
assert_eq!(bo.overflow_checks, true); // SAME as normal

Expand All @@ -454,7 +454,7 @@ fn named_config_profile() {
assert_eq!(po.name, "foo");
assert_eq!(po.codegen_units, Some(7)); // "foo" package override from config
assert_eq!(po.opt_level, "1"); // SAME as normal
assert_eq!(po.debuginfo, Some(1)); // SAME as normal
assert_eq!(po.debuginfo.to_option(), Some(1)); // SAME as normal
assert_eq!(po.debug_assertions, true); // SAME as normal
assert_eq!(po.overflow_checks, false); // "middle" package override from manifest
}
Expand Down