From 78b8d52e78bbe8b1023e78fb3d1169fe247c762f Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Thu, 23 Jul 2015 16:28:12 -0700 Subject: [PATCH] Implement selective optimization levels on a per-dependency basis. Inside the `dependencies.foo` section, you may now specify a minimum optimization level to be applied to this dependency. If multiple such optimization levels are supplied, Cargo chooses the highest one. This will probably need to go through an RFC before landing. Addresses #1359. --- src/cargo/core/dependency.rs | 10 +++ src/cargo/core/resolver/encode.rs | 1 + src/cargo/core/resolver/mod.rs | 35 ++++++++++- src/cargo/ops/cargo_compile.rs | 5 +- src/cargo/ops/cargo_rustc/mod.rs | 1 + src/cargo/util/toml.rs | 6 +- src/doc/manifest.md | 15 +++++ tests/test_cargo_opt_levels.rs | 101 ++++++++++++++++++++++++++++++ tests/tests.rs | 1 + 9 files changed, 171 insertions(+), 4 deletions(-) create mode 100644 tests/test_cargo_opt_levels.rs diff --git a/src/cargo/core/dependency.rs b/src/cargo/core/dependency.rs index bf7378fd51b..6ae3c656b9d 100644 --- a/src/cargo/core/dependency.rs +++ b/src/cargo/core/dependency.rs @@ -16,6 +16,7 @@ pub struct Dependency { optional: bool, default_features: bool, features: Vec, + opt_level: Option, // This dependency should be used only for this platform. // `None` means *all platforms*. @@ -59,6 +60,7 @@ impl Dependency { default_features: true, specified_req: None, only_for_platform: None, + opt_level: None, } } @@ -111,6 +113,12 @@ impl Dependency { self } + /// Set the minimum optimization level for this dependency + pub fn set_opt_level(mut self, opt_level: u32) -> Dependency { + self.opt_level = Some(opt_level); + self + } + pub fn set_only_for_platform(mut self, platform: Option) -> Dependency { self.only_for_platform = platform; @@ -140,6 +148,8 @@ impl Dependency { pub fn uses_default_features(&self) -> bool { self.default_features } /// Returns the list of features that are requested by the dependency. pub fn features(&self) -> &[String] { &self.features } + /// Returns the optimization level requested for this dependency. + pub fn opt_level(&self) -> Option { self.opt_level } /// Returns true if the package (`sum`) can fulfill this dependency request. pub fn matches(&self, sum: &Summary) -> bool { diff --git a/src/cargo/core/resolver/encode.rs b/src/cargo/core/resolver/encode.rs index 9ef4a4ff46f..3df9050f267 100644 --- a/src/cargo/core/resolver/encode.rs +++ b/src/cargo/core/resolver/encode.rs @@ -73,6 +73,7 @@ impl EncodableResolve { graph: g, root: try!(self.root.to_package_id(default)), features: HashMap::new(), + opt_levels: HashMap::new(), metadata: self.metadata.clone(), }) } diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index c7c245b2c48..f66f10a94bd 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -93,8 +93,9 @@ //! for a bit longer as well! use std::cell::RefCell; +use std::cmp; use std::collections::HashSet; -use std::collections::hash_map::HashMap; +use std::collections::hash_map::{Entry, HashMap}; use std::fmt; use std::rc::Rc; use std::slice; @@ -120,6 +121,7 @@ mod encode; pub struct Resolve { graph: Graph, features: HashMap>, + opt_levels: HashMap, root: PackageId, metadata: Option, } @@ -132,6 +134,7 @@ pub enum Method<'a> { features: &'a [String], uses_default_features: bool, target_platform: Option<&'a str>, + opt_level: Option, }, } @@ -149,7 +152,13 @@ impl Resolve { fn new(root: PackageId) -> Resolve { let mut g = Graph::new(); g.add(root.clone(), &[]); - Resolve { graph: g, root: root, features: HashMap::new(), metadata: None } + Resolve { + graph: g, + root: root, + features: HashMap::new(), + opt_levels: HashMap::new(), + metadata: None, + } } pub fn copy_metadata(&mut self, other: &Resolve) { @@ -215,6 +224,10 @@ impl Resolve { pub fn features(&self, pkg: &PackageId) -> Option<&HashSet> { self.features.get(pkg) } + + pub fn opt_level(&self, pkg: &PackageId) -> Option { + self.opt_levels.get(pkg).map(|opt_level| *opt_level) + } } impl fmt::Debug for Resolve { @@ -329,6 +342,7 @@ fn activate_deps<'a>(cx: Box, features: features, uses_default_features: dep.uses_default_features(), target_platform: platform, + opt_level: dep.opt_level(), }; let prev_active = cx.prev_active(dep); @@ -630,6 +644,23 @@ impl Context { // for our own dependencies. let deps = try!(self.resolve_features(parent, method)); + // Record the optimization level for this package. + //println!("about to record optimzation level for {:?}...", parent.package_id()); + if let Method::Required{opt_level: Some(opt_level), ..} = method { + //println!("recording optimization level {}!", opt_level); + match self.resolve.opt_levels.entry(parent.package_id().clone()) { + Entry::Occupied(mut entry) => { + let max_opt_level = cmp::max(*entry.get(), opt_level); + entry.insert(max_opt_level); + } + Entry::Vacant(entry) => { + entry.insert(opt_level); + } + } + } else { + //println!("no optimization level present!"); + } + // Next, transform all dependencies into a list of possible candidates // which can satisfy that dependency. let mut deps = try!(deps.into_iter().map(|(dep, features)| { diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index a927208042b..35ef7a47ba3 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -148,11 +148,14 @@ pub fn compile_pkg<'a>(package: &Package, let platform = target.as_ref().map(|e| &e[..]).or(Some(&rustc_host[..])); + // We specify no optimization level here because at the top level + // the user can already effectively specify that via the profile. let method = Method::Required{ dev_deps: true, // TODO: remove this option? features: &features, uses_default_features: !no_default_features, - target_platform: platform}; + target_platform: platform, + opt_level: None}; let resolved_with_overrides = try!(ops::resolve_with_previous(&mut registry, package, method, diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index 486ce8d5c89..c3a18921eb3 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -626,6 +626,7 @@ fn build_base_args(cx: &Context, cmd.arg("-C").arg("prefer-dynamic"); } + let opt_level = cx.resolve.opt_level(pkg.package_id()).unwrap_or(opt_level); if opt_level != 0 { cmd.arg("-C").arg(&format!("opt-level={}", opt_level)); } diff --git a/src/cargo/util/toml.rs b/src/cargo/util/toml.rs index 2b488beb83c..c24a72fac10 100644 --- a/src/cargo/util/toml.rs +++ b/src/cargo/util/toml.rs @@ -206,6 +206,7 @@ pub struct DetailedTomlDependency { features: Option>, optional: Option, default_features: Option, + opt_level: Option, } #[derive(RustcDecodable)] @@ -665,10 +666,13 @@ fn process_dependencies(cx: &mut Context, details.version.as_ref() .map(|v| &v[..]), &new_source_id)); - let dep = f(dep) + let mut dep = f(dep) .set_features(details.features.unwrap_or(Vec::new())) .set_default_features(details.default_features.unwrap_or(true)) .set_optional(details.optional.unwrap_or(false)); + if let Some(opt_level) = details.opt_level { + dep = dep.set_opt_level(opt_level) + } cx.deps.push(dep); } diff --git a/src/doc/manifest.md b/src/doc/manifest.md index a49e3a81754..fd226d7061d 100644 --- a/src/doc/manifest.md +++ b/src/doc/manifest.md @@ -191,6 +191,20 @@ openssl = "1.0.1" native = { path = "native/x86_64" } ``` +Sometimes, you may wish a dependency to be compiled at a minimum optimization +level. This is useful if you strongly suspect you will not need to debug the +dependency itself. To use this feature, specify an `opt_level` key in the +dependency table as follows: + +```toml +[dependencies.hammer] +version = "0.5.0" +opt_level = 2 +``` + +If multiple crates specify different optimization levels for the same +dependency, Cargo chooses the highest level of optimization. + # The `[profile.*]` Sections Cargo supports custom configuration of how rustc is invoked through **profiles** @@ -504,3 +518,4 @@ crate-type = ["dylib"] The available options are `dylib`, `rlib`, and `staticlib`. You should only use this option in a project. Cargo will always compile **packages** (dependencies) based on the requirements of the project that includes them. + diff --git a/tests/test_cargo_opt_levels.rs b/tests/test_cargo_opt_levels.rs new file mode 100644 index 00000000000..aea7738b3a7 --- /dev/null +++ b/tests/test_cargo_opt_levels.rs @@ -0,0 +1,101 @@ +use std::path::MAIN_SEPARATOR as SEP; + +use support::{project, execs}; +use support::{COMPILING, RUNNING}; +use hamcrest::assert_that; + +fn setup() { +} + +test!(basic_per_dependency_opt_levels { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies.bar] + path = "bar" + opt_level = 2 + "#) + .file("src/main.rs", r#" + extern crate bar; + fn main() {} + "#) + .file("bar/Cargo.toml", r#" + [package] + name = "bar" + version = "0.0.1" + authors = [] + "#) + .file("bar/src/lib.rs", "pub fn bar() {}"); + + assert_that(p.cargo_process("build").arg("-v"), + execs().with_status(0).with_stdout(format!("\ +{compiling} bar v0.0.1 ({url}) +{running} `rustc bar{sep}src{sep}lib.rs --crate-name bar --crate-type lib \ + -C opt-level=2 -g [..] +{compiling} foo v0.0.1 ({url}) +{running} `rustc src{sep}main.rs --crate-name foo --crate-type bin -g \ + --out-dir {dir}{sep}target{sep}debug [..]", + running = RUNNING, compiling = COMPILING, sep = SEP, + dir = p.root().display(), url = p.url()))); +}); + +test!(highest_opt_level_wins_in_per_dependency_opt_levels { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies.bar] + path = "bar" + + [dependencies.baz] + path = "baz" + opt_level = 1 + "#) + .file("src/main.rs", r#" + extern crate bar; + fn main() {} + "#) + .file("bar/Cargo.toml", r#" + [package] + name = "bar" + version = "0.0.1" + authors = [] + + [dependencies.baz] + path = "../baz" + opt_level = 2 + "#) + .file("bar/src/lib.rs", r#" + extern crate baz; + pub fn bar() {} + "#) + .file("baz/Cargo.toml", r#" + [package] + name = "baz" + version = "0.0.1" + authors = [] + "#) + .file("baz/src/lib.rs", "pub fn baz() {}"); + + assert_that(p.cargo_process("build").arg("-v"), + execs().with_status(0).with_stdout(format!("\ +{compiling} baz v0.0.1 ({url}) +{running} `rustc baz{sep}src{sep}lib.rs --crate-name baz --crate-type lib \ + -C opt-level=2 -g [..] +{compiling} bar v0.0.1 ({url}) +{running} `rustc bar{sep}src{sep}lib.rs --crate-name bar --crate-type lib \ + -g [..] +{compiling} foo v0.0.1 ({url}) +{running} `rustc src{sep}main.rs --crate-name foo --crate-type bin -g \ + --out-dir {dir}{sep}target{sep}debug [..]", + running = RUNNING, compiling = COMPILING, sep = SEP, + dir = p.root().display(), url = p.url()))); +}); + diff --git a/tests/tests.rs b/tests/tests.rs index b82c2ecf5ff..307c3a2c98d 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -46,6 +46,7 @@ mod test_cargo_fetch; mod test_cargo_freshness; mod test_cargo_generate_lockfile; mod test_cargo_new; +mod test_cargo_opt_levels; mod test_cargo_package; mod test_cargo_profiles; mod test_cargo_publish;