Drop the im-rc dependency

Cargo.toml

@@ -66,7 +66,6 @@ walkdir = "2.2"
 clap = "2.31.2"
 unicode-width = "0.1.5"
 openssl = { version = '0.10.11', optional = true }
-im-rc = "15.0.0"
 itertools = "0.10.0"
 
 # A noop dependency that changes in the Rust repository, it's a bit of a hack.

src/cargo/core/resolver/context.rs

@@ -7,7 +7,7 @@ use crate::util::interning::InternedString;
 use crate::util::Graph;
 use anyhow::format_err;
 use log::debug;
-use std::collections::HashMap;
+use std::collections::{hash_map::Entry, HashMap, HashSet};
 use std::num::NonZeroU64;
 
 pub use super::encode::Metadata;
@@ -23,16 +23,16 @@ pub struct Context {
     pub age: ContextAge,
     pub activations: Activations,
     /// list the features that are activated for each package
-    pub resolve_features: im_rc::HashMap<PackageId, FeaturesSet>,
+    pub resolve_features: HashMap<PackageId, FeaturesSet>,
     /// get the package that will be linking to a native library by its links attribute
-    pub links: im_rc::HashMap<InternedString, PackageId>,
+    pub links: HashMap<InternedString, PackageId>,
     /// for each package the list of names it can see,
     /// then for each name the exact version that name represents and whether the name is public.
     pub public_dependency: Option<PublicDependency>,
 
     /// 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, im_rc::HashSet<Dependency>>,
+    pub parents: Graph<PackageId, HashSet<Dependency>>,
 }
 
 /// When backtracking it can be useful to know how far back to go.
@@ -47,7 +47,7 @@ pub type ContextAge = usize;
 /// semver compatible version of each crate.
 /// This all so stores the `ContextAge`.
 pub type ActivationsKey = (InternedString, SourceId, SemverCompatibility);
-pub type Activations = im_rc::HashMap<ActivationsKey, (Summary, ContextAge)>;
+pub type Activations = HashMap<ActivationsKey, (Summary, ContextAge)>;
 
 /// A type that represents when cargo treats two Versions as compatible.
 /// Versions `a` and `b` are compatible if their left-most nonzero digit is the
