Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve documentation of partial solution #294

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
+104 −47
Open

Improve documentation of partial solution #294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
12140d7
Improve documentation of partial solution
konstin Dec 11, 2024
12d48d9
Update src/internal/core.rs
konstin Dec 12, 2024
8ac333a
Update src/internal/partial_solution.rs
konstin Dec 12, 2024
1c003c4
Add context
konstin Dec 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/internal/core.rs
View file
Open in desktop
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 an 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
148 changes: 102 additions & 46 deletions src/internal/partial_solution.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,24 +30,45 @@ 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,
Comment on lines +33 to 34
Copy link
Member

@zanieb zanieb Dec 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"packages with decisions" being "packages with a decided version"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

/// `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. We combine this in a single index map, where different sections (of
/// indexes) contain package with different level of information, and make a decision moves a
/// package from the derivations sections to the decisions section.
///
/// `[..current_decision_level]`: Packages that have had a decision made, sorted by the
konstin marked this conversation as resolved.
Show resolved Hide resolved
/// `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`.
///
/// `[prioritize_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,
/// Whether we have never backtracked, to enable fast path optimizations.
has_ever_backtracked: bool,
}

Expand Down Expand Up @@ -78,15 +99,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 +137,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 +157,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 +202,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 +215,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 +231,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 +242,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,17 +277,17 @@ 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) => {
*t = t.intersection(&dated_derivation.accumulated_intersection);
dated_derivation.accumulated_intersection = t.clone();
if t.is_positive() {
// we can use `swap_indices` to make `changed_this_decision_level` only go down by 1
// we can use `swap_indices` to make `prioritize_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 +296,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 +314,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 +334,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 +346,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 +395,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 +532,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 +584,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 +609,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 +624,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
Loading