Skip to content

Commit

Permalink
Improve documentation of partial solution
Browse files Browse the repository at this point in the history
Improve the docstrings for the partial solution and rename some fields to be easier to understand.
  • Loading branch information
konstin committed Dec 11, 2024
1 parent 3741e3b commit 12140d7
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 46 deletions.
3 changes: 2 additions & 1 deletion src/internal/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ pub(crate) struct State<DP: DependencyProvider> {
#[allow(clippy::type_complexity)]
incompatibilities: Map<Id<DP::P>, Vec<IncompDpId<DP>>>,

/// Store the ids of incompatibilities that are already contradicted.
/// As optimization, store the ids of incompatibilities that are already contradicted.
///
/// For each one keep track of the decision level when it was found to be contradicted.
/// These will stay contradicted until we have backtracked beyond its associated decision level.
contradicted_incompatibilities: Map<IncompDpId<DP>, DecisionLevel>,
Expand Down
144 changes: 99 additions & 45 deletions src/internal/partial_solution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,24 +30,43 @@ impl DecisionLevel {
#[derive(Clone, Debug)]
pub(crate) struct PartialSolution<DP: DependencyProvider> {
next_global_index: u32,
/// The number of decisions that have been made, equal to the number of packages with decisions.
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 been called. Within this range there is no sorting.
/// 3. `[changed_this_decision_level..]` Contains all packages that **have** had there assignments changed since
/// the last time `prioritize` has been called. The inverse is not necessarily true, some packages in the range
/// did not have a change. Within this range there is no sorting.
/// Store for all known package decisions and package derivations.
///
/// "assignment" refers to both packages with decisions and package with only derivations and
/// no decision yet.
///
/// `[..current_decision_level]`: Packages that have had a decision made, sorted by the
/// `decision_level`. The section is can be seen as the partial solution, it contains a
/// mapping from package name to decided version. The sorting makes it very efficient to
/// extract the solution, and to backtrack to a particular decision level. The
/// `AssignmentsIntersection` is always a `Decision`.
///
/// `[changed_this_decision_level..]`: Packages that are dependencies of some other package,
/// but have not yet been decided. The `AssignmentsIntersection` is always a `Derivations`, the
/// derivations store the obligations from the decided packages. This section has two
/// subsections to optimize the number of `prioritize` calls:
///
/// `[current_decision_level..prioritize_decision_level]`: The assignments of packages in this
/// range have not changed since the last time `prioritize` has been called, their
/// priority in `prioritized_potential_packages` is fresh. There is no sorting within this
/// range.
///
/// `[prioritize_decision_level..]`: The assignments of packages in this range may have changed
/// since the last time `prioritize` has been called, their priority in
/// `prioritized_potential_packages` needs to be refreshed. There is no sorting within this
/// range.
#[allow(clippy::type_complexity)]
package_assignments: FnvIndexMap<Id<DP::P>, PackageAssignments<DP::P, DP::VS, DP::M>>,
/// `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.
/// Index into `package_assignments` to decide which packages need to be re-prioritized.
prioritize_decision_level: usize,
/// The undecided packages order by their `Priority`.
///
/// The max heap allows quickly `pop`ing the highest priority package.
prioritized_potential_packages:
PriorityQueue<Id<DP::P>, DP::Priority, BuildHasherDefault<FxHasher>>,
changed_this_decision_level: usize,
/// Track whether we have never backtracked to enable fast path optimizations.
has_ever_backtracked: bool,
}

Expand Down Expand Up @@ -78,15 +97,19 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
}
}

/// Package assignments contain the potential decision and derivations
/// that have already been made for a given package,
/// as well as the intersection of terms by all of these.
/// A package assignment is either a decision or a list of (accumulated) derivations without a
/// decision.
#[derive(Clone, Debug)]
struct PackageAssignments<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> {
/// Whether the assigment is a decision or a derivation.
assignments_intersection: AssignmentsIntersection<VS>,
/// All constraints on the package version from previous decisions, accumulated by decision
/// level.
dated_derivations: SmallVec<DatedDerivation<P, VS, M>>,
/// Smallest [`DecisionLevel`] in `dated_derivations`.
smallest_decision_level: DecisionLevel,
/// Highest [`DecisionLevel`] in `dated_derivations`.
highest_decision_level: DecisionLevel,
dated_derivations: SmallVec<DatedDerivation<P, VS, M>>,
assignments_intersection: AssignmentsIntersection<VS>,
}

