count rejected decision and steps in conflict resolution

pubgrub-rs · Dec 18, 2024 · 6502125 · 6502125
1 parent 1044406
commit 6502125
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 51 deletions.
diff --git a/src/internal/core.rs b/src/internal/core.rs
@@ -208,34 +208,38 @@ impl<DP: DependencyProvider> State<DP> {
                 .is_terminal(self.root_package, &self.root_version)
             {
                 return Err(current_incompat_id);
-            } else {
-                let (package, satisfier_search_result) = self.partial_solution.satisfier_search(
-                    &self.incompatibility_store[current_incompat_id],
-                    &self.incompatibility_store,
-                );
-                match satisfier_search_result {
-                    SatisfierSearch::DifferentDecisionLevels {
+            }
+            let (package, satisfier_search_result) = self.partial_solution.satisfier_search(
+                &self.incompatibility_store[current_incompat_id],
+                &self.incompatibility_store,
+            );
+            match satisfier_search_result {
+                SatisfierSearch::DifferentDecisionLevels {
+                    previous_satisfier_level,
+                } => {
+                    self.backtrack(
+                        current_incompat_id,
+                        current_incompat_changed,
                         previous_satisfier_level,
-                    } => {
-                        self.backtrack(
-                            current_incompat_id,
-                            current_incompat_changed,
-                            previous_satisfier_level,
-                        );
-                        log::info!("backtrack to {:?}", previous_satisfier_level);
-                        return Ok((package, current_incompat_id));
-                    }
-                    SatisfierSearch::SameDecisionLevels { satisfier_cause } => {
-                        let prior_cause = Incompatibility::prior_cause(
-                            current_incompat_id,
-                            satisfier_cause,
-                            package,
-                            &self.incompatibility_store,
-                        );
-                        log::info!("prior cause: {}", prior_cause.display(&self.package_store));
-                        current_incompat_id = self.incompatibility_store.alloc(prior_cause);
-                        current_incompat_changed = true;
+                    );
+                    log::info!("backtrack to {:?}", previous_satisfier_level);
+                    return Ok((package, current_incompat_id));
+                }
+                SatisfierSearch::SameDecisionLevels { satisfier_cause } => {
+                    let prior_cause = Incompatibility::prior_cause(
+                        current_incompat_id,
+                        satisfier_cause,
+                        package,
+                        &self.incompatibility_store,
+                    );
+
+                    for (p, _) in prior_cause.iter() {
+                        *self.conflict_count.entry(p).or_default() += 1;
                     }
+
+                    log::info!("prior cause: {}", prior_cause.display(&self.package_store));
+                    current_incompat_id = self.incompatibility_store.alloc(prior_cause);
+                    current_incompat_changed = true;
                 }
             }
         }

diff --git a/src/internal/partial_solution.rs b/src/internal/partial_solution.rs
@@ -408,34 +408,39 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
         version: DP::V,
         new_incompatibilities: std::ops::Range<IncompId<DP::P, DP::VS, DP::M>>,
         store: &Arena<Incompatibility<DP::P, DP::VS, DP::M>>,
-    ) {
+    ) -> Option<IncompId<DP::P, DP::VS, DP::M>> {
         if !self.has_ever_backtracked {
-            // Nothing has yet gone wrong during this resolution. This call is unlikely to be the first problem.
+            // Fast path: Nothing has yet gone wrong during this resolution. This call is unlikely to be the first problem.
             // So let's live with a little bit of risk and add the decision without checking the dependencies.
             // The worst that can happen is we will have to do a full backtrack which only removes this one decision.
             log::info!("add_decision: {package:?} @ {version} without checking dependencies");
             self.add_decision(package, version);
+            return None;
+        }
+
+        // Check if any of the dependencies preclude deciding on this crate version.
+        let package_term = Term::exact(version.clone());
+        let relation = |incompat: IncompId<DP::P, DP::VS, DP::M>| {
+            store[incompat].relation(|p| {
+                // The current package isn't part of the package assignments yet.
+                if p == package {
+                    Some(&package_term)
+                } else {
+                    self.term_intersection_for_package(p)
+                }
+            })
+        };
+        if let Some(satisfied) = Id::range_to_iter(new_incompatibilities)
+            .find(|incompat| relation(*incompat) == Relation::Satisfied)
+        {
+            log::info!(
+                "rejecting decision {package:?} @ {version} because its dependencies conflict"
+            );
+            Some(satisfied)
         } else {
-            // Check if any of the new dependencies preclude deciding on this crate version.
-            let exact = Term::exact(version.clone());
-            let not_satisfied = |incompat: &Incompatibility<DP::P, DP::VS, DP::M>| {
-                incompat.relation(|p| {
-                    if p == package {
-                        Some(&exact)
-                    } else {
-                        self.term_intersection_for_package(p)
-                    }
-                }) != Relation::Satisfied
-            };
-
-            // Check none of the dependencies (new_incompatibilities)
-            // would create a conflict (be satisfied).
-            if store[new_incompatibilities].iter().all(not_satisfied) {
-                log::info!("add_decision: {package:?} @ {version}");
-                self.add_decision(package, version);
-            } else {
-                log::info!("not adding {package:?} @ {version} because of its dependencies",);
-            }
+            log::info!("adding decision: {package:?} @ {version}");
+            self.add_decision(package, version);
+            None
         }
     }
 

diff --git a/src/solver.rs b/src/solver.rs
@@ -175,9 +175,16 @@ pub fn resolve<DP: DependencyProvider>(
             let dep_incompats =
                 state.add_incompatibility_from_dependencies(p, v.clone(), dependencies);
 
-            state
-                .partial_solution
-                .add_version(p, v, dep_incompats, &state.incompatibility_store);
+            if let Some(conflict) = state.partial_solution.add_version(
+                p,
+                v,
+                dep_incompats,
+                &state.incompatibility_store,
+            ) {
+                for (incompat_package, _) in state.incompatibility_store[conflict].iter() {
+                    *state.conflict_count.entry(incompat_package).or_default() += 1;
+                }
+            }
         } else {
             // `dep_incompats` are already in `incompatibilities` so we know there are not satisfied
             // terms and can add the decision directly.