maintain a graph of parents so we can quickly walk to the root

Author: Eh2406

src/cargo/core/resolver/context.rs

@@ -28,6 +28,10 @@ pub struct Context {
     pub resolve_features: im_rc::HashMap<PackageId, Rc<HashSet<InternedString>>>,
     pub links: im_rc::HashMap<InternedString, PackageId>,
 
+    // This is somewhat redundant with the `resolve_graph` that stores the same data,
+    //   but for querying in the opposite order.
+    pub parents: Graph<PackageId, Rc<Vec<Dependency>>>,
+
     // These are two cheaply-cloneable lists (O(1) clone) which are effectively
     // hash maps but are built up as "construction lists". We'll iterate these
     // at the very end and actually construct the map that we're making.
@@ -46,6 +50,7 @@ impl Context {
             resolve_graph: RcList::new(),
             resolve_features: im_rc::HashMap::new(),
             links: im_rc::HashMap::new(),
+            parents: Graph::new(),
             resolve_replacements: RcList::new(),
             activations: im_rc::HashMap::new(),
             warnings: RcList::new(),

src/cargo/core/resolver/errors.rs

@@ -77,12 +77,11 @@ pub(super) fn activation_error(
     candidates: &[Candidate],
     config: Option<&Config>,
 ) -> ResolveError {
-    let graph = cx.graph();
     let to_resolve_err = |err| {
         ResolveError::new(
             err,
-            graph
-                .path_to_top(&parent.package_id())
+            cx.parents
+                .path_to_bottom(&parent.package_id())
                 .into_iter()
                 .cloned()
                 .collect(),
@@ -92,7 +91,9 @@ pub(super) fn activation_error(
     if !candidates.is_empty() {
         let mut msg = format!("failed to select a version for `{}`.", dep.package_name());
         msg.push_str("\n    ... required by ");
-        msg.push_str(&describe_path(&graph.path_to_top(&parent.package_id())));
+        msg.push_str(&describe_path(
+            &cx.parents.path_to_bottom(&parent.package_id()),
+        ));
 
         msg.push_str("\nversions that meet the requirements `");
         msg.push_str(&dep.version_req().to_string());
@@ -123,7 +124,7 @@ pub(super) fn activation_error(
                 msg.push_str(link);
                 msg.push_str("` as well:\n");
             }
-            msg.push_str(&describe_path(&graph.path_to_top(p)));
+            msg.push_str(&describe_path(&cx.parents.path_to_bottom(p)));
         }
 
         let (features_errors, other_errors): (Vec<_>, Vec<_>) = other_errors
@@ -154,7 +155,7 @@ pub(super) fn activation_error(
 
         for &(p, _) in other_errors.iter() {
             msg.push_str("\n\n  previously selected ");
-            msg.push_str(&describe_path(&graph.path_to_top(p)));
+            msg.push_str(&describe_path(&cx.parents.path_to_bottom(p)));
         }
 
         msg.push_str("\n\nfailed to select a version for `");
@@ -204,7 +205,9 @@ pub(super) fn activation_error(
             registry.describe_source(dep.source_id()),
         );
         msg.push_str("required by ");
-        msg.push_str(&describe_path(&graph.path_to_top(&parent.package_id())));
+        msg.push_str(&describe_path(
+            &cx.parents.path_to_bottom(&parent.package_id()),
+        ));
 
         // If we have a path dependency with a locked version, then this may
         // indicate that we updated a sub-package and forgot to run `cargo
@@ -265,7 +268,9 @@ pub(super) fn activation_error(
             msg.push_str("\n");
         }
         msg.push_str("required by ");
-        msg.push_str(&describe_path(&graph.path_to_top(&parent.package_id())));
+        msg.push_str(&describe_path(
+            &cx.parents.path_to_bottom(&parent.package_id()),
+        ));
 
         msg
     };

src/cargo/core/resolver/mod.rs

@@ -591,6 +591,11 @@ fn activate(
             candidate.summary.package_id(),
             dep.clone(),
         ));
+        Rc::make_mut(
+            cx.parents
+                .link(candidate.summary.package_id(), parent.package_id()),
+        )
+        .push(dep.clone());
     }
 
     let activated = cx.flag_activated(&candidate.summary, method)?;

src/cargo/core/resolver/resolve.rs

@@ -1,7 +1,6 @@
 use std::borrow::Borrow;
 use std::collections::{HashMap, HashSet};
 use std::fmt;
-use std::hash::Hash;
 use std::iter::FromIterator;
 
 use url::Url;
@@ -163,7 +162,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: Hash + Eq,
+        Q: Ord + Eq,
     {
         self.graph.contains(k)
     }
@@ -179,11 +178,11 @@ unable to verify that `{0}` is the same as when the lockfile was generated
     pub fn deps(&self, pkg: PackageId) -> impl Iterator<Item = (PackageId, &[Dependency])> {
         self.graph
             .edges(&pkg)
-            .map(move |(&id, deps)| (self.replacement(id).unwrap_or(id), deps.as_slice()))
+            .map(move |&(id, ref deps)| (self.replacement(id).unwrap_or(id), deps.as_slice()))
     }
 
     pub fn deps_not_replaced<'a>(&'a self, pkg: PackageId) -> impl Iterator<Item = PackageId> + 'a {
-        self.graph.edges(&pkg).map(|(&id, _)| id)
+        self.graph.edges(&pkg).map(|&(id, _)| id)
     }
 
     pub fn replacement(&self, pkg: PackageId) -> Option<PackageId> {

src/cargo/util/graph.rs

@@ -1,35 +1,36 @@
 use std::borrow::Borrow;
-use std::collections::{HashMap, HashSet};
+use std::collections::BTreeSet;
 use std::fmt;
-use std::hash::Hash;
 
-pub struct Graph<N, E> {
-    nodes: HashMap<N, HashMap<N, E>>,
+use im_rc;
+
+pub struct Graph<N: Clone, E: Clone> {
+    nodes: im_rc::OrdMap<N, im_rc::OrdMap<N, E>>,
 }
 
-impl<N: Eq + Hash + Clone, E: Default> Graph<N, E> {
+impl<N: Eq + Ord + Clone, E: Default + Clone> Graph<N, E> {
     pub fn new() -> Graph<N, E> {
         Graph {
-            nodes: HashMap::new(),
+            nodes: im_rc::OrdMap::new(),
         }
     }
 
     pub fn add(&mut self, node: N) {
-        self.nodes.entry(node).or_insert_with(HashMap::new);
+        self.nodes.entry(node).or_insert_with(im_rc::OrdMap::new);
     }
 
     pub fn link(&mut self, node: N, child: N) -> &mut E {
         self.nodes
             .entry(node)
-            .or_insert_with(HashMap::new)
+            .or_insert_with(im_rc::OrdMap::new)
             .entry(child)
             .or_insert_with(Default::default)
     }
 
     pub fn contains<Q: ?Sized>(&self, k: &Q) -> bool
     where
         N: Borrow<Q>,
-        Q: Hash + Eq,
+        Q: Ord + Eq,
     {
         self.nodes.contains_key(k)
     }
@@ -38,14 +39,14 @@ impl<N: Eq + Hash + Clone, E: Default> Graph<N, E> {
         self.nodes.get(from)?.get(to)
     }
 
-    pub fn edges(&self, from: &N) -> impl Iterator<Item = (&N, &E)> {
+    pub fn edges(&self, from: &N) -> impl Iterator<Item = &(N, E)> {
         self.nodes.get(from).into_iter().flat_map(|x| x.iter())
     }
 
     /// A topological sort of the `Graph`
     pub fn sort(&self) -> Vec<N> {
         let mut ret = Vec::new();
-        let mut marks = HashSet::new();
+        let mut marks = BTreeSet::new();
 
         for node in self.nodes.keys() {
             self.sort_inner_visit(node, &mut ret, &mut marks);
@@ -54,7 +55,7 @@ impl<N: Eq + Hash + Clone, E: Default> Graph<N, E> {
         ret
     }
 
-    fn sort_inner_visit(&self, node: &N, dst: &mut Vec<N>, marks: &mut HashSet<N>) {
+    fn sort_inner_visit(&self, node: &N, dst: &mut Vec<N>, marks: &mut BTreeSet<N>) {
         if !marks.insert(node.clone()) {
             return;
         }
@@ -70,6 +71,23 @@ impl<N: Eq + Hash + Clone, E: Default> Graph<N, E> {
         self.nodes.keys()
     }
 
+    /// Resolves one of the paths from the given dependent package down to
+    /// a leaf.
+    pub fn path_to_bottom<'a>(&'a self, mut pkg: &'a N) -> Vec<&'a N> {
+        let mut result = vec![pkg];
+        while let Some(p) = self.nodes.get(pkg).and_then(|p| {
+            p.iter()
+                // Note that we can have "cycles" introduced through dev-dependency
+                // edges, so make sure we don't loop infinitely.
+                .find(|&(node, _)| !result.contains(&node))
+                .map(|(ref p, _)| p)
+        }) {
+            result.push(p);
+            pkg = p;
+        }
+        result
+    }
+
     /// Resolves one of the paths from the given dependent package up to
     /// the root.
     pub fn path_to_top<'a>(&'a self, mut pkg: &'a N) -> Vec<&'a N> {
@@ -84,7 +102,7 @@ impl<N: Eq + Hash + Clone, E: Default> Graph<N, E> {
                 // Note that we can have "cycles" introduced through dev-dependency
                 // edges, so make sure we don't loop infinitely.
                 .find(|&(node, _)| !res.contains(&node))
-                .map(|p| p.0)
+                .map(|(ref p, _)| p)
         };
         while let Some(p) = first_pkg_depending_on(pkg, &result) {
             result.push(p);
@@ -94,13 +112,13 @@ impl<N: Eq + Hash + Clone, E: Default> Graph<N, E> {
     }
 }
 
-impl<N: Eq + Hash + Clone, E: Default> Default for Graph<N, E> {
+impl<N: Eq + Ord + Clone, E: Default + Clone> Default for Graph<N, E> {
     fn default() -> Graph<N, E> {
         Graph::new()
     }
 }
 
-impl<N: fmt::Display + Eq + Hash, E> fmt::Debug for Graph<N, E> {
+impl<N: fmt::Display + Eq + Ord + Clone, E: Clone> fmt::Debug for Graph<N, E> {
     fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
         writeln!(fmt, "Graph {{")?;
 
@@ -118,14 +136,14 @@ impl<N: fmt::Display + Eq + Hash, E> fmt::Debug for Graph<N, E> {
     }
 }
 
-impl<N: Eq + Hash, E: Eq> PartialEq for Graph<N, E> {
+impl<N: Eq + Ord + Clone, E: Eq + Clone> PartialEq for Graph<N, E> {
     fn eq(&self, other: &Graph<N, E>) -> bool {
         self.nodes.eq(&other.nodes)
     }
 }
-impl<N: Eq + Hash, E: Eq> Eq for Graph<N, E> {}
+impl<N: Eq + Ord + Clone, E: Eq + Clone> Eq for Graph<N, E> {}
 
-impl<N: Eq + Hash + Clone, E: Clone> Clone for Graph<N, E> {
+impl<N: Eq + Ord + Clone, E: Clone> Clone for Graph<N, E> {
     fn clone(&self) -> Graph<N, E> {
         Graph {
             nodes: self.nodes.clone(),