perf: choose package with fewest versions matching (30x speedup) (#32)

* perf: choose package with fewest matching versions (30x speedup)

* docs: change heuristic description for picking packages

Co-authored-by: Alex Tokarev <aleksator@gmail.com>

benches/large_case.rs

@@ -10,8 +10,6 @@ use pubgrub::version::NumberVersion;
 #[cfg(feature = "serde")]
 #[bench]
 /// This is an entirely synthetic benchmark. It may not be realistic.
-/// It is too slow to be useful in the long term. But hopefully that can be fixed by making [resolve](crate::solver::resolve) faster.
-/// It has not bean minimized. There are many [add_dependencies](crate::solver::DependencyProvider::add_dependencies) that have no impact on the runtime or output.
 fn large_case(b: &mut Bencher) {
     let s = std::fs::read_to_string("test-examples/large_case_u16_NumberVersion.ron").unwrap();
     let dependency_provider: OfflineDependencyProvider<u16, NumberVersion> =

src/internal/incompatibility.rs

@@ -260,7 +260,7 @@ impl<P: Package, V: Version> Incompatibility<P, V> {
             false
         } else {
             let (package, term) = self.package_terms.iter().next().unwrap();
-            (package == root_package) && term.accept_version(&root_version)
+            (package == root_package) && term.contains(&root_version)
         }
     }
 

src/internal/partial_solution.rs

@@ -3,13 +3,19 @@
 //! The partial solution is the current state
 //! of the solution being built by the algorithm.
 
-use crate::internal::assignment::Assignment::{self, Decision, Derivation};
-use crate::internal::incompatibility::{Incompatibility, Relation};
 use crate::internal::memory::Memory;
 use crate::package::Package;
 use crate::term::Term;
 use crate::type_aliases::Map;
 use crate::version::Version;
+use crate::{
+    error::PubGrubError,
+    internal::incompatibility::{Incompatibility, Relation},
+};
+use crate::{
+    internal::assignment::Assignment::{self, Decision, Derivation},
+    solver::DependencyProvider,
+};
 
 /// The partial solution is the current state
 /// of the solution being built by the algorithm.
@@ -92,19 +98,36 @@ impl<P: Package, V: Version> PartialSolution<P, V> {
     /// This should be a package with a positive derivation but no decision yet.
     /// If multiple choices are possible, use a heuristic.
     ///
-    /// Pub chooses the package with the fewest versions
-    /// matching the outstanding constraint.
+    /// Current heuristic employed by this and Pub's implementations is to choose
+    /// the package with the fewest versions matching the outstanding constraint.
     /// This tends to find conflicts earlier if any exist,
     /// since these packages will run out of versions to try more quickly.
-    /// But there's likely room for improvement in these heuristics.
-    ///
-    /// Here we just pick the first one.
-    /// TODO: improve? (do not introduce any side effect if trying to improve)
-    pub fn pick_package(&self) -> Option<(P, Term<V>)> {
-        self.memory
+    pub fn pick_package(
+        &self,
+        dependency_provider: &impl DependencyProvider<P, V>,
+    ) -> Result<Option<(P, Term<V>)>, PubGrubError<P, V>> {
+        let mut out: Option<(P, Term<V>)> = None;
+        let mut min_key = usize::MAX;
+        for (p, term) in self
+            .memory
             .potential_packages()
-            .next()
-            .map(|(p, all_terms)| (p.clone(), Term::intersect_all(all_terms.iter())))
+            .map(|(p, all_terms)| (p, Term::intersect_all(all_terms.iter())))
+        {
+            let key = dependency_provider
+                .list_available_versions(p)
+                .map_err(|err| PubGrubError::ErrorRetrievingVersions {
+                    package: p.clone(),
+                    source: err,
+                })?
+                .iter()
+                .filter(|&v| term.contains(v))
+                .count();
+            if key < min_key {
+                min_key = key;
+                out = Some((p.clone(), term));
+            }
+        }
+        Ok(out)
     }
 
     /// Pub chooses the latest matching version of the package
@@ -116,7 +139,7 @@ impl<P: Package, V: Version> PartialSolution<P, V> {
     pub fn pick_version(available_versions: &[V], partial_solution_term: &Term<V>) -> Option<V> {
         available_versions
             .iter()
-            .find(|v| partial_solution_term.accept_version(v))
+            .find(|v| partial_solution_term.contains(v))
             .cloned()
     }
 

src/solver.rs

@@ -91,7 +91,7 @@ pub fn resolve<P: Package, V: Version>(
         state.unit_propagation(next)?;
 
         // Pick the next package.
-        let (p, term) = match state.partial_solution.pick_package() {
+        let (p, term) = match state.partial_solution.pick_package(dependency_provider)? {
             None => {
                 return state
                     .partial_solution

src/term.rs

@@ -61,7 +61,7 @@ impl<V: Version> Term<V> {
     }
 
     /// Evaluate a term regarding a given choice of version.
-    pub(crate) fn accept_version(&self, v: &V) -> bool {
+    pub(crate) fn contains(&self, v: &V) -> bool {
         match self {
             Self::Positive(range) => range.contains(v),
             Self::Negative(range) => !(range.contains(v)),