pre calculate how far back to back-jump

Author: Eh2406

src/cargo/core/resolver/conflict_cache.rs

@@ -29,7 +29,7 @@ impl ConflictStoreTrie {
                 if must_contain.is_none() {
                     // `is_conflicting` checks that all the elements are active,
                     // but we have checked each one by the recursion of this function.
-                    debug_assert!(cx.is_conflicting(None, c));
+                    debug_assert!(cx.is_conflicting(None, c).is_some());
                     Some(c)
                 } else {
                     // We did not find `must_contain`, so we need to keep looking.
@@ -42,7 +42,7 @@ impl ConflictStoreTrie {
                     .unwrap_or_else(|| m.range(..))
                 {
                     // If the key is active, then we need to check all of the corresponding subtrie.
-                    if cx.is_active(pid) {
+                    if cx.is_active(pid).is_some() {
                         if let Some(o) =
                             store.find_conflicting(cx, must_contain.filter(|&f| f != pid))
                         {

src/cargo/core/resolver/context.rs

@@ -54,7 +54,10 @@ pub struct Context {
 }
 
 /// find the activated version of a crate based on the name, source, and semver compatibility
-pub type Activations = im_rc::HashMap<(InternedString, SourceId, SemverCompatibility), Summary>;
+/// This all so stores the size of `Activations` when that version was add as an "age".
+/// This is used to speed up backtracking.
+pub type Activations =
+    im_rc::HashMap<(InternedString, SourceId, SemverCompatibility), (Summary, usize)>;
 
 /// 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
@@ -107,10 +110,11 @@ impl Context {
     /// Returns `true` if this summary with the given method is already activated.
     pub fn flag_activated(&mut self, summary: &Summary, method: &Method<'_>) -> CargoResult<bool> {
         let id = summary.package_id();
+        let activations_len = self.activations.len();
         match self.activations.entry(id.as_activations_key()) {
             im_rc::hashmap::Entry::Occupied(o) => {
                 debug_assert_eq!(
-                    o.get(),
+                    &o.get().0,
                     summary,
                     "cargo does not allow two semver compatible versions"
                 );
@@ -125,7 +129,7 @@ impl Context {
                         &*link
                     );
                 }
-                v.insert(summary.clone());
+                v.insert((summary.clone(), activations_len));
                 return Ok(false);
             }
         }
@@ -183,24 +187,30 @@ impl Context {
         Ok(deps)
     }
 
-    pub fn is_active(&self, id: PackageId) -> bool {
+    /// If the package is active returns the "age" (len of activations) when it was added
+    pub fn is_active(&self, id: PackageId) -> Option<usize> {
         self.activations
             .get(&id.as_activations_key())
-            .map(|s| s.package_id() == id)
-            .unwrap_or(false)
+            .and_then(|(s, l)| if s.package_id() == id { Some(*l) } else { None })
     }
 
     /// Checks whether all of `parent` and the keys of `conflicting activations`
     /// are still active.
+    /// If so returns the "age" (len of activations) when the newest one was added.
     pub fn is_conflicting(
         &self,
         parent: Option<PackageId>,
         conflicting_activations: &ConflictMap,
-    ) -> bool {
-        conflicting_activations
-            .keys()
-            .chain(parent.as_ref())
-            .all(|&id| self.is_active(id))
+    ) -> Option<usize> {
+        let mut max = 0;
+        for &id in conflicting_activations.keys().chain(parent.as_ref()) {
+            if let Some(age) = self.is_active(id) {
+                max = std::cmp::max(max, age);
+            } else {
+                return None;
+            }
+        }
+        Some(max)
     }
 
     /// Returns all dependencies and the features we want from them.

src/cargo/core/resolver/mod.rs

@@ -137,7 +137,7 @@ pub fn resolve(
     let cx = activate_deps_loop(cx, &mut registry, summaries, config)?;
 
     let mut cksums = HashMap::new();
-    for summary in cx.activations.values() {
+    for (summary, _) in cx.activations.values() {
         let cksum = summary.checksum().map(|s| s.to_string());
         cksums.insert(summary.package_id(), cksum);
     }
@@ -302,6 +302,7 @@ fn activate_deps_loop(
                 }
 
                 match find_candidate(
+                    &cx,
                     &mut backtrack_stack,
                     &parent,
                     backtracked,
@@ -493,6 +494,7 @@ fn activate_deps_loop(
                     let activate_for_error_message = has_past_conflicting_dep && !has_another && {
                         just_here_for_the_error_messages || {
                             find_candidate(
+                                &cx,
                                 &mut backtrack_stack.clone(),
                                 &parent,
                                 backtracked,
@@ -777,7 +779,7 @@ impl RemainingCandidates {
             //
             // Here we throw out our candidate if it's *compatible*, yet not
             // equal, to all previously activated versions.
-            if let Some(a) = cx.activations.get(&b_id.as_activations_key()) {
+            if let Some((a, _)) = cx.activations.get(&b_id.as_activations_key()) {
                 if *a != b.summary {
                     conflicting_prev_active
                         .entry(a.package_id())
@@ -850,11 +852,33 @@ impl RemainingCandidates {
 /// Read <https://github.com/rust-lang/cargo/pull/4834>
 /// For several more detailed explanations of the logic here.
 fn find_candidate(
+    cx: &Context,
     backtrack_stack: &mut Vec<BacktrackFrame>,
     parent: &Summary,
     backtracked: bool,
     conflicting_activations: &ConflictMap,
 ) -> Option<(Candidate, bool, BacktrackFrame)> {
+    // When we're calling this method we know that `parent` failed to
+    // activate. That means that some dependency failed to get resolved for
+    // whatever reason. Normally, that means that all of those reasons
+    // (plus maybe some extras) are listed in `conflicting_activations`.
+    //
+    // The abnormal situations are things that do not put all of the reasons in `conflicting_activations`:
+    // If we backtracked we do not know how our `conflicting_activations` related to
+    // the cause of that backtrack, so we do not update it.
+    // If we had a PublicDependency conflict, then we do not yet have a compact way to
+    // represent all the parts of the problem, so `conflicting_activations` is incomplete.
+    let age = if !backtracked
+        && !conflicting_activations
+            .values()
+            .any(|c| *c == ConflictReason::PublicDependency)
+    {
+        // we dont have abnormal situations. So we can ask `cx` for how far back we need to go.
+        cx.is_conflicting(Some(parent.package_id()), conflicting_activations)
+    } else {
+        None
+    };
+
     while let Some(mut frame) = backtrack_stack.pop() {
         let next = frame.remaining_candidates.next(
             &mut frame.conflicting_activations,
@@ -866,37 +890,37 @@ fn find_candidate(
             Some(pair) => pair,
             None => continue,
         };
-        // When we're calling this method we know that `parent` failed to
-        // activate. That means that some dependency failed to get resolved for
-        // whatever reason. Normally, that means that all of those reasons
-        // (plus maybe some extras) are listed in `conflicting_activations`.
-        //
-        // This means that if all members of `conflicting_activations` are still
+
+        // If all members of `conflicting_activations` are still
         // active in this back up we know that we're guaranteed to not actually
         // make any progress. As a result if we hit this condition we can
         // completely skip this backtrack frame and move on to the next.
-        //
-        // The abnormal situations are things that do not put all of the reasons in `conflicting_activations`:
-        // If we backtracked we do not know how our `conflicting_activations` related to
-        // the cause of that backtrack, so we do not update it.
-        // If we had a PublicDependency conflict, then we do not yet have a compact way to
-        // represent all the parts of the problem, so `conflicting_activations` is incomplete.
-        if !backtracked
-            && !conflicting_activations
-                .values()
-                .any(|c| *c == ConflictReason::PublicDependency)
-            && frame
-                .context
-                .is_conflicting(Some(parent.package_id()), conflicting_activations)
-        {
-            trace!(
-                "{} = \"{}\" skip as not solving {}: {:?}",
-                frame.dep.package_name(),
-                frame.dep.version_req(),
-                parent.package_id(),
-                conflicting_activations
-            );
-            continue;
+        if let Some(age) = age {
+            if frame.context.activations.len() > age {
+                trace!(
+                    "{} = \"{}\" skip as not solving {}: {:?}",
+                    frame.dep.package_name(),
+                    frame.dep.version_req(),
+                    parent.package_id(),
+                    conflicting_activations
+                );
+                // above we use `cx` to determine that this is still going to be conflicting.
+                // but lets just double check.
+                debug_assert!(
+                    frame
+                        .context
+                        .is_conflicting(Some(parent.package_id()), conflicting_activations)
+                        == Some(age)
+                );
+                continue;
+            } else {
+                // above we use `cx` to determine that this is not going to be conflicting.
+                // but lets just double check.
+                debug_assert!(frame
+                    .context
+                    .is_conflicting(Some(parent.package_id()), conflicting_activations)
+                    .is_none());
+            }
         }
 
         return Some((candidate, has_another, frame));
@@ -905,8 +929,10 @@ fn find_candidate(
 }
 
 fn check_cycles(resolve: &Resolve, activations: &Activations) -> CargoResult<()> {
-    let summaries: HashMap<PackageId, &Summary> =
-        activations.values().map(|s| (s.package_id(), s)).collect();
+    let summaries: HashMap<PackageId, &Summary> = activations
+        .values()
+        .map(|(s, _)| (s.package_id(), s))
+        .collect();
 
     // Sort packages to produce user friendly deterministic errors.
     let mut all_packages: Vec<_> = resolve.iter().collect();