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

De-duplicate edges #7993

Merged
merged 3 commits into from
Mar 16, 2020
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
15 changes: 7 additions & 8 deletions src/cargo/core/resolver/context.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::collections::HashMap;
use std::num::NonZeroU64;
use std::rc::Rc;

use anyhow::format_err;
use log::debug;
Expand Down Expand Up @@ -35,7 +34,7 @@ pub struct Context {

/// a way to look up for a package in activations what packages required it
/// and all of the exact deps that it fulfilled.
pub parents: Graph<PackageId, Rc<Vec<Dependency>>>,
pub parents: Graph<PackageId, im_rc::HashSet<Dependency>>,
}

/// When backtracking it can be useful to know how far back to go.
Expand Down Expand Up @@ -255,8 +254,8 @@ impl Context {
.collect()
}

pub fn graph(&self) -> Graph<PackageId, Vec<Dependency>> {
let mut graph: Graph<PackageId, Vec<Dependency>> = Graph::new();
pub fn graph(&self) -> Graph<PackageId, std::collections::HashSet<Dependency>> {
let mut graph: Graph<PackageId, std::collections::HashSet<Dependency>> = Graph::new();
self.activations
.values()
.for_each(|(r, _)| graph.add(r.package_id()));
Expand All @@ -265,14 +264,14 @@ impl Context {
for (o, e) in self.parents.edges(i) {
let old_link = graph.link(*o, *i);
assert!(old_link.is_empty());
*old_link = e.to_vec();
*old_link = e.iter().cloned().collect();
}
}
graph
}
}

impl Graph<PackageId, Rc<Vec<Dependency>>> {
impl Graph<PackageId, im_rc::HashSet<Dependency>> {
pub fn parents_of(&self, p: PackageId) -> impl Iterator<Item = (PackageId, bool)> + '_ {
self.edges(&p)
.map(|(grand, d)| (*grand, d.iter().any(|x| x.is_public())))
Expand Down Expand Up @@ -338,7 +337,7 @@ impl PublicDependency {
parent_pid: PackageId,
is_public: bool,
age: ContextAge,
parents: &Graph<PackageId, Rc<Vec<Dependency>>>,
parents: &Graph<PackageId, im_rc::HashSet<Dependency>>,
) {
// one tricky part is that `candidate_pid` may already be active and
// have public dependencies of its own. So we not only need to mark
Expand Down Expand Up @@ -383,7 +382,7 @@ impl PublicDependency {
b_id: PackageId,
parent: PackageId,
is_public: bool,
parents: &Graph<PackageId, Rc<Vec<Dependency>>>,
parents: &Graph<PackageId, im_rc::HashSet<Dependency>>,
) -> Result<
(),
(
Expand Down
11 changes: 5 additions & 6 deletions src/cargo/core/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -609,12 +609,11 @@ fn activate(
cx.age += 1;
if let Some((parent, dep)) = parent {
let parent_pid = parent.package_id();
Rc::make_mut(
// add a edge from candidate to parent in the parents graph
cx.parents.link(candidate_pid, parent_pid),
)
// and associate dep with that edge
.push(dep.clone());
// add a edge from candidate to parent in the parents graph
cx.parents
.link(candidate_pid, parent_pid)
// and associate dep with that edge
.insert(dep.clone());
if let Some(public_dependency) = cx.public_dependency.as_mut() {
public_dependency.add_edge(
candidate_pid,
Expand Down
17 changes: 8 additions & 9 deletions src/cargo/core/resolver/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub struct Resolve {
/// A graph, whose vertices are packages and edges are dependency specifications
/// from `Cargo.toml`. We need a `Vec` here because the same package
/// might be present in both `[dependencies]` and `[build-dependencies]`.
graph: Graph<PackageId, Vec<Dependency>>,
graph: Graph<PackageId, HashSet<Dependency>>,
/// Replacements from the `[replace]` table.
replacements: HashMap<PackageId, PackageId>,
/// Inverted version of `replacements`.
Expand Down Expand Up @@ -70,7 +70,7 @@ pub enum ResolveVersion {

impl Resolve {
pub fn new(
graph: Graph<PackageId, Vec<Dependency>>,
graph: Graph<PackageId, HashSet<Dependency>>,
replacements: HashMap<PackageId, PackageId>,
features: HashMap<PackageId, Vec<InternedString>>,
checksums: HashMap<PackageId, Option<String>>,
Expand Down Expand Up @@ -264,18 +264,16 @@ unable to verify that `{0}` is the same as when the lockfile was generated
self.graph.iter().cloned()
}

pub fn deps(&self, pkg: PackageId) -> impl Iterator<Item = (PackageId, &[Dependency])> {
pub fn deps(&self, pkg: PackageId) -> impl Iterator<Item = (PackageId, &HashSet<Dependency>)> {
self.deps_not_replaced(pkg)
.map(move |(id, deps)| (self.replacement(id).unwrap_or(id), deps))
}

pub fn deps_not_replaced(
&self,
pkg: PackageId,
) -> impl Iterator<Item = (PackageId, &[Dependency])> {
self.graph
.edges(&pkg)
.map(|(id, deps)| (*id, deps.as_slice()))
) -> impl Iterator<Item = (PackageId, &HashSet<Dependency>)> {
self.graph.edges(&pkg).map(|(id, deps)| (*id, deps))
}

pub fn replacement(&self, pkg: PackageId) -> Option<PackageId> {
Expand Down Expand Up @@ -325,8 +323,9 @@ unable to verify that `{0}` is the same as when the lockfile was generated
to: PackageId,
to_target: &Target,
) -> CargoResult<String> {
let empty_set: HashSet<Dependency> = HashSet::new();
let deps = if from == to {
&[]
&empty_set
} else {
self.dependencies_listed(from, to)
};
Expand All @@ -349,7 +348,7 @@ unable to verify that `{0}` is the same as when the lockfile was generated
Ok(name)
}

fn dependencies_listed(&self, from: PackageId, to: PackageId) -> &[Dependency] {
fn dependencies_listed(&self, from: PackageId, to: PackageId) -> &HashSet<Dependency> {
// We've got a dependency on `from` to `to`, but this dependency edge
// may be affected by [replace]. If the `to` package is listed as the
// target of a replacement (aka the key of a reverse replacement map)
Expand Down
6 changes: 1 addition & 5 deletions src/cargo/ops/cargo_output_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,11 +184,7 @@ fn build_resolve_graph_r(
CompileKind::Host => true,
})
.filter_map(|(dep_id, deps)| {
let mut dep_kinds: Vec<_> = deps.iter().map(DepKindInfo::from).collect();
// Duplicates may appear if the same package is used by different
// members of a workspace with different features selected.
dep_kinds.sort_unstable();
dep_kinds.dedup();
let dep_kinds: Vec<_> = deps.iter().map(DepKindInfo::from).collect();
package_map
.get(&dep_id)
.and_then(|pkg| pkg.targets().iter().find(|t| t.is_lib()))
Expand Down