more docs

src/error.rs

@@ -46,7 +46,7 @@ pub enum PubGrubError<P: Package, VS: VersionSet> {
     /// Error arising when the implementer of
     /// [DependencyProvider](crate::solver::DependencyProvider)
     /// returned an error in the method
-    /// [choose_package_version](crate::solver::DependencyProvider::choose_package_version).
+    /// [choose_version](crate::solver::DependencyProvider::choose_version).
     #[error("Decision making failed")]
     ErrorChoosingPackageVersion(Box<dyn std::error::Error + Send + Sync>),
 

src/internal/partial_solution.rs

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: MPL-2.0
 
 //! A Memory acts like a structured partial solution
-//! where terms are regrouped by package in a [Map].
+//! where terms are regrouped by package in a [Map](crate::type_aliases::Map).
 
 use std::fmt::Display;
 use std::hash::BuildHasherDefault;
@@ -36,7 +36,19 @@ impl DecisionLevel {
 pub struct PartialSolution<P: Package, VS: VersionSet, Priority: Ord + Clone> {
     next_global_index: u32,
     current_decision_level: DecisionLevel,
+    /// `package_assignments` is primarily a HashMap from a package to its
+    /// `PackageAssignments`. But it can also keep the items in an order.
+    ///  We maintain three sections in this order:
+    /// 1. `[..current_decision_level]` Are packages that have had a decision made sorted by the `decision_level`.
+    ///    This makes it very efficient to extract the solution, And to backtrack to a particular decision level.
+    /// 2. `[current_decision_level..changed_this_decision_level]` Are packages that have **not** had there assignments
+    ///    changed since the last time `prioritize` has bean called. Within this range there is no sorting.
+    /// 3. `[changed_this_decision_level..]` Containes all packages that **have** had there assignments changed since
+    ///    the last time `prioritize` has bean called. The inverse is not necessarily true, some packages in the range
+    ///    did not have a change. Within this range there is no sorting.
     package_assignments: FnvIndexMap<P, PackageAssignments<P, VS>>,
+    /// `prioritized_potential_packages` is primarily a HashMap from a package with no desition and a positive assignment
+    /// to its `Priority`. But, it also maintains a max heap of packages by `Priority` order.
     prioritized_potential_packages: PriorityQueue<P, Priority, BuildHasherDefault<FxHasher>>,
     changed_this_decision_level: usize,
 }
@@ -174,6 +186,7 @@ impl<P: Package, VS: VersionSet, Priority: Ord + Clone> PartialSolution<P, VS, P
             version.clone(),
             Term::exact(version),
         ));
+        // Maintain that the beginning of the `package_assignments` Have all decisions in sorted order.
         if new_idx != old_idx {
             self.package_assignments.swap_indices(new_idx, old_idx);
         }
@@ -241,7 +254,13 @@ impl<P: Package, VS: VersionSet, Priority: Ord + Clone> PartialSolution<P, VS, P
         let prioritized_potential_packages = &mut self.prioritized_potential_packages;
         (self.changed_this_decision_level..package_assignments.len())
             .map(|i| package_assignments.get_index(i).unwrap())
-            .filter(|(_, pa)| check_all || pa.highest_decision_level == current_decision_level)
+            .filter(|(_, pa)| {
+                // We only actually need to update the package if its Been changed
+                // since the last time we called prioritize.
+                // Which means it's highest decision level is the current decision level,
+                // or if we backtracked in the mean time.
+                check_all || pa.highest_decision_level == current_decision_level
+            })
             .filter_map(|(p, pa)| pa.assignments_intersection.potential_package_filter(p))
             .for_each(|(p, r)| {
                 let priority = prioritizer(p, r);
@@ -311,6 +330,7 @@ impl<P: Package, VS: VersionSet, Priority: Ord + Clone> PartialSolution<P, VS, P
                 true
             }
         });
+        // Throw away all stored priority levels, And mark that they all need to be recomputed.
         self.prioritized_potential_packages.clear();
         self.changed_this_decision_level = self.current_decision_level.0.saturating_sub(1) as usize;
     }

src/lib.rs

@@ -109,18 +109,17 @@
 //! ```
 //!
 //! The first method
