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

Fix exponentiality in depend_on_deps_of_deps. #7062

Merged
merged 2 commits into from
Jun 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
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
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