diff --git a/Cargo.toml b/Cargo.toml index af1946dd8ad..8c0b29eb1d7 100644 --- a/Cargo.toml +++ b/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. diff --git a/src/cargo/core/resolver/context.rs b/src/cargo/core/resolver/context.rs index 4854dcde7e2..db8a01613f8 100644 --- a/src/cargo/core/resolver/context.rs +++ b/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, + pub resolve_features: HashMap, /// get the package that will be linking to a native library by its links attribute - pub links: im_rc::HashMap, + pub links: HashMap, /// 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, /// 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>, + pub parents: Graph>, } /// 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; +pub type Activations = HashMap; /// 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> { +impl Graph> { pub fn parents_of(&self, p: PackageId) -> impl Iterator + '_ { 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)>, - >, + inner: HashMap)>>, } impl PublicDependency { fn new() -> Self { PublicDependency { - inner: im_rc::HashMap::new(), + inner: HashMap::new(), } } fn publicly_exports(&self, candidate_pid: PackageId) -> Vec { @@ -344,7 +341,7 @@ impl PublicDependency { parent_pid: PackageId, is_public: bool, age: ContextAge, - parents: &Graph>, + parents: &Graph>, ) { // 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>, + parents: &Graph>, ) -> Result< (), ( diff --git a/src/cargo/core/resolver/types.rs b/src/cargo/core/resolver/types.rs index 4617e3ea8e5..fbcb7df34c0 100644 --- a/src/cargo/core/resolver/types.rs +++ b/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,41 +200,37 @@ 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, } 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 @@ -242,14 +238,14 @@ impl RemainingDeps { // 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 + '_ { - self.data.iter().flat_map(|(other, _)| other.flatten()) + self.deps.iter().flat_map(|other| other.flatten()) } } diff --git a/src/cargo/util/graph.rs b/src/cargo/util/graph.rs index ff4018201fe..7df159367d7 100644 --- a/src/cargo/util/graph.rs +++ b/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 { - nodes: im_rc::OrdMap>, + nodes: Rc>>>, } impl Graph { pub fn new() -> Graph { 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(&self, k: &Q) -> bool @@ -148,7 +153,7 @@ impl fmt::Debug for Graph { 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() {