Faster resolver: clean code and the backtrack_stack

src/cargo/core/resolver/mod.rs

@@ -951,12 +951,9 @@ fn activate_deps_loop(
             && past_conflicting_activations
                 .get(&dep)
                 .and_then(|past_bad| {
-                    past_bad.iter().find(|conflicting| {
-                        conflicting
-                    .iter()
-                    // note: a lot of redundant work in is_active for similar debs
-                    .all(|(con, _)| cx.is_active(con))
-                    })
+                    past_bad
+                        .iter()
+                        .find(|conflicting| cx.is_conflicting(None, conflicting))
                 })
                 .is_some();
 
@@ -1091,12 +1088,9 @@ fn activate_deps_loop(
                             .filter_map(|(_, (deb, _, _))| {
                                 past_conflicting_activations.get(&deb).and_then(|past_bad| {
                                     // for each dependency check all of its cashed conflicts
-                                    past_bad.iter().find(|conflicting| {
-                                        conflicting
+                                    past_bad
                                         .iter()
-                                        // note: a lot of redundant work in is_active for similar debs
-                                        .all(|(con, _)| cx.is_active(con))
-                                    })
+                                        .find(|conflicting| cx.is_conflicting(None, conflicting))
                                 })
                             })
                             .next()
@@ -1123,6 +1117,14 @@ fn activate_deps_loop(
                                 conflicting_activations
                             );
                             past.push(conflicting_activations.clone());
+                            // also clean the `backtrack_stack` so we don't
+                            // backtrack to a place where we will try this again.
+                            backtrack_stack.retain(|bframe| {
+                                !bframe.context_backup.is_conflicting(
+                                    Some(parent.package_id()),
+                                    &conflicting_activations,
+                                )
+                            });
                         }
                     }
                     // if not has_another we we activate for the better error messages
@@ -1195,11 +1197,9 @@ fn find_candidate<'a>(
             frame.context_backup.prev_active(&frame.dep),
             &frame.context_backup.links,
         );
-        if frame.context_backup.is_active(parent.package_id())
-            && conflicting_activations
-           .iter()
-           // note: a lot of redundant work in is_active for similar debs
-           .all(|(con, _)| frame.context_backup.is_active(con))
+        if frame
+            .context_backup
+            .is_conflicting(Some(parent.package_id()), conflicting_activations)
         {
             continue;
         }
@@ -1643,6 +1643,20 @@ impl Context {
             .unwrap_or(false)
     }
 
+    /// checks whether all of `parent` and the keys of `conflicting activations`
+    /// are still active
+    fn is_conflicting(
+        &self,
+        parent: Option<&PackageId>,
+        conflicting_activations: &HashMap<PackageId, ConflictReason>,
+    ) -> bool {
+        parent.map(|p| self.is_active(p)).unwrap_or(true)
+            && conflicting_activations
+                .keys()
+                // note: a lot of redundant work in is_active for similar debs
+                .all(|con| self.is_active(con))
+    }
+
     /// Return all dependencies and the features we want from them.
     fn resolve_features<'b>(
         &mut self,

tests/testsuite/resolve.rs

@@ -608,6 +608,49 @@ fn resolving_with_many_equivalent_backtracking() {
     assert!(res.is_err());
 }
 
+#[test]
+fn resolving_with_deep_traps() {
+    let mut reglist = Vec::new();
+
+    const DEPTH: usize = 200;
+    const BRANCHING_FACTOR: usize = 100;
+
+    // Each backtrack_trap depends on the next, and adds a backtrack frame.
+    // None of witch is going to help with `bad`.
+    for l in 0..DEPTH {
+        let name = format!("backtrack_trap{}", l);
+        let next = format!("backtrack_trap{}", l + 1);
+        for i in 1..BRANCHING_FACTOR {
+            let vsn = format!("1.0.{}", i);
+            reglist.push(pkg!((name.as_str(), vsn.as_str()) => [dep_req(next.as_str(), "*")]));
+        }
+    }
+    {
+        let name = format!("backtrack_trap{}", DEPTH);
+        for i in 1..BRANCHING_FACTOR {
+            let vsn = format!("1.0.{}", i);
+            reglist.push(pkg!((name.as_str(), vsn.as_str())));
+        }
+    }
+    {
+        // slightly less constrained to make sure `cloaking` gets picked last.
+        for i in 1..(BRANCHING_FACTOR + 10) {
+            let vsn = format!("1.0.{}", i);
+            reglist.push(pkg!(("cloaking", vsn.as_str()) => [dep_req("bad", "1.0.1")]));
+        }
+    }
+
+    let reg = registry(reglist.clone());
+
+    let res = resolve(
+        &pkg_id("root"),
+        vec![dep_req("backtrack_trap0", "*"), dep_req("cloaking", "*")],
+        &reg,
+    );
+
+    assert!(res.is_err());
+}
+
 #[test]
 fn resolving_with_constrained_sibling_backtrack_activation() {
     // It makes sense to resolve most-constrained deps first, but