Return and track affected and culprit on conflicts

examples/caching_dependency_provider.rs

@@ -2,7 +2,10 @@
 
 use std::cell::RefCell;
 
-use pubgrub::{resolve, Dependencies, DependencyProvider, OfflineDependencyProvider, Ranges};
+use pubgrub::{
+    resolve, Dependencies, DependencyProvider, OfflineDependencyProvider,
+    PackageResolutionStatistics, Ranges,
+};
 
 type NumVS = Ranges<u32>;
 
@@ -57,8 +60,14 @@ impl<DP: DependencyProvider<M = String>> DependencyProvider for CachingDependenc
 
     type Priority = DP::Priority;
 
-    fn prioritize(&self, package: &DP::P, ranges: &DP::VS) -> Self::Priority {
-        self.remote_dependencies.prioritize(package, ranges)
+    fn prioritize(
+        &self,
+        package: &Self::P,
+        range: &Self::VS,
+        package_statistics: &PackageResolutionStatistics,
+    ) -> Self::Priority {
+        self.remote_dependencies
+            .prioritize(package, range, package_statistics)
     }
 
     type Err = DP::Err;

src/internal/core.rs

@@ -7,8 +7,8 @@ use std::collections::HashSet as Set;
 use std::sync::Arc;
 
 use crate::internal::{
-    Arena, DecisionLevel, HashArena, Id, IncompDpId, Incompatibility, PartialSolution, Relation,
-    SatisfierSearch, SmallVec,
+    Arena, DecisionLevel, HashArena, Id, IncompDpId, IncompId, Incompatibility, PartialSolution,
+    Relation, SatisfierSearch, SmallVec,
 };
 use crate::{DependencyProvider, DerivationTree, Map, NoSolutionError, VersionSet};
 
@@ -79,7 +79,7 @@ impl<DP: DependencyProvider> State<DP> {
         package: Id<DP::P>,
         version: DP::V,
         dependencies: impl IntoIterator<Item = (DP::P, DP::VS)>,
-    ) {
+    ) -> Option<IncompId<DP::P, DP::VS, DP::M>> {
         let dep_incompats =
             self.add_incompatibility_from_dependencies(package, version.clone(), dependencies);
         self.partial_solution.add_package_version_dependencies(
@@ -124,8 +124,16 @@ impl<DP: DependencyProvider> State<DP> {
 
     /// Unit propagation is the core mechanism of the solving algorithm.
     /// CF <https://github.com/dart-lang/pub/blob/master/doc/solver.md#unit-propagation>
+    ///
+    /// For each package with a satisfied incompatibility, returns the package and the root cause
+    /// incompatibility.
     #[cold]
-    pub fn unit_propagation(&mut self, package: Id<DP::P>) -> Result<(), NoSolutionError<DP>> {
+    #[allow(clippy::type_complexity)] // Type definitions don't support impl trait.
+    pub fn unit_propagation(
+        &mut self,
+        package: Id<DP::P>,
+    ) -> Result<Vec<(Id<DP::P>, IncompDpId<DP>)>, NoSolutionError<DP>> {
+        let mut root_causes = Vec::new();
         self.unit_propagation_buffer.clear();
         self.unit_propagation_buffer.push(package);
         while let Some(current_package) = self.unit_propagation_buffer.pop() {
@@ -183,6 +191,7 @@ impl<DP: DependencyProvider> State<DP> {
                         .map_err(|terminal_incompat_id| {
                             self.build_derivation_tree(terminal_incompat_id)
                         })?;
+                root_causes.push((package, root_cause));
                 self.unit_propagation_buffer.clear();
                 self.unit_propagation_buffer.push(package_almost);
                 // Add to the partial solution with incompat as cause.
@@ -198,7 +207,7 @@ impl<DP: DependencyProvider> State<DP> {
             }
         }
         // If there are no more changed packages, unit propagation is done.
-        Ok(())
+        Ok(root_causes)
     }
 
     /// Return the root cause or the terminal incompatibility.

src/internal/partial_solution.rs

@@ -295,7 +295,7 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
     #[cold]
     pub fn pick_highest_priority_pkg(
         &mut self,
-        prioritizer: impl Fn(Id<DP::P>, &DP::VS) -> DP::Priority,
+        mut prioritizer: impl FnMut(Id<DP::P>, &DP::VS) -> DP::Priority,
     ) -> Option<Id<DP::P>> {
         let check_all = self.changed_this_decision_level
             == self.current_decision_level.0.saturating_sub(1) as usize;
@@ -331,7 +331,21 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
             .map(|(&p, pa)| match &pa.assignments_intersection {
                 AssignmentsIntersection::Decision((_, v, _)) => (p, v.clone()),
                 AssignmentsIntersection::Derivations(_) => {
-                    panic!("Derivations in the Decision part")
+                    let mut context = String::new();
+                    for (id, assignment) in self
+                        .package_assignments
+                        .iter()
+                        .take(self.current_decision_level.0 as usize)
+                    {
+                        context.push_str(&format!(
+                            " * {:?} {:?}\n",
+                            id, assignment.assignments_intersection
+                        ));
+                    }
+                    panic!(
+                        "Derivations in the Decision part. Decision level {}\n{}",
+                        self.current_decision_level.0, context
+                    )
                 }
             })
     }
@@ -414,34 +428,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
         }
     }
 

src/lib.rs

@@ -71,7 +71,7 @@
 //! and [SemanticVersion] for versions.
 //! This may be done quite easily by implementing the three following functions.
 //! ```
-//! # use pubgrub::{DependencyProvider, Dependencies, SemanticVersion, Ranges, DependencyConstraints, Map};
+//! # use pubgrub::{DependencyProvider, Dependencies, SemanticVersion, Ranges, DependencyConstraints, Map, PackageResolutionStatistics};
 //! # use std::error::Error;
 //! # use std::borrow::Borrow;
 //! # use std::convert::Infallible;
@@ -86,7 +86,7 @@
 //!     }
 //!
 //!     type Priority = usize;
-//!     fn prioritize(&self, package: &String, range: &SemVS) -> Self::Priority {
+//!     fn prioritize(&self, package: &String, range: &SemVS, conflicts_counts: &PackageResolutionStatistics) -> Self::Priority {
 //!         unimplemented!()
 //!     }
 //!
@@ -227,7 +227,7 @@ pub use report::{
     DefaultStringReportFormatter, DefaultStringReporter, DerivationTree, Derived, External,
     ReportFormatter, Reporter,
 };
-pub use solver::{resolve, Dependencies, DependencyProvider};
+pub use solver::{resolve, Dependencies, DependencyProvider, PackageResolutionStatistics};
 pub use term::Term;
 pub use type_aliases::{DependencyConstraints, Map, SelectedDependencies, Set};
 pub use version::{SemanticVersion, VersionParseError};

src/provider.rs

@@ -2,6 +2,7 @@ use std::cmp::Reverse;
 use std::collections::BTreeMap;
 use std::convert::Infallible;
 
+use crate::solver::PackageResolutionStatistics;
 use crate::{Dependencies, DependencyConstraints, DependencyProvider, Map, Package, VersionSet};
 
 /// A basic implementation of [DependencyProvider].
@@ -92,15 +93,23 @@ impl<P: Package, VS: VersionSet> DependencyProvider for OfflineDependencyProvide
             .and_then(|versions| versions.keys().rev().find(|v| range.contains(v)).cloned()))
     }
 
-    type Priority = Reverse<usize>;
+    type Priority = (u32, Reverse<usize>);
 
     #[inline]
-    fn prioritize(&self, package: &P, range: &VS) -> Self::Priority {
-        Reverse(
-            self.dependencies
-                .get(package)
-                .map(|versions| versions.keys().filter(|v| range.contains(v)).count())
-                .unwrap_or(0),
+    fn prioritize(
+        &self,
+        package: &Self::P,
+        range: &Self::VS,
+        package_statistics: &PackageResolutionStatistics,
+    ) -> Self::Priority {
+        (
+            package_statistics.conflict_count(),
+            Reverse(
+                self.dependencies
+                    .get(package)
+                    .map(|versions| versions.keys().filter(|v| range.contains(v)).count())
+                    .unwrap_or(0),
+            ),
         )
     }
 

src/solver.rs

@@ -68,6 +68,31 @@ use log::{debug, info};
 use crate::internal::{Id, Incompatibility, State};
 use crate::{DependencyConstraints, Map, Package, PubGrubError, SelectedDependencies, VersionSet};
 
+/// Statistics on how often a package conflicted with other packages.
+#[derive(Debug, Default, Clone)]
+pub struct PackageResolutionStatistics {
+    unit_propagation_affected: u32,
+    unit_propagation_culprit: u32,
+    dependencies_affected: u32,
+    dependencies_culprit: u32,
+}
+
+impl PackageResolutionStatistics {
+    /// The number of conflicts this package was involved in.
+    ///
+    /// Processing packages with a high conflict count earlier usually speeds up resolution.
+    ///
+    /// Whenever a package is part of the root cause incompatibility of a conflict, we increase its
+    /// count by one. Since the structure of the incompatibilities may change, this count too may
+    /// change in the future.
+    pub fn conflict_count(&self) -> u32 {
+        self.unit_propagation_affected
+            + self.unit_propagation_culprit
+            + self.dependencies_affected
+            + self.dependencies_culprit
+    }
+}
+
 /// Main function of the library.
 /// Finds a set of packages satisfying dependency bounds for a given package + version pair.
 #[cold]
@@ -77,6 +102,7 @@ pub fn resolve<DP: DependencyProvider>(
     version: impl Into<DP::V>,
 ) -> Result<SelectedDependencies<DP>, PubGrubError<DP>> {
     let mut state: State<DP> = State::init(package.clone(), version.into());
+    let mut conflict_tracker: Map<Id<DP::P>, PackageResolutionStatistics> = Map::default();
     let mut added_dependencies: Map<Id<DP::P>, Set<DP::V>> = Map::default();
     let mut next = state.root_package;
     loop {
@@ -88,7 +114,22 @@ pub fn resolve<DP: DependencyProvider>(
             "unit_propagation: {:?} = '{}'",
             &next, state.package_store[next]
         );
-        state.unit_propagation(next)?;
+        let root_causes = state.unit_propagation(next)?;
+        for (affected, incompat) in root_causes {
+            conflict_tracker
+                .entry(affected)
+                .or_default()
+                .unit_propagation_affected += 1;
+            for (conflict_package, _) in state.incompatibility_store[incompat].iter() {
+                if conflict_package == affected {
+                    continue;
+                }
+                conflict_tracker
+                    .entry(conflict_package)
+                    .or_default()
+                    .unit_propagation_culprit += 1;
+            }
+        }
 
         debug!(
             "Partial solution after unit propagation: {}",
@@ -97,7 +138,11 @@ pub fn resolve<DP: DependencyProvider>(
 
         let Some(highest_priority_pkg) =
             state.partial_solution.pick_highest_priority_pkg(|p, r| {
-                dependency_provider.prioritize(&state.package_store[p], r)
+                dependency_provider.prioritize(
+                    &state.package_store[p],
+                    r,
+                    conflict_tracker.entry(p).or_default(),
+                )
             })
         else {
             return Ok(state
@@ -171,7 +216,20 @@ pub fn resolve<DP: DependencyProvider>(
             };
 
             // Add that package and version if the dependencies are not problematic.
-            state.add_package_version_dependencies(p, v.clone(), dependencies);
+            if let Some(conflict) =
+                state.add_package_version_dependencies(p, v.clone(), dependencies)
+            {
+                conflict_tracker.entry(p).or_default().dependencies_affected += 1;
+                for (incompat_package, _) in state.incompatibility_store[conflict].iter() {
+                    if incompat_package == p {
+                        continue;
+                    }
+                    conflict_tracker
+                        .entry(incompat_package)
+                        .or_default()
+                        .unit_propagation_culprit += 1;
+                }
+            }
         } else {
             // `dep_incompats` are already in `incompatibilities` so we know there are not satisfied
             // terms and can add the decision directly.
@@ -249,7 +307,12 @@ pub trait DependencyProvider {
     ///
     /// Note: the resolver may call this even when the range has not changed,
     /// if it is more efficient for the resolvers internal data structures.
-    fn prioritize(&self, package: &Self::P, range: &Self::VS) -> Self::Priority;
+    fn prioritize(
+        &self,
+        package: &Self::P,
+        range: &Self::VS,
+        package_conflicts_counts: &PackageResolutionStatistics,
+    ) -> Self::Priority;
     /// The type returned from `prioritize`. The resolver does not care what type this is
     /// as long as it can pick a largest one and clone it.
     ///

tests/proptest.rs

@@ -13,8 +13,8 @@ use proptest::string::string_regex;
 
 use pubgrub::{
     resolve, DefaultStringReporter, Dependencies, DependencyProvider, DerivationTree, External,
-    OfflineDependencyProvider, Package, PubGrubError, Ranges, Reporter, SelectedDependencies,
-    VersionSet,
+    OfflineDependencyProvider, Package, PackageResolutionStatistics, PubGrubError, Ranges,
+    Reporter, SelectedDependencies, VersionSet,
 };
 
 use crate::sat_dependency_provider::SatResolve;
@@ -49,8 +49,13 @@ impl<P: Package, VS: VersionSet> DependencyProvider for OldestVersionsDependency
 
     type Priority = <OfflineDependencyProvider<P, VS> as DependencyProvider>::Priority;
 
-    fn prioritize(&self, package: &P, range: &VS) -> Self::Priority {
-        self.0.prioritize(package, range)
+    fn prioritize(
+        &self,
+        package: &Self::P,
+        range: &Self::VS,
+        package_statistics: &PackageResolutionStatistics,
+    ) -> Self::Priority {
+        self.0.prioritize(package, range, package_statistics)
     }
 
     type Err = Infallible;
@@ -104,8 +109,13 @@ impl<DP: DependencyProvider> DependencyProvider for TimeoutDependencyProvider<DP
 
     type Priority = DP::Priority;
 
-    fn prioritize(&self, package: &DP::P, range: &DP::VS) -> Self::Priority {
-        self.dp.prioritize(package, range)
+    fn prioritize(
+        &self,
+        package: &Self::P,
+        range: &Self::VS,
+        package_statistics: &PackageResolutionStatistics,
+    ) -> Self::Priority {
+        self.dp.prioritize(package, range, package_statistics)
     }
 
     type Err = DP::Err;