Skip to content

Commit

Permalink
Auto merge of #7062 - goffrie:master, r=alexcrichton
Browse files Browse the repository at this point in the history
Fix exponentiality in depend_on_deps_of_deps.

With `CARGO_BUILD_PIPELINING=true`, cargo was spending a long time (15 seconds) before starting any compilation. That is caused by a naive graph traversal in `depend_on_deps_of_deps`. Instead, let's make sure not to keep traversing the same deps. With this patch, things are fast again.
  • Loading branch information
bors committed Jun 25, 2019
2 parents 4c1fa54 + 15e0802 commit 83d086d
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 9 deletions.
17 changes: 8 additions & 9 deletions src/cargo/core/compiler/job_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,12 @@ impl<'a, 'cfg> JobQueue<'a, 'cfg> {
let dependencies = cx.dep_targets(unit);
let mut queue_deps = dependencies
.iter()
.cloned()
.filter(|unit| {
// Binaries aren't actually needed to *compile* tests, just to run
// them, so we don't include this dependency edge in the job graph.
!unit.target.is_test() || !unit.target.is_bin()
})
.cloned()
.map(|dep| {
// Handle the case here where our `unit -> dep` dependency may
// only require the metadata, not the full compilation to
Expand All @@ -172,7 +172,7 @@ impl<'a, 'cfg> JobQueue<'a, 'cfg> {
};
(dep, artifact)
})
.collect::<Vec<_>>();
.collect::<HashMap<_, _>>();

// This is somewhat tricky, but we may need to synthesize some
// dependencies for this target if it requires full upstream
Expand All @@ -196,19 +196,18 @@ impl<'a, 'cfg> JobQueue<'a, 'cfg> {
// `Metadata` propagate upwards `All` dependencies to anything that
// transitively contains the `Metadata` edge.
if unit.requires_upstream_objects() {
for dep in dependencies.iter() {
for dep in dependencies {
depend_on_deps_of_deps(cx, &mut queue_deps, dep);
}

fn depend_on_deps_of_deps<'a>(
cx: &Context<'a, '_>,
deps: &mut Vec<(Unit<'a>, Artifact)>,
unit: &Unit<'a>,
deps: &mut HashMap<Unit<'a>, Artifact>,
unit: Unit<'a>,
) {
for dep in cx.dep_targets(unit) {
if cx.only_requires_rmeta(unit, &dep) {
deps.push((dep, Artifact::All));
depend_on_deps_of_deps(cx, deps, &dep);
for dep in cx.dep_targets(&unit) {
if deps.insert(dep, Artifact::All).is_none() {
depend_on_deps_of_deps(cx, deps, dep);
}
}
}
Expand Down
61 changes: 61 additions & 0 deletions tests/testsuite/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4583,6 +4583,67 @@ fn pipelining_works() {
.run();
}

#[cargo_test]
fn pipelining_big_graph() {
if !crate::support::is_nightly() {
return;
}

// Create a crate graph of the form {a,b}{0..19}, where {a,b}(n) depend on {a,b}(n+1)
// Then have `foo`, a binary crate, depend on the whole thing.
let mut project = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
[dependencies]
a1 = { path = "a1" }
b1 = { path = "b1" }
"#,
)
.file("src/main.rs", "fn main(){}");

for n in 0..30 {
for x in &["a", "b"] {
project = project
.file(
&format!("{x}{n}/Cargo.toml", x = x, n = n),
&format!(
r#"
[package]
name = "{x}{n}"
version = "0.1.0"
[dependencies]
a{np1} = {{ path = "../a{np1}" }}
b{np1} = {{ path = "../b{np1}" }}
"#,
x = x,
n = n,
np1 = n + 1
),
)
.file(&format!("{x}{n}/src/lib.rs", x = x, n = n), "");
}
}

let foo = project
.file("a30/Cargo.toml", &basic_lib_manifest("a30"))
.file(
"a30/src/lib.rs",
r#"compile_error!("don't actually build me");"#,
)
.file("b30/Cargo.toml", &basic_lib_manifest("b30"))
.file("b30/src/lib.rs", "")
.build();
foo.cargo("build -p foo")
.env("CARGO_BUILD_PIPELINING", "true")
.with_status(101)
.with_stderr_contains("[ERROR] Could not compile `a30`[..]")
.run();
}

#[cargo_test]
fn forward_rustc_output() {
let foo = project()
Expand Down

0 comments on commit 83d086d

Please sign in to comment.