Faster conflict cache

src/cargo/core/resolver/conflict_cache.rs

@@ -3,7 +3,7 @@ use std::collections::{BTreeMap, HashMap};
 use rustc_hash::{FxHashMap, FxHashSet};
 use tracing::trace;
 
-use super::types::ConflictMap;
+use super::types::{ActivationsKey, ConflictMap};
 use crate::core::resolver::ResolverContext;
 use crate::core::{Dependency, PackageId};
 
@@ -14,7 +14,12 @@ enum ConflictStoreTrie {
     Leaf(ConflictMap),
     /// A map from an element to a subtrie where
     /// all the sets in the subtrie contains that element.
-    Node(BTreeMap<PackageId, ConflictStoreTrie>),
+    Node(
+        BTreeMap<
+            ActivationsKey,
+            HashMap<&'static semver::Version, ConflictStoreTrie, rustc_hash::FxBuildHasher>,
+        >,
+    ),
 }
 
 impl ConflictStoreTrie {
@@ -24,7 +29,7 @@ impl ConflictStoreTrie {
     /// one that will allow for the most jump-back.
     fn find(
         &self,
-        is_active: &impl Fn(PackageId) -> Option<usize>,
+        in_activation_slot: &impl Fn(&ActivationsKey) -> Option<(PackageId, usize)>,
         must_contain: Option<PackageId>,
         mut max_age: usize,
     ) -> Option<(&ConflictMap, usize)> {
@@ -39,36 +44,45 @@ impl ConflictStoreTrie {
             }
             ConflictStoreTrie::Node(m) => {
                 let mut out = None;
-                for (&pid, store) in must_contain
-                    .map(|f| m.range(..=f))
+                for (activations_key, map) in must_contain
+                    .map(|f| m.range(..=(f.as_activations_key())))
                     .unwrap_or_else(|| m.range(..))
                 {
-                    // If the key is active, then we need to check all of the corresponding subtrie.
-                    if let Some(age_this) = is_active(pid) {
-                        if age_this >= max_age && must_contain != Some(pid) {
-                            // not worth looking at, it is to old.
-                            continue;
-                        }
-                        if let Some((o, age_o)) =
-                            store.find(is_active, must_contain.filter(|&f| f != pid), max_age)
-                        {
-                            let age = if must_contain == Some(pid) {
-                                // all the results will include `must_contain`
-                                // so the age of must_contain is not relevant to find the best result.
-                                age_o
-                            } else {
-                                std::cmp::max(age_this, age_o)
-                            };
-                            if max_age > age {
-                                // we found one that can jump-back further so replace the out.
-                                out = Some((o, age));
-                                // and don't look at anything older
-                                max_age = age
-                            }
-                        }
+                    // Find the package that is active for this activation key.
+                    let Some((pid, age_this)) = in_activation_slot(&activations_key) else {
+                        // Else, if it is not active then there is no way any of the corresponding
+                        // subtries will be conflicting.
+                        continue;
+                    };
+                    if age_this >= max_age && must_contain != Some(pid) {
+                        // not worth looking at, it is to old.
+                        continue;
+                    }
+                    // If the active package has a stored conflict ...
+                    let Some(store) = map.get(pid.version()) else {
+                        continue;
+                    };
+                    // then we need to check the corresponding subtrie.
+                    let Some((o, age_o)) = store.find(
+                        in_activation_slot,
+                        must_contain.filter(|&f| f != pid),
+                        max_age,
+                    ) else {
+                        continue;
+                    };
+                    let age = if must_contain == Some(pid) {
+                        // all the results will include `must_contain`
+                        // so the age of must_contain is not relevant to find the best result.
+                        age_o
+                    } else {
+                        std::cmp::max(age_this, age_o)
+                    };
+                    if max_age > age {
+                        // we found one that can jump-back further so replace the out.
+                        out = Some((o, age));
+                        // and don't look at anything older
+                        max_age = age
                     }
-                    // Else, if it is not active then there is no way any of the corresponding
-                    // subtrie will be conflicting.
                 }
                 out
             }
@@ -78,7 +92,9 @@ impl ConflictStoreTrie {
     fn insert(&mut self, mut iter: impl Iterator<Item = PackageId>, con: ConflictMap) {
         if let Some(pid) = iter.next() {
             if let ConflictStoreTrie::Node(p) = self {
-                p.entry(pid)
+                p.entry(pid.as_activations_key().clone())
+                    .or_default()
+                    .entry(pid.version())
                     .or_insert_with(|| ConflictStoreTrie::Node(BTreeMap::new()))
                     .insert(iter, con);
             }
@@ -157,13 +173,13 @@ impl ConflictCache {
     pub fn find(
         &self,
         dep: &Dependency,
-        is_active: &impl Fn(PackageId) -> Option<usize>,
+        in_activation_slot: &impl Fn(&ActivationsKey) -> Option<(PackageId, usize)>,
         must_contain: Option<PackageId>,
         max_age: usize,
     ) -> Option<&ConflictMap> {
         self.con_from_dep
             .get(dep)?
-            .find(is_active, must_contain, max_age)
+            .find(in_activation_slot, must_contain, max_age)
             .map(|(c, _)| c)
     }
     /// Finds any known set of conflicts, if any,
@@ -176,7 +192,12 @@ impl ConflictCache {
         dep: &Dependency,
         must_contain: Option<PackageId>,
     ) -> Option<&ConflictMap> {
-        let out = self.find(dep, &|id| cx.is_active(id), must_contain, usize::MAX);
+        let out = self.find(
+            dep,
+            &|id| cx.in_activation_slot(id),
+            must_contain,
+            usize::MAX,
+        );
         if cfg!(debug_assertions) {
             if let Some(c) = &out {
                 assert!(cx.is_conflicting(None, c).is_some());
@@ -195,6 +216,13 @@ impl ConflictCache {
     /// `dep` is known to be unresolvable if
     /// all the `PackageId` entries are activated.
     pub fn insert(&mut self, dep: &Dependency, con: &ConflictMap) {
+        if cfg!(debug_assertions) {
+            // Check that the iterator is sorted by activation key.
+            // `ConflictMap` is a `BTreeMap` so it is already sorted.
+            // But, if the ord on `ActivationsKey` changes to not match ord on `PackageId`, this will fail.
+            assert!(con.keys().is_sorted_by_key(|p| p.as_activations_key()))
+        }
+
         self.con_from_dep
             .entry(dep.clone())
             .or_insert_with(|| ConflictStoreTrie::Node(BTreeMap::new()))

src/cargo/core/resolver/context.rs

@@ -156,6 +156,12 @@ impl ResolverContext {
             .and_then(|(s, l)| if s.package_id() == id { Some(*l) } else { None })
     }
 
+    /// If a package is active that has the same semver compatibility range
+    /// returns the `PackageId` and `ContextAge` when it was added
+    pub fn in_activation_slot(&self, id: &ActivationsKey) -> Option<(PackageId, ContextAge)> {
+        self.activations.get(id).map(|(s, l)| (s.package_id(), *l))
+    }
+
     /// Checks whether all of `parent` and the keys of `conflicting activations`
     /// are still active.
     /// If so returns the `ContextAge` when the newest one was added.

src/cargo/core/resolver/mod.rs

@@ -857,15 +857,16 @@ fn generalize_conflicting(
                 .iter()
                 .rev() // the last one to be tried is the least likely to be in the cache, so start with that.
                 .map(|other| {
+                    let other_activations_key = other.package_id().as_activations_key();
                     past_conflicting_activations
                         .find(
                             dep,
                             &|id| {
-                                if id == other.package_id() {
+                                if id == &other_activations_key {
                                     // we are imagining that we used other instead
-                                    Some(backtrack_critical_age)
+                                    Some((other.package_id(), backtrack_critical_age))
                                 } else {
-                                    cx.is_active(id)
+                                    cx.in_activation_slot(id)
                                 }
                             },
                             Some(other.package_id()),

src/cargo/core/resolver/types.rs

@@ -193,9 +193,9 @@ impl std::hash::Hash for ActivationsKey {
 /// same.
 #[derive(Clone, Copy, Eq, PartialEq, Hash, Debug, PartialOrd, Ord)]
 pub enum SemverCompatibility {
-    Major(NonZeroU64),
-    Minor(NonZeroU64),
     Patch(u64),
+    Minor(NonZeroU64),
+    Major(NonZeroU64),
 }
 
 impl From<&semver::Version> for SemverCompatibility {