impl<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> Display
Expand All @@ -112,8 +135,13 @@ impl<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> Display
#[derive(Clone, Debug)]
struct DatedDerivation<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> {
global_index: u32,
/// Only decisions up this level has been used to compute the accumulated term.
decision_level: DecisionLevel,
cause: IncompId<P, VS, M>,
/// The intersection of all terms up to `decision_level`.
///
/// It may not contain all terms of this `decision_level`, there may be more than one
/// `DatedDerivation` per decision level.
accumulated_intersection: Term<VS>,
}

Expand All @@ -127,15 +155,25 @@ impl<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> Display

#[derive(Clone, Debug)]
enum AssignmentsIntersection<VS: VersionSet> {
Decision((u32, VS::V, Term<VS>)),
/// A decision on package for version has been made at the given level.
Decision {
decision_level: u32,
version: VS::V,
/// The version, but as positive singleton term.
term: Term<VS>,
},
Derivations(Term<VS>),
}

impl<VS: VersionSet> Display for AssignmentsIntersection<VS> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::Decision((lvl, version, _)) => {
write!(f, "Decision: level {}, v = {}", lvl, version)
Self::Decision {
decision_level,
version,
term: _,
} => {
write!(f, "Decision: level {}, v = {}", decision_level, version)
}
Self::Derivations(term) => write!(f, "Derivations term: {}", term),
}
Expand All @@ -162,7 +200,7 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
current_decision_level: DecisionLevel(0),
package_assignments: FnvIndexMap::default(),
prioritized_potential_packages: PriorityQueue::default(),
changed_this_decision_level: 0,
prioritize_decision_level: 0,
has_ever_backtracked: false,
}
}
Expand All @@ -175,7 +213,9 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
None => panic!("Derivations must already exist"),
Some(pa) => match &pa.assignments_intersection {
// Cannot be called when a decision has already been taken.
AssignmentsIntersection::Decision(_) => panic!("Already existing decision"),
AssignmentsIntersection::Decision { .. } => {
panic!("Already existing decision")
}
// Cannot be called if the versions is not contained in the terms' intersection.
AssignmentsIntersection::Derivations(term) => {
debug_assert!(
Expand All @@ -189,7 +229,7 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
},
}
assert_eq!(
self.changed_this_decision_level,
self.prioritize_decision_level,
self.package_assignments.len()
);
}
Expand All @@ -200,11 +240,11 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
.get_full_mut(&package)
.expect("Derivations must already exist");
pa.highest_decision_level = self.current_decision_level;
pa.assignments_intersection = AssignmentsIntersection::Decision((
self.next_global_index,
version.clone(),
Term::exact(version),
));
pa.assignments_intersection = AssignmentsIntersection::Decision {
decision_level: self.next_global_index,
version: version.clone(),
term: 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);
Expand Down Expand Up @@ -235,7 +275,7 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
pa.highest_decision_level = self.current_decision_level;
match &mut pa.assignments_intersection {
// Check that add_derivation is never called in the wrong context.
AssignmentsIntersection::Decision(_) => {
AssignmentsIntersection::Decision { .. } => {
panic!("add_derivation should not be called after a decision")
}
AssignmentsIntersection::Derivations(t) => {
Expand All @@ -244,8 +284,8 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
if t.is_positive() {
// we can use `swap_indices` to make `changed_this_decision_level` only go down by 1
// but the copying is slower then the larger search
self.changed_this_decision_level =
std::cmp::min(self.changed_this_decision_level, idx);
self.prioritize_decision_level =
std::cmp::min(self.prioritize_decision_level, idx);
}
}
}
Expand All @@ -254,8 +294,8 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
Entry::Vacant(v) => {
let term = dated_derivation.accumulated_intersection.clone();
if term.is_positive() {
self.changed_this_decision_level =
std::cmp::min(self.changed_this_decision_level, pa_last_index);
self.prioritize_decision_level =
std::cmp::min(self.prioritize_decision_level, pa_last_index);
}
v.insert(PackageAssignments {
smallest_decision_level: self.current_decision_level,
Expand All @@ -272,12 +312,12 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
&mut self,
prioritizer: impl Fn(Id<DP::P>, &DP::VS) -> DP::Priority,
) -> Option<Id<DP::P>> {
let check_all = self.changed_this_decision_level
let check_all = self.prioritize_decision_level
== self.current_decision_level.0.saturating_sub(1) as usize;
let current_decision_level = self.current_decision_level;
let prioritized_potential_packages = &mut self.prioritized_potential_packages;
self.package_assignments
.get_range(self.changed_this_decision_level..)
.get_range(self.prioritize_decision_level..)
.unwrap()
.iter()
.filter(|(_, pa)| {
Expand All @@ -292,7 +332,7 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
let priority = prioritizer(p, r);
prioritized_potential_packages.push(p, priority);
});
self.changed_this_decision_level = self.package_assignments.len();
self.prioritize_decision_level = self.package_assignments.len();
prioritized_potential_packages.pop().map(|(p, _)| p)
}

Expand All @@ -304,7 +344,11 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
.iter()
.take(self.current_decision_level.0 as usize)
.map(|(&p, pa)| match &pa.assignments_intersection {
AssignmentsIntersection::Decision((_, v, _)) => (p, v.clone()),
AssignmentsIntersection::Decision {
decision_level: _,
version: v,
term: _,
} => (p, v.clone()),
AssignmentsIntersection::Derivations(_) => {
panic!("Derivations in the Decision part")
}
Expand Down Expand Up @@ -349,7 +393,7 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
});
// 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;
self.prioritize_decision_level = self.current_decision_level.0.saturating_sub(1) as usize;
self.has_ever_backtracked = true;
}

Expand Down Expand Up @@ -486,7 +530,11 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
} else {
match &satisfier_pa.assignments_intersection {
AssignmentsIntersection::Derivations(_) => panic!("must be a decision"),
AssignmentsIntersection::Decision((_, _, term)) => term.clone(),
AssignmentsIntersection::Decision {
decision_level: _,
version: _,
term,
} => term.clone(),
}
};

Expand Down Expand Up @@ -534,9 +582,11 @@ impl<P: Package, VS: VersionSet, M: Eq + Clone + Debug + Display> PackageAssignm
// If it wasn't found in the derivations,
// it must be the decision which is last (if called in the right context).
match &self.assignments_intersection {
AssignmentsIntersection::Decision((global_index, _, _)) => {
(None, *global_index, self.highest_decision_level)
}
AssignmentsIntersection::Decision {
decision_level: global_index,
version: _,
term: _,
} => (None, *global_index, self.highest_decision_level),
AssignmentsIntersection::Derivations(accumulated_intersection) => {
unreachable!(
concat!(
Expand All @@ -557,7 +607,11 @@ impl<VS: VersionSet> AssignmentsIntersection<VS> {
/// Returns the term intersection of all assignments (decision included).
fn term(&self) -> &Term<VS> {
match self {
Self::Decision((_, _, term)) => term,
Self::Decision {
decision_level: _,
version: _,
term,
} => term,
Self::Derivations(term) => term,
}
}
Expand All @@ -568,7 +622,7 @@ impl<VS: VersionSet> AssignmentsIntersection<VS> {
/// in the partial solution.
fn potential_package_filter<P: Package>(&self, package: Id<P>) -> Option<(Id<P>, &VS)> {
match self {
Self::Decision(_) => None,
Self::Decision { .. } => None,
Self::Derivations(term_intersection) => {
if term_intersection.is_positive() {
Some((package, term_intersection.unwrap_positive()))
Expand Down

0 comments on commit 12140d7

Please sign in to comment.