optimize conflict_store for looking up only older matches

Author: Eh2406

crates/resolver-tests/tests/resolve.rs

@@ -1333,6 +1333,25 @@ fn large_conflict_cache() {
     let _ = resolve(root_deps, &reg);
 }
 
+#[test]
+fn off_by_one_bug() {
+    let input = vec![
+        pkg!(("A-sys", "0.0.1")),
+        pkg!(("A-sys", "0.0.4")),
+        pkg!(("A-sys", "0.0.6")),
+        pkg!(("A-sys", "0.0.7")),
+        pkg!(("NA", "0.0.0") => [dep_req("A-sys", "<= 0.0.5"),]),
+        pkg!(("NA", "0.0.1") => [dep_req("A-sys", ">= 0.0.6, <= 0.0.8"),]),
+        pkg!(("a", "0.0.1")),
+        pkg!(("a", "0.0.2")),
+        pkg!(("aa", "0.0.0") => [dep_req("A-sys", ">= 0.0.4, <= 0.0.6"),dep_req("NA", "<= 0.0.0"),]),
+        pkg!(("f", "0.0.3") => [dep("NA"),dep_req("a", "<= 0.0.2"),dep("aa"),]),
+    ];
+
+    let reg = registry(input);
+    let _ = resolve_and_validated(vec![dep("f")], &reg, None);
+}
+
 #[test]
 fn conflict_store_bug() {
     let input = vec![

src/cargo/core/resolver/conflict_cache.rs

@@ -25,6 +25,7 @@ impl ConflictStoreTrie {
         &self,
         is_active: &impl Fn(PackageId) -> Option<usize>,
         must_contain: Option<PackageId>,
+        mut max_age: usize,
     ) -> Option<(&ConflictMap, usize)> {
         match self {
             ConflictStoreTrie::Leaf(c) => {
@@ -43,8 +44,12 @@ impl ConflictStoreTrie {
                 {
                     // 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))
+                            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`
@@ -53,10 +58,11 @@ impl ConflictStoreTrie {
                             } else {
                                 std::cmp::max(age_this, age_o)
                             };
-                            let out_age = out.get_or_insert((o, age)).1;
-                            if out_age > age {
+                            if max_age > age {
                                 // we found one that can jump-back further so replace the out.
                                 out = Some((o, age));
+                                // and dont look at anything older
+                                max_age = age
                             }
                         }
                     }
@@ -152,10 +158,11 @@ impl ConflictCache {
         dep: &Dependency,
         is_active: &impl Fn(PackageId) -> Option<usize>,
         must_contain: Option<PackageId>,
+        max_age: usize,
     ) -> Option<&ConflictMap> {
         self.con_from_dep
             .get(dep)?
-            .find(is_active, must_contain)
+            .find(is_active, must_contain, max_age)
             .map(|(c, _)| c)
     }
     /// Finds any known set of conflicts, if any,
@@ -168,7 +175,7 @@ impl ConflictCache {
         dep: &Dependency,
         must_contain: Option<PackageId>,
     ) -> Option<&ConflictMap> {
-        let out = self.find(dep, &|id| cx.is_active(id), must_contain);
+        let out = self.find(dep, &|id| cx.is_active(id), must_contain, std::usize::MAX);
         if cfg!(debug_assertions) {
             if let Some(c) = &out {
                 assert!(cx.is_conflicting(None, c).is_some());

src/cargo/core/resolver/mod.rs

@@ -902,12 +902,12 @@ fn generalize_conflicting(
                                     // we are imagining that we used other instead
                                     Some(backtrack_critical_age)
                                 } else {
-                                    cx.is_active(id).filter(|&age|
-                                        // we only care about things that are older then critical_age
-                                        age < backtrack_critical_age)
+                                    cx.is_active(id)
                                 }
                             },
                             Some(other.package_id()),
+                            // we only care about things that are newer then critical_age
+                            backtrack_critical_age,
                         )
                         .map(|con| (other.package_id(), con))
                 })