remove the O(n) insertion, but loos the hack for lookup

Author: Eh2406

src/cargo/core/resolver/conflict_cache.rs

@@ -1,4 +1,4 @@
-use std::collections::{HashMap, HashSet};
+use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
 
 use super::types::ConflictReason;
 use core::resolver::Context;
@@ -34,7 +34,7 @@ pub(super) struct ConflictCache {
     // as a global cache which we never delete from. Any entry in this map is
     // unconditionally true regardless of our resolution history of how we got
     // here.
-    con_from_dep: HashMap<Dependency, Vec<HashMap<PackageId, ConflictReason>>>,
+    con_from_dep: HashMap<Dependency, BTreeSet<BTreeMap<PackageId, ConflictReason>>>,
     // `dep_from_pid` is an inverse-index of `con_from_dep`.
     // For every `PackageId` this lists the `Dependency`s that mention it in `dep_from_pid`.
     dep_from_pid: HashMap<PackageId, HashSet<Dependency>>,
@@ -54,9 +54,9 @@ impl ConflictCache {
         cx: &Context,
         dep: &Dependency,
         filter: F,
-    ) -> Option<&HashMap<PackageId, ConflictReason>>
+    ) -> Option<&BTreeMap<PackageId, ConflictReason>>
     where
-        for<'r> F: FnMut(&'r &HashMap<PackageId, ConflictReason>) -> bool,
+        for<'r> F: FnMut(&'r &BTreeMap<PackageId, ConflictReason>) -> bool,
     {
         self.con_from_dep
             .get(dep)?
@@ -69,26 +69,26 @@ impl ConflictCache {
         &self,
         cx: &Context,
         dep: &Dependency,
-    ) -> Option<&HashMap<PackageId, ConflictReason>> {
+    ) -> Option<&BTreeMap<PackageId, ConflictReason>> {
         self.find_conflicting(cx, dep, |_| true)
     }
 
     /// Add to the cache a conflict of the form:
     /// `dep` is known to be unresolvable if
     /// all the `PackageId` entries are activated
-    pub fn insert(&mut self, dep: &Dependency, con: &HashMap<PackageId, ConflictReason>) {
+    pub fn insert(&mut self, dep: &Dependency, con: &BTreeMap<PackageId, ConflictReason>) {
         let past = self
             .con_from_dep
             .entry(dep.clone())
-            .or_insert_with(Vec::new);
+            .or_insert_with(BTreeSet::new);
         if !past.contains(con) {
             trace!(
                 "{} = \"{}\" adding a skip {:?}",
                 dep.package_name(),
                 dep.version_req(),
                 con
             );
-            past.push(con.clone());
+            past.insert(con.clone());
             for c in con.keys() {
                 self.dep_from_pid
                     .entry(c.clone())

src/cargo/core/resolver/context.rs

@@ -1,13 +1,13 @@
-use std::collections::{HashMap, HashSet};
+use std::collections::{BTreeMap, HashMap, HashSet};
 use std::rc::Rc;
 
 use core::interning::InternedString;
 use core::{Dependency, FeatureValue, PackageId, SourceId, Summary};
 use util::CargoResult;
 use util::Graph;
 
-use super::types::{ConflictReason, DepInfo, GraphNode, Method, RcList, RegistryQueryer};
 use super::errors::ActivateResult;
+use super::types::{ConflictReason, DepInfo, GraphNode, Method, RcList, RegistryQueryer};
 
 pub use super::encode::{EncodableDependency, EncodablePackageId, EncodableResolve};
 pub use super::encode::{Metadata, WorkspaceResolve};
@@ -145,7 +145,7 @@ impl Context {
     pub fn is_conflicting(
         &self,
         parent: Option<&PackageId>,
-        conflicting_activations: &HashMap<PackageId, ConflictReason>,
+        conflicting_activations: &BTreeMap<PackageId, ConflictReason>,
     ) -> bool {
         conflicting_activations
             .keys()
@@ -186,10 +186,11 @@ impl Context {
             // name.
             let base = reqs.deps.get(&dep.name_in_toml()).unwrap_or(&default_dep);
             used_features.insert(dep.name_in_toml());
-            let always_required = !dep.is_optional() && !s
-                .dependencies()
-                .iter()
-                .any(|d| d.is_optional() && d.name_in_toml() == dep.name_in_toml());
+            let always_required = !dep.is_optional()
+                && !s
+                    .dependencies()
+                    .iter()
+                    .any(|d| d.is_optional() && d.name_in_toml() == dep.name_in_toml());
             if always_required && base.0 {
                 self.warnings.push(format!(
                     "Package `{}` does not have feature `{}`. It has a required dependency \

src/cargo/core/resolver/errors.rs

@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use std::collections::BTreeMap;
 use std::fmt;
 
 use core::{Dependency, PackageId, Registry, Summary};
@@ -73,7 +73,7 @@ pub(super) fn activation_error(
     registry: &mut Registry,
     parent: &Summary,
     dep: &Dependency,
-    conflicting_activations: &HashMap<PackageId, ConflictReason>,
+    conflicting_activations: &BTreeMap<PackageId, ConflictReason>,
     candidates: &[Candidate],
     config: Option<&Config>,
 ) -> ResolveError {

src/cargo/core/resolver/mod.rs

@@ -247,7 +247,7 @@ fn activate_deps_loop(
         //
         // This is a map of package id to a reason why that packaged caused a
         // conflict for us.
-        let mut conflicting_activations = HashMap::new();
+        let mut conflicting_activations = BTreeMap::new();
 
         // When backtracking we don't fully update `conflicting_activations`
         // especially for the cases that we didn't make a backtrack frame in the
@@ -641,7 +641,7 @@ struct BacktrackFrame {
     parent: Summary,
     dep: Dependency,
     features: Rc<Vec<InternedString>>,
-    conflicting_activations: HashMap<PackageId, ConflictReason>,
+    conflicting_activations: BTreeMap<PackageId, ConflictReason>,
 }
 
 /// A helper "iterator" used to extract candidates within a current `Context` of
@@ -688,7 +688,7 @@ impl RemainingCandidates {
     /// original list for the reason listed.
     fn next(
         &mut self,
-        conflicting_prev_active: &mut HashMap<PackageId, ConflictReason>,
+        conflicting_prev_active: &mut BTreeMap<PackageId, ConflictReason>,
         cx: &Context,
         dep: &Dependency,
     ) -> Option<(Candidate, bool)> {
@@ -781,7 +781,7 @@ fn find_candidate(
     backtrack_stack: &mut Vec<BacktrackFrame>,
     parent: &Summary,
     backtracked: bool,
-    conflicting_activations: &HashMap<PackageId, ConflictReason>,
+    conflicting_activations: &BTreeMap<PackageId, ConflictReason>,
 ) -> Option<(Candidate, bool, BacktrackFrame)> {
     while let Some(mut frame) = backtrack_stack.pop() {
         let next = frame.remaining_candidates.next(