@@ -81,15 +81,15 @@ impl Context {
     pub fn new(check_public_visible_dependencies: bool) -> Context {
         Context {
             age: 0,
-            resolve_features: im_rc::HashMap::new(),
-            links: im_rc::HashMap::new(),
+            resolve_features: HashMap::new(),
+            links: HashMap::new(),
             public_dependency: if check_public_visible_dependencies {
                 Some(PublicDependency::new())
             } else {
                 None
             },
             parents: Graph::new(),
-            activations: im_rc::HashMap::new(),
+            activations: HashMap::new(),
         }
     }
 
@@ -109,14 +109,14 @@ impl Context {
         let id = summary.package_id();
         let age: ContextAge = self.age;
         match self.activations.entry(id.as_activations_key()) {
-            im_rc::hashmap::Entry::Occupied(o) => {
+            Entry::Occupied(o) => {
                 debug_assert_eq!(
                     &o.get().0,
                     summary,
                     "cargo does not allow two semver compatible versions"
                 );
             }
-            im_rc::hashmap::Entry::Vacant(v) => {
+            Entry::Vacant(v) => {
                 if let Some(link) = summary.links() {
                     if self.links.insert(link, id).is_some() {
                         return Err(format_err!(
@@ -278,7 +278,7 @@ impl Context {
     }
 }
 
-impl Graph<PackageId, im_rc::HashSet<Dependency>> {
+impl Graph<PackageId, 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())))
@@ -291,16 +291,13 @@ pub struct PublicDependency {
     /// for each name the exact package that name resolves to,
     ///     the `ContextAge` when it was first visible,
     ///     and the `ContextAge` when it was first exported.
-    inner: im_rc::HashMap<
-        PackageId,
-        im_rc::HashMap<InternedString, (PackageId, ContextAge, Option<ContextAge>)>,
-    >,
+    inner: HashMap<PackageId, HashMap<InternedString, (PackageId, ContextAge, Option<ContextAge>)>>,
 }
 
 impl PublicDependency {
     fn new() -> Self {
         PublicDependency {
-            inner: im_rc::HashMap::new(),
+            inner: HashMap::new(),
         }
     }
     fn publicly_exports(&self, candidate_pid: PackageId) -> Vec<PackageId> {
@@ -344,7 +341,7 @@ impl PublicDependency {
         parent_pid: PackageId,
         is_public: bool,
         age: ContextAge,
-        parents: &Graph<PackageId, im_rc::HashSet<Dependency>>,
+        parents: &Graph<PackageId, 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
@@ -355,7 +352,7 @@ impl PublicDependency {
             let mut stack = vec![(parent_pid, is_public)];
             while let Some((p, public)) = stack.pop() {
                 match self.inner.entry(p).or_default().entry(c.name()) {
-                    im_rc::hashmap::Entry::Occupied(mut o) => {
+                    Entry::Occupied(mut o) => {
                         // the (transitive) parent can already see something by `c`s name, it had better be `c`.
                         assert_eq!(o.get().0, c);
                         if o.get().2.is_some() {
@@ -370,7 +367,7 @@ impl PublicDependency {
                             o.insert((c, old_age, if public { Some(age) } else { None }));
                         }
                     }
-                    im_rc::hashmap::Entry::Vacant(v) => {
+                    Entry::Vacant(v) => {
                         // The (transitive) parent does not have anything by `c`s name,
                         // so we add `c`.
                         v.insert((c, age, if public { Some(age) } else { None }));
@@ -389,7 +386,7 @@ impl PublicDependency {
         b_id: PackageId,
         parent: PackageId,
         is_public: bool,
-        parents: &Graph<PackageId, im_rc::HashSet<Dependency>>,
+        parents: &Graph<PackageId, HashSet<Dependency>>,
     ) -> Result<
         (),
         (

src/cargo/core/resolver/types.rs

@@ -4,7 +4,7 @@ use crate::util::errors::CargoResult;
 use crate::util::interning::InternedString;
 use crate::util::Config;
 use std::cmp::Ordering;
-use std::collections::{BTreeMap, BTreeSet};
+use std::collections::{BTreeMap, BTreeSet, BinaryHeap};
 use std::ops::Range;
 use std::rc::Rc;
 use std::time::{Duration, Instant};
@@ -91,7 +91,7 @@ impl ResolverProgress {
 /// This is sorted so that it impls Hash, and owns its contents,
 /// needed so it can be part of the key for caching in the `DepsCache`.
 /// It is also cloned often as part of `Context`, hence the `RC`.
-/// `im-rs::OrdSet` was slower of small sets like this,
+/// `im_rc::OrdSet` was slower of small sets like this,
 /// but this can change with improvements to std, im, or llvm.
 /// Using a consistent type for this allows us to use the highly
 /// optimized comparison operators like `is_subset` at the interfaces.
@@ -200,56 +200,52 @@ impl Ord for DepsFrame {
     fn cmp(&self, other: &DepsFrame) -> Ordering {
         self.just_for_error_messages
             .cmp(&other.just_for_error_messages)
-            .reverse()
-            .then_with(|| self.min_candidates().cmp(&other.min_candidates()))
+            .then_with(||
+            // the frame with the sibling that has the least number of candidates
+            // needs to get bubbled up to the top of the heap we use below, so
+            // reverse comparison here.
+            self.min_candidates().cmp(&other.min_candidates()).reverse())
     }
 }
 
-/// Note that an `OrdSet` is used for the remaining dependencies that need
-/// activation. This set is sorted by how many candidates each dependency has.
+/// Note that a `BinaryHeap` is used for the remaining dependencies that need
+/// activation. This heap is sorted such that the "largest value" is the most
+/// constrained dependency, or the one with the least candidates.
 ///
 /// This helps us get through super constrained portions of the dependency
 /// graph quickly and hopefully lock down what later larger dependencies can
 /// use (those with more candidates).
 #[derive(Clone)]
 pub struct RemainingDeps {
-    /// a monotonic counter, increased for each new insertion.
-    time: u32,
-    /// the data is augmented by the insertion time.
-    /// This insures that no two items will cmp eq.
-    /// Forcing the OrdSet into a multi set.
-    data: im_rc::OrdSet<(DepsFrame, u32)>,
+    deps: BinaryHeap<DepsFrame>,
 }
 
 impl RemainingDeps {
     pub fn new() -> RemainingDeps {
         RemainingDeps {
-            time: 0,
-            data: im_rc::OrdSet::new(),
+            deps: BinaryHeap::new(),
         }
     }
     pub fn push(&mut self, x: DepsFrame) {
-        let insertion_time = self.time;
-        self.data.insert((x, insertion_time));
-        self.time += 1;
+        self.deps.push(x);
     }
     pub fn pop_most_constrained(&mut self) -> Option<(bool, (Summary, DepInfo))> {
-        while let Some((mut deps_frame, insertion_time)) = self.data.remove_min() {
+        while let Some(mut deps_frame) = self.deps.pop() {
             let just_here_for_the_error_messages = deps_frame.just_for_error_messages;
 
             // Figure out what our next dependency to activate is, and if nothing is
             // listed then we're entirely done with this frame (yay!) and we can
             // move on to the next frame.
             if let Some(sibling) = deps_frame.remaining_siblings.next() {
                 let parent = Summary::clone(&deps_frame.parent);
-                self.data.insert((deps_frame, insertion_time));
+                self.deps.push(deps_frame);
                 return Some((just_here_for_the_error_messages, (parent, sibling)));
             }
         }
         None
     }
     pub fn iter(&mut self) -> impl Iterator<Item = (PackageId, Dependency)> + '_ {
-        self.data.iter().flat_map(|(other, _)| other.flatten())
+        self.deps.iter().flat_map(|other| other.flatten())
     }
 }
 

src/cargo/util/graph.rs

@@ -1,28 +1,33 @@
 use std::borrow::Borrow;
-use std::collections::BTreeSet;
+use std::collections::{BTreeMap, BTreeSet};
 use std::fmt;
+use std::rc::Rc;
 
 pub struct Graph<N: Clone, E: Clone> {
-    nodes: im_rc::OrdMap<N, im_rc::OrdMap<N, E>>,
+    nodes: Rc<BTreeMap<N, Rc<BTreeMap<N, E>>>>,
 }
 
 impl<N: Eq + Ord + Clone, E: Default + Clone> Graph<N, E> {
     pub fn new() -> Graph<N, E> {
         Graph {
-            nodes: im_rc::OrdMap::new(),
+            nodes: Rc::new(BTreeMap::new()),
         }
     }
 
     pub fn add(&mut self, node: N) {
-        self.nodes.entry(node).or_insert_with(im_rc::OrdMap::new);
+        Rc::make_mut(&mut self.nodes)
+            .entry(node)
+            .or_insert(Rc::new(BTreeMap::new()));
     }
 
     pub fn link(&mut self, node: N, child: N) -> &mut E {
-        self.nodes
-            .entry(node)
-            .or_insert_with(im_rc::OrdMap::new)
-            .entry(child)
-            .or_insert_with(Default::default)
+        Rc::make_mut(
+            Rc::make_mut(&mut self.nodes)
+                .entry(node)
+                .or_insert_with(|| Rc::new(BTreeMap::new())),
+        )
+        .entry(child)
+        .or_insert_with(Default::default)
     }
 
     pub fn contains<Q: ?Sized>(&self, k: &Q) -> bool
@@ -148,7 +153,7 @@ 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 {{")?;
 
-        for (n, e) in &self.nodes {
+        for (n, e) in self.nodes.iter() {
             writeln!(fmt, "  - {}", n)?;
 
             for n in e.keys() {