-//! [choose_package_version](crate::solver::DependencyProvider::choose_package_version)
-//! chooses a package and available version compatible with the provided options.
-//! A helper function
-//! [choose_package_with_fewest_versions](crate::solver::choose_package_with_fewest_versions)
-//! is provided for convenience
-//! in cases when lists of available versions for packages are easily obtained.
-//! The strategy of that helper function consists in choosing the package
-//! with the fewest number of compatible versions to speed up resolution.
+//! [choose_version](crate::solver::DependencyProvider::choose_version)
+//! chooses a version compatible with the provided range for a package.
+//! The second method
+//! [prioritize](crate::solver::DependencyProvider::prioritize)
+//! in which order different packages should be chosen.
+//! Usually prioritizing packages
+//! with the fewest number of compatible versions speeds up resolution.
 //! But in general you are free to employ whatever strategy suits you best
 //! to pick a package and a version.
 //!
-//! The second method [get_dependencies](crate::solver::DependencyProvider::get_dependencies)
+//! The third method [get_dependencies](crate::solver::DependencyProvider::get_dependencies)
 //! aims at retrieving the dependencies of a given package at a given version.
 //! Returns [None] if dependencies are unknown.
 //!

src/solver.rs

@@ -206,15 +206,49 @@ pub enum Dependencies<P: Package, VS: VersionSet> {
 /// Trait that allows the algorithm to retrieve available packages and their dependencies.
 /// An implementor needs to be supplied to the [resolve] function.
 pub trait DependencyProvider<P: Package, VS: VersionSet> {
+    /// [Decision making](https://github.com/dart-lang/pub/blob/master/doc/solver.md#decision-making)
+    /// is the process of choosing the next package
+    /// and version that will be appended to the partial solution.
+    ///
+    /// Every time such a decision must be made, the resolver looks at all the potential valid
+    /// packages that have changed, and a asks the dependency provider how important each one is.
+    /// For each one it calls `prioritize` with the name of the package and the current set of
+    /// acceptable versions.
+    /// The resolver will then pick the package with the highes priority from all the potential valid
+    /// packages.
+    ///
+    /// The strategy employed to prioritize packages
+    /// cannot change the existence of a solution or not,
+    /// but can drastically change the performances of the solver,
+    /// or the properties of the solution.
+    /// The documentation of Pub (PubGrub implementation for the dart programming language)
+    /// states the following:
+    ///
+    /// > Pub chooses the latest matching version of the package
+    /// > with the fewest versions that match 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.
+    ///
+    /// Note: the resolver may call this even when the range has not change,
+    /// if it is more efficient for the resolveres internal data structures.
+    fn prioritize(&self, package: &P, range: &VS) -> 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.
+    ///
+    /// [std::cmp::Reverse] can be useful if you want to pick the package with
+    /// the fewest versions that match the outstanding constraint.
+    type Priority: Ord + Clone;
+
+    /// Once the resolver has found the highest `Priority` package from all potential valid
+    /// packages, it needs to know what vertion of that package to use. The most common pattern
+    /// is to select the largest vertion that the range contains.
     fn choose_version(
         &self,
         package: &P,
         range: &VS,
     ) -> Result<Option<VS::V>, Box<dyn Error + Send + Sync>>;
 
-    type Priority: Ord + Clone;
-    fn prioritize(&self, package: &P, range: &VS) -> Self::Priority;
-
     /// Retrieves the package dependencies.
     /// Return [Dependencies::Unknown] if its dependencies are unknown.
     fn get_dependencies(
@@ -302,7 +336,8 @@ impl<P: Package, VS: VersionSet> OfflineDependencyProvider<P, VS> {
 
 /// An implementation of [DependencyProvider] that
 /// contains all dependency information available in memory.
-/// Packages are picked with the fewest versions contained in the constraints first.
+/// Currently packages are picked with the fewest versions contained in the constraints first.
+/// But, that may change in new versions if better heuristics are found.
 /// Versions are picked with the newest versions first.
 impl<P: Package, VS: VersionSet> DependencyProvider<P, VS> for OfflineDependencyProvider<P, VS> {
     fn choose_version(

src/version.rs

@@ -121,7 +121,7 @@ impl SemanticVersion {
 }
 
 /// Error creating [SemanticVersion] from [String].
-#[derive(Error, Debug, PartialEq)]
+#[derive(Error, Debug, PartialEq, Eq)]
 pub enum VersionParseError {
     /// [SemanticVersion] must contain major, minor, patch versions.
     #[error("version {full_version} must contain 3 numbers separated by dot")]