Fast clone graph

Cargo.toml

@@ -40,6 +40,7 @@ hex = "0.4"
 home = "0.5"
 humantime = "1.2.0"
 ignore = "0.4.7"
+indexmap = "1.0.2"
 lazy_static = "1.2.0"
 jobserver = "0.1.13"
 lazycell = "1.2.0"

crates/resolver-tests/src/lib.rs

@@ -70,7 +70,7 @@ pub fn resolve_and_validated(
                 let mut self_pub_dep = HashSet::new();
                 self_pub_dep.insert(p);
                 for (dp, deps) in resolve.deps(p) {
-                    if deps.iter().any(|d| d.is_public()) {
+                    if deps.clone().any(|d| d.is_public()) {
                         self_pub_dep.extend(pub_deps[&dp].iter().cloned())
                     }
                 }

src/cargo/core/compiler/unit_dependencies.rs

@@ -225,9 +225,9 @@ fn compute_deps<'a, 'cfg>(
 
     let bcx = state.bcx;
     let id = unit.pkg.package_id();
-    let deps = state.resolve().deps(id).filter(|&(_id, deps)| {
+    let deps = state.resolve().deps(id).filter(|(_id, deps)| {
         assert!(!deps.is_empty());
-        deps.iter().any(|dep| {
+        (*deps).clone().any(|dep| {
             // If this target is a build command, then we only want build
             // dependencies, otherwise we want everything *other than* build
             // dependencies.
@@ -391,8 +391,8 @@ fn compute_deps_doc<'a, 'cfg>(
     let deps = state
         .resolve()
         .deps(unit.pkg.package_id())
-        .filter(|&(_id, deps)| {
-            deps.iter().any(|dep| match dep.kind() {
+        .filter(|(_id, deps)| {
+            (*deps).clone().any(|dep| match dep.kind() {
                 DepKind::Normal => bcx.dep_platform_activated(dep, unit.kind),
                 _ => false,
             })

src/cargo/core/resolver/context.rs

@@ -1,13 +1,12 @@
 use std::collections::HashMap;
 use std::num::NonZeroU64;
-use std::rc::Rc;
 
 use failure::format_err;
 use log::debug;
 
 use crate::core::interning::InternedString;
 use crate::core::{Dependency, PackageId, SourceId, Summary};
-use crate::util::Graph;
+use crate::util::{Graph, StackGraph};
 
 use super::dep_cache::RegistryQueryer;
 use super::errors::ActivateResult;
@@ -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: StackGraph<PackageId, Dependency>,
 }
 
 /// When backtracking it can be useful to know how far back to go.
@@ -90,7 +89,7 @@ impl Context {
             } else {
                 None
             },
-            parents: Graph::new(),
+            parents: StackGraph::new(),
             activations: im_rc::HashMap::new(),
         }
     }
@@ -224,17 +223,17 @@ 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, Dependency> {
+        let mut graph: Graph<PackageId, Dependency> = Graph::new();
         self.activations
             .values()
             .for_each(|(r, _)| graph.add(r.package_id()));
-        for i in self.parents.iter() {
+        for i in self.parents.borrow().iter() {
             graph.add(*i);
-            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();
+            for (o, edges) in self.parents.borrow().edges(i) {
+                for e in edges {
+                    graph.add_edge(*o, *i, e.clone())
+                }
             }
         }
         graph

src/cargo/core/resolver/errors.rs

@@ -77,10 +77,11 @@ pub(super) fn activation_error(
     candidates: &[Summary],
     config: Option<&Config>,
 ) -> ResolveError {
+    let parents = cx.parents.clone_into_graph();
     let to_resolve_err = |err| {
         ResolveError::new(
             err,
-            cx.parents
+            parents
                 .path_to_bottom(&parent.package_id())
                 .into_iter()
                 .cloned()
@@ -92,7 +93,7 @@ pub(super) fn activation_error(
         let mut msg = format!("failed to select a version for `{}`.", dep.package_name());
         msg.push_str("\n    ... required by ");
         msg.push_str(&describe_path(
-            &cx.parents.path_to_bottom(&parent.package_id()),
+            &parents.path_to_bottom(&parent.package_id()),
         ));
 
         msg.push_str("\nversions that meet the requirements `");
@@ -124,7 +125,7 @@ pub(super) fn activation_error(
                 msg.push_str(link);
                 msg.push_str("` as well:\n");
             }
-            msg.push_str(&describe_path(&cx.parents.path_to_bottom(p)));
+            msg.push_str(&describe_path(&parents.path_to_bottom(p)));
         }
 
         let (features_errors, mut other_errors): (Vec<_>, Vec<_>) = other_errors
@@ -178,7 +179,7 @@ pub(super) fn activation_error(
 
         for &(p, _) in other_errors.iter() {
             msg.push_str("\n\n  previously selected ");
-            msg.push_str(&describe_path(&cx.parents.path_to_bottom(p)));
+            msg.push_str(&describe_path(&parents.path_to_bottom(p)));
         }
 
         msg.push_str("\n\nfailed to select a version for `");
@@ -230,7 +231,7 @@ pub(super) fn activation_error(
             );
             msg.push_str("required by ");
             msg.push_str(&describe_path(
-                &cx.parents.path_to_bottom(&parent.package_id()),
+                &parents.path_to_bottom(&parent.package_id()),
             ));
 
             // If we have a path dependency with a locked version, then this may
@@ -307,7 +308,7 @@ pub(super) fn activation_error(
             }
             msg.push_str("required by ");
             msg.push_str(&describe_path(
-                &cx.parents.path_to_bottom(&parent.package_id()),
+                &parents.path_to_bottom(&parent.package_id()),
             ));
 
             msg

src/cargo/core/resolver/mod.rs

@@ -596,12 +596,8 @@ fn activate(
     let candidate_pid = candidate.package_id();
     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 and associate dep with that edge
+        cx.parents.add_edge(candidate_pid, parent_pid, dep.clone());
         if let Some(public_dependency) = cx.public_dependency.as_mut() {
             // 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
@@ -643,8 +639,8 @@ fn activate(
                     // if `candidate_pid` was a private dependency of `p` then `p` parents can't see `c` thru `p`
                     if public {
                         // if it was public, then we add all of `p`s parents to be checked
-                        for &(grand, ref d) in cx.parents.edges(&p) {
-                            stack.push((grand, d.iter().any(|x| x.is_public())));
+                        for (&grand, mut d) in cx.parents.borrow().edges(&p) {
+                            stack.push((grand, d.any(|x| x.is_public())));
                         }
                     }
                 }
@@ -825,8 +821,8 @@ impl RemainingCandidates {
                         // if `b` was a private dependency of `p` then `p` parents can't see `t` thru `p`
                         if public {
                             // if it was public, then we add all of `p`s parents to be checked
-                            for &(grand, ref d) in cx.parents.edges(&p) {
-                                stack.push((grand, d.iter().any(|x| x.is_public())));
+                            for (&grand, mut d) in cx.parents.borrow().edges(&p) {
+                                stack.push((grand, d.any(|x| x.is_public())));
                             }
                         }
                     }
@@ -884,13 +880,16 @@ fn generalize_conflicting(
         return None;
     }
     // What parents does that critical activation have
-    for (critical_parent, critical_parents_deps) in
-        cx.parents.edges(&backtrack_critical_id).filter(|(p, _)| {
+    for (critical_parent, critical_parents_deps) in cx
+        .parents
+        .borrow()
+        .edges(&backtrack_critical_id)
+        .filter(|(&p, _)| {
             // it will only help backjump further if it is older then the critical_age
-            cx.is_active(*p).expect("parent not currently active!?") < backtrack_critical_age
+            cx.is_active(p).expect("parent not currently active!?") < backtrack_critical_age
         })
     {
-        for critical_parents_dep in critical_parents_deps.iter() {
+        for critical_parents_dep in critical_parents_deps {
             // A dep is equivalent to one of the things it can resolve to.
             // Thus, if all the things it can resolve to have already ben determined
             // to be conflicting, then we can just say that we conflict with the parent.
@@ -1083,8 +1082,8 @@ fn check_cycles(resolve: &Resolve) -> CargoResult<()> {
         // visitation list as we can't induce a cycle through transitive
         // dependencies.
         if checked.insert(id) {
-            for (dep, listings) in resolve.deps_not_replaced(id) {
-                let is_transitive = listings.iter().any(|d| d.is_transitive());
+            for (dep, mut listings) in resolve.deps_not_replaced(id) {
+                let is_transitive = listings.any(|d| d.is_transitive());
                 let mut empty = HashSet::new();
                 let visited = if is_transitive {
                     &mut *visited

src/cargo/core/resolver/resolve.rs

@@ -1,12 +1,13 @@
 use std::borrow::Borrow;
 use std::collections::{HashMap, HashSet};
 use std::fmt;
+use std::hash::Hash;
 use std::iter::FromIterator;
 
 use crate::core::dependency::Kind;
 use crate::core::{Dependency, PackageId, PackageIdSpec, Summary, Target};
 use crate::util::errors::CargoResult;
-use crate::util::Graph;
+use crate::util::graph::{self, Graph};
 
 use super::encode::Metadata;
 
@@ -18,9 +19,9 @@ use super::encode::Metadata;
 #[derive(PartialEq)]
 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
+    /// from `Cargo.toml`. We may have more then one here because the same package
     /// might be present in both `[dependencies]` and `[build-dependencies]`.
-    graph: Graph<PackageId, Vec<Dependency>>,
+    graph: Graph<PackageId, Dependency>,
     /// Replacements from the `[replace]` table.
     replacements: HashMap<PackageId, PackageId>,
     /// Inverted version of `replacements`.
@@ -67,7 +68,7 @@ pub enum ResolveVersion {
 
 impl Resolve {
     pub fn new(
-        graph: Graph<PackageId, Vec<Dependency>>,
+        graph: Graph<PackageId, Dependency>,
         replacements: HashMap<PackageId, PackageId>,
         features: HashMap<PackageId, HashSet<String>>,
         checksums: HashMap<PackageId, Option<String>>,
@@ -82,7 +83,8 @@ impl Resolve {
                 let public_deps = graph
                     .edges(p)
                     .filter(|(_, deps)| {
-                        deps.iter()
+                        (*deps)
+                            .clone()
                             .any(|d| d.kind() == Kind::Normal && d.is_public())
                     })
                     .map(|(dep_package, _)| *dep_package)
@@ -232,7 +234,7 @@ unable to verify that `{0}` is the same as when the lockfile was generated
     pub fn contains<Q: ?Sized>(&self, k: &Q) -> bool
     where
         PackageId: Borrow<Q>,
-        Q: Ord + Eq,
+        Q: Hash + Eq,
     {
         self.graph.contains(k)
     }
@@ -245,18 +247,19 @@ 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, graph::Edges<'_, 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, graph::Edges<'_, Dependency>)> {
+        self.graph.edges(&pkg).map(|(id, deps)| (*id, deps))
     }
 
     pub fn replacement(&self, pkg: PackageId) -> Option<PackageId> {
@@ -306,14 +309,10 @@ unable to verify that `{0}` is the same as when the lockfile was generated
         to: PackageId,
         to_target: &Target,
     ) -> CargoResult<String> {
-        let deps = if from == to {
-            &[]
-        } else {
-            self.dependencies_listed(from, to)
-        };
+        let deps = self.dependencies_listed(from, to);
 
         let crate_name = to_target.crate_name();
-        let mut names = deps.iter().map(|d| {
+        let mut names = deps.map(|d| {
             d.explicit_name_in_toml()
                 .map(|s| s.as_str().replace("-", "_"))
                 .unwrap_or_else(|| crate_name.clone())
@@ -330,7 +329,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) -> graph::Edges<'_, 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)
@@ -341,14 +340,16 @@ unable to verify that `{0}` is the same as when the lockfile was generated
         // that's where the dependency originates from, and we only replace
         // targets of dependencies not the originator.
         if let Some(replace) = self.reverse_replacements.get(&to) {
-            if let Some(deps) = self.graph.edge(&from, replace) {
+            let deps = self.graph.edge(&from, replace);
+            if !deps.is_empty() {
                 return deps;
             }
         }
-        match self.graph.edge(&from, &to) {
-            Some(ret) => ret,
-            None => panic!("no Dependency listed for `{}` => `{}`", from, to),
+        let deps = self.graph.edge(&from, &to);
+        if deps.is_empty() && from != to {
+            panic!("no Dependency listed for `{}` => `{}`", from, to);
         }
+        deps
     }
 
     /// Returns the version of the encoding that's being used for this lock

src/cargo/ops/cargo_fetch.rs

@@ -41,8 +41,8 @@ pub fn fetch<'a>(
         to_download.push(id);
         let deps = resolve
             .deps(id)
-            .filter(|&(_id, deps)| {
-                deps.iter().any(|d| {
+            .filter(|(_id, deps)| {
+                (*deps).clone().any(|d| {
                     // If no target was specified then all dependencies can
                     // be fetched.
                     let target = match options.target {