diff --git a/Cargo.toml b/Cargo.toml index a796249e..b9caae8f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,6 +22,7 @@ include = ["Cargo.toml", "LICENSE", "README.md", "src/**", "tests/**", "examples [dependencies] thiserror = "1.0" rustc-hash = "1.1.0" +indexmap = "1.6.2" serde = { version = "1.0", features = ["derive"], optional = true } log = "0.4.14" # for debug logs in tests diff --git a/src/internal/partial_solution.rs b/src/internal/partial_solution.rs index 23960a45..d85620e4 100644 --- a/src/internal/partial_solution.rs +++ b/src/internal/partial_solution.rs @@ -10,11 +10,15 @@ use crate::internal::incompatibility::{IncompId, Incompatibility, Relation}; use crate::internal::small_map::SmallMap; use crate::package::Package; use crate::term::Term; -use crate::type_aliases::{Map, SelectedDependencies}; +use crate::type_aliases::SelectedDependencies; use crate::version_set::VersionSet; use super::small_vec::SmallVec; +use std::hash::BuildHasherDefault; + +type FnvIndexMap = indexmap::IndexMap>; + #[derive(Debug, Copy, Clone, Ord, PartialOrd, Eq, PartialEq)] pub struct DecisionLevel(pub u32); @@ -30,7 +34,7 @@ impl DecisionLevel { pub struct PartialSolution { next_global_index: u32, current_decision_level: DecisionLevel, - package_assignments: Map>, + package_assignments: FnvIndexMap>, } impl Display for PartialSolution { @@ -126,7 +130,7 @@ impl PartialSolution { Self { next_global_index: 0, current_decision_level: DecisionLevel(0), - package_assignments: Map::default(), + package_assignments: FnvIndexMap::default(), } } @@ -146,10 +150,11 @@ impl PartialSolution { }, } } + let new_idx = self.current_decision_level.0 as usize; self.current_decision_level = self.current_decision_level.increment(); - let mut pa = self + let (old_idx, _, mut pa) = self .package_assignments - .get_mut(&package) + .get_full_mut(&package) .expect("Derivations must already exist"); pa.highest_decision_level = self.current_decision_level; pa.assignments_intersection = AssignmentsIntersection::Decision(( @@ -157,6 +162,9 @@ impl PartialSolution { version.clone(), Term::exact(version), )); + if new_idx != old_idx { + self.package_assignments.swap_indices(new_idx, old_idx); + } self.next_global_index += 1; } @@ -167,7 +175,7 @@ impl PartialSolution { cause: IncompId, store: &Arena>, ) { - use std::collections::hash_map::Entry; + use indexmap::map::Entry; let term = store[cause].get(&package).unwrap().negate(); let dated_derivation = DatedDerivation { global_index: self.next_global_index, @@ -208,9 +216,8 @@ impl PartialSolution { /// and if it contains at least one positive derivation term /// in the partial solution. pub fn potential_packages(&self) -> Option> { - let mut iter = self - .package_assignments - .iter() + let mut iter = (self.current_decision_level.0 as usize..self.package_assignments.len()) + .map(move |i| self.package_assignments.get_index(i).unwrap()) .filter_map(|(p, pa)| pa.assignments_intersection.potential_package_filter(p)) .peekable(); if iter.peek().is_some() { @@ -223,21 +230,17 @@ impl PartialSolution { /// If a partial solution has, for every positive derivation, /// a corresponding decision that satisfies that assignment, /// it's a total solution and version solving has succeeded. - pub fn extract_solution(&self) -> Option> { - let mut solution = Map::default(); - for (p, pa) in &self.package_assignments { - match &pa.assignments_intersection { - AssignmentsIntersection::Decision((_, v, _)) => { - solution.insert(p.clone(), v.clone()); - } - AssignmentsIntersection::Derivations(term) => { - if term.is_positive() { - return None; - } + pub fn extract_solution(&self) -> SelectedDependencies { + self.package_assignments + .iter() + .take(self.current_decision_level.0 as usize) + .map(|(p, pa)| match &pa.assignments_intersection { + AssignmentsIntersection::Decision((_, v, _)) => (p.clone(), v.clone()), + AssignmentsIntersection::Derivations(_) => { + panic!("Derivations in the Decision part") } - } - } - Some(solution) + }) + .collect() } /// Backtrack the partial solution to a given decision level. @@ -380,7 +383,7 @@ impl PartialSolution { /// to return a coherent previous_satisfier_level. fn find_satisfier( incompat: &Incompatibility, - package_assignments: &Map>, + package_assignments: &FnvIndexMap>, store: &Arena>, ) -> SmallMap { let mut satisfied = SmallMap::Empty; @@ -401,7 +404,7 @@ impl PartialSolution { incompat: &Incompatibility, satisfier_package: &P, mut satisfied_map: SmallMap, - package_assignments: &Map>, + package_assignments: &FnvIndexMap>, store: &Arena>, ) -> DecisionLevel { // First, let's retrieve the previous derivations and the initial accum_term. diff --git a/src/solver.rs b/src/solver.rs index 846f220c..f2fc4eb5 100644 --- a/src/solver.rs +++ b/src/solver.rs @@ -107,11 +107,7 @@ pub fn resolve( // The borrow checker did not like using a match on potential_packages. // This `if ... is_none ... drop` is a workaround. // I believe this is a case where Polonius could help, when and if it lands in rustc. - return state.partial_solution.extract_solution().ok_or_else(|| { - PubGrubError::Failure( - "How did we end up with no package to choose but no solution?".into(), - ) - }); + return Ok(state.partial_solution.extract_solution()); } let decision = dependency_provider .choose_package_version(potential_packages.unwrap())