From bda6b97df2ae315f23bd09df685215b5a2f70578 Mon Sep 17 00:00:00 2001 From: x-hgg-x <39058530+x-hgg-x@users.noreply.github.com> Date: Sat, 2 Nov 2024 23:52:15 +0100 Subject: [PATCH] Use smallvec crate in pubgrub --- Cargo.lock | 1 + Cargo.toml | 3 +- src/internal/core.rs | 12 +- src/internal/mod.rs | 2 - src/internal/partial_solution.rs | 14 +- src/internal/small_vec.rs | 232 ------------------------------- version-ranges/src/lib.rs | 2 +- 7 files changed, 18 insertions(+), 248 deletions(-) delete mode 100644 src/internal/small_vec.rs diff --git a/Cargo.lock b/Cargo.lock index 4d87781b..7763faf9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -609,6 +609,7 @@ dependencies = [ "ron", "rustc-hash 2.0.0", "serde", + "smallvec", "thiserror 2.0.0", "varisat", "version-ranges", diff --git a/Cargo.toml b/Cargo.toml index 49362ab5..4bca2e59 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,8 +20,6 @@ keywords = ["dependency", "pubgrub", "semver", "solver", "version"] categories = ["algorithms"] include = ["Cargo.toml", "LICENSE", "README.md", "src/**", "tests/**", "examples/**", "benches/**"] -# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html - [dependencies] indexmap = "2.6.0" # for debug logs in tests @@ -29,6 +27,7 @@ log = "0.4.22" priority-queue = "2.1.1" rustc-hash = ">=1.0.0, <3.0.0" serde = { version = "1.0", features = ["derive"], optional = true } +smallvec = { version = "1.13.2", features = ["union"] } thiserror = "2.0" version-ranges = { version = "0.1.0", path = "version-ranges" } diff --git a/src/internal/core.rs b/src/internal/core.rs index 71659394..8831b199 100644 --- a/src/internal/core.rs +++ b/src/internal/core.rs @@ -5,10 +5,12 @@ use std::sync::Arc; +use smallvec::SmallVec; + use crate::{ internal::{ Arena, DecisionLevel, IncompDpId, Incompatibility, PartialSolution, Relation, - SatisfierSearch, SmallVec, + SatisfierSearch, }, DependencyProvider, DerivationTree, Map, PackageArena, PackageId, Set, Term, VersionIndex, VersionSet, @@ -26,7 +28,7 @@ pub(crate) struct State { /// All incompatibilities expressing dependencies, /// with common dependents merged. #[allow(clippy::type_complexity)] - merged_dependencies: Map<(PackageId, PackageId), SmallVec>>, + merged_dependencies: Map<(PackageId, PackageId), SmallVec<[IncompDpId; 4]>>, /// Partial solution. /// TODO: remove pub. @@ -38,7 +40,7 @@ pub(crate) struct State { /// This is a stack of work to be done in `unit_propagation`. /// It can definitely be a local variable to that method, but /// this way we can reuse the same allocation for better performance. - unit_propagation_buffer: SmallVec, + unit_propagation_buffer: Vec, } impl State { @@ -57,7 +59,7 @@ impl State { incompatibilities, partial_solution: PartialSolution::empty(), incompatibility_store, - unit_propagation_buffer: SmallVec::Empty, + unit_propagation_buffer: Vec::new(), merged_dependencies: Map::default(), } } @@ -254,7 +256,7 @@ impl State { if let Some((pid1, pid2)) = self.incompatibility_store[id].as_dependency() { // If we are a dependency, there's a good chance we can be merged with a previous dependency let deps_lookup = self.merged_dependencies.entry((pid1, pid2)).or_default(); - if let Some((past, merged)) = deps_lookup.as_mut_slice().iter_mut().find_map(|past| { + if let Some((past, merged)) = deps_lookup.iter_mut().find_map(|past| { self.incompatibility_store[id] .merge_dependents(&self.incompatibility_store[*past]) .map(|m| (past, m)) diff --git a/src/internal/mod.rs b/src/internal/mod.rs index 643634f3..08aea9cd 100644 --- a/src/internal/mod.rs +++ b/src/internal/mod.rs @@ -7,11 +7,9 @@ mod core; mod incompatibility; mod partial_solution; mod small_map; -mod small_vec; pub(crate) use arena::{Arena, Id}; pub(crate) use core::State; pub(crate) use incompatibility::{IncompDpId, IncompId, Incompatibility, Relation}; pub(crate) use partial_solution::{DecisionLevel, PartialSolution, SatisfierSearch}; pub(crate) use small_map::SmallMap; -pub(crate) use small_vec::SmallVec; diff --git a/src/internal/partial_solution.rs b/src/internal/partial_solution.rs index 14d143e0..cd957bfc 100644 --- a/src/internal/partial_solution.rs +++ b/src/internal/partial_solution.rs @@ -7,10 +7,11 @@ use std::hash::BuildHasherDefault; use priority_queue::PriorityQueue; use rustc_hash::FxHasher; +use smallvec::{smallvec, SmallVec}; use crate::Map; use crate::{ - internal::{Arena, IncompDpId, IncompId, Incompatibility, Relation, SmallMap, SmallVec}, + internal::{Arena, IncompDpId, IncompId, Incompatibility, Relation, SmallMap}, DependencyProvider, FxIndexMap, PackageArena, PackageId, SelectedDependencies, Term, VersionIndex, VersionSet, }; @@ -97,7 +98,7 @@ impl<'a, DP: DependencyProvider> Display for PartialSolutionDisplay<'a, DP> { struct PackageAssignments { smallest_decision_level: DecisionLevel, highest_decision_level: DecisionLevel, - dated_derivations: SmallVec>, + dated_derivations: SmallVec<[DatedDerivation; 1]>, assignments_intersection: AssignmentsIntersection, } @@ -298,7 +299,7 @@ impl PartialSolution { v.insert(PackageAssignments { smallest_decision_level: self.current_decision_level, highest_decision_level: self.current_decision_level, - dated_derivations: SmallVec::One([dated_derivation]), + dated_derivations: smallvec![dated_derivation], assignments_intersection: AssignmentsIntersection::derivations(term), }); } @@ -599,14 +600,15 @@ impl PackageAssignments { start_term: Term, package_store: &PackageArena, ) -> (Option>, u32, DecisionLevel) { - let empty = Term::empty(); // Indicate if we found a satisfier in the list of derivations, otherwise it will be the decision. let idx = self .dated_derivations - .as_slice() .partition_point(|dd| !dd.accumulated_intersection.is_disjoint(start_term)); if let Some(dd) = self.dated_derivations.get(idx) { - debug_assert_eq!(dd.accumulated_intersection.intersection(start_term), empty); + debug_assert_eq!( + dd.accumulated_intersection.intersection(start_term), + Term::empty(), + ); return (Some(dd.cause), dd.global_index, dd.decision_level); } // If it wasn't found in the derivations, it must be the decision which is last (if called in the right context). diff --git a/src/internal/small_vec.rs b/src/internal/small_vec.rs deleted file mode 100644 index 621fc9bf..00000000 --- a/src/internal/small_vec.rs +++ /dev/null @@ -1,232 +0,0 @@ -use std::fmt; -use std::hash::{Hash, Hasher}; -use std::ops::Deref; - -#[derive(Clone)] -pub enum SmallVec { - Empty, - One([T; 1]), - Two([T; 2]), - Flexible(Vec), -} - -impl SmallVec { - pub fn empty() -> Self { - Self::Empty - } - - pub fn one(t: T) -> Self { - Self::One([t]) - } - - pub fn as_slice(&self) -> &[T] { - match self { - Self::Empty => &[], - Self::One(v) => v, - Self::Two(v) => v, - Self::Flexible(v) => v, - } - } - - pub fn as_mut_slice(&mut self) -> &mut [T] { - match self { - Self::Empty => &mut [], - Self::One(v) => v, - Self::Two(v) => v, - Self::Flexible(v) => v, - } - } - - pub fn push(&mut self, new: T) { - *self = match std::mem::take(self) { - Self::Empty => Self::One([new]), - Self::One([v1]) => Self::Two([v1, new]), - Self::Two([v1, v2]) => Self::Flexible(vec![v1, v2, new]), - Self::Flexible(mut v) => { - v.push(new); - Self::Flexible(v) - } - } - } - - pub fn pop(&mut self) -> Option { - match std::mem::take(self) { - Self::Empty => None, - Self::One([v1]) => { - *self = Self::Empty; - Some(v1) - } - Self::Two([v1, v2]) => { - *self = Self::One([v1]); - Some(v2) - } - Self::Flexible(mut v) => { - let out = v.pop(); - *self = Self::Flexible(v); - out - } - } - } - - pub fn clear(&mut self) { - if let Self::Flexible(mut v) = std::mem::take(self) { - v.clear(); - *self = Self::Flexible(v); - } // else: self already eq Empty from the take - } - - pub fn iter(&self) -> std::slice::Iter<'_, T> { - self.as_slice().iter() - } -} - -impl Default for SmallVec { - fn default() -> Self { - Self::Empty - } -} - -impl Deref for SmallVec { - type Target = [T]; - - fn deref(&self) -> &Self::Target { - self.as_slice() - } -} - -impl<'a, T> IntoIterator for &'a SmallVec { - type Item = &'a T; - - type IntoIter = std::slice::Iter<'a, T>; - - fn into_iter(self) -> Self::IntoIter { - self.iter() - } -} - -impl Eq for SmallVec {} - -impl PartialEq for SmallVec { - fn eq(&self, other: &Self) -> bool { - self.as_slice() == other.as_slice() - } -} - -impl fmt::Debug for SmallVec { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.as_slice().fmt(f) - } -} - -impl Hash for SmallVec { - fn hash(&self, state: &mut H) { - self.len().hash(state); - Hash::hash_slice(self.as_slice(), state); - } -} - -#[cfg(feature = "serde")] -impl serde::Serialize for SmallVec { - fn serialize(&self, s: S) -> Result { - serde::Serialize::serialize(self.as_slice(), s) - } -} - -#[cfg(feature = "serde")] -impl<'de, T: serde::Deserialize<'de>> serde::Deserialize<'de> for SmallVec { - fn deserialize>(d: D) -> Result { - struct SmallVecVisitor { - marker: std::marker::PhantomData, - } - - impl<'de, T> serde::de::Visitor<'de> for SmallVecVisitor - where - T: serde::Deserialize<'de>, - { - type Value = SmallVec; - - fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - formatter.write_str("a sequence") - } - - fn visit_seq(self, mut seq: A) -> Result - where - A: serde::de::SeqAccess<'de>, - { - let mut values = SmallVec::empty(); - while let Some(value) = seq.next_element()? { - values.push(value); - } - Ok(values) - } - } - - let visitor = SmallVecVisitor { - marker: Default::default(), - }; - d.deserialize_seq(visitor) - } -} - -impl IntoIterator for SmallVec { - type Item = T; - type IntoIter = SmallVecIntoIter; - - fn into_iter(self) -> Self::IntoIter { - match self { - SmallVec::Empty => SmallVecIntoIter::Empty, - SmallVec::One(a) => SmallVecIntoIter::One(a.into_iter()), - SmallVec::Two(a) => SmallVecIntoIter::Two(a.into_iter()), - SmallVec::Flexible(v) => SmallVecIntoIter::Flexible(v.into_iter()), - } - } -} - -pub enum SmallVecIntoIter { - Empty, - One(<[T; 1] as IntoIterator>::IntoIter), - Two(<[T; 2] as IntoIterator>::IntoIter), - Flexible( as IntoIterator>::IntoIter), -} - -impl Iterator for SmallVecIntoIter { - type Item = T; - - fn next(&mut self) -> Option { - match self { - SmallVecIntoIter::Empty => None, - SmallVecIntoIter::One(it) => it.next(), - SmallVecIntoIter::Two(it) => it.next(), - SmallVecIntoIter::Flexible(it) => it.next(), - } - } -} - -// TESTS ####################################################################### - -#[cfg(test)] -pub mod tests { - use proptest::prelude::*; - - use super::*; - - proptest! { - #[test] - fn push_and_pop(commands: Vec>) { - let mut v = vec![]; - let mut sv = SmallVec::Empty; - for command in commands { - match command { - Some(i) => { - v.push(i); - sv.push(i); - } - None => { - assert_eq!(v.pop(), sv.pop()); - } - } - assert_eq!(v.as_slice(), sv.as_slice()); - } - } - } -} diff --git a/version-ranges/src/lib.rs b/version-ranges/src/lib.rs index 8b19881b..d3c17f0b 100644 --- a/version-ranges/src/lib.rs +++ b/version-ranges/src/lib.rs @@ -305,7 +305,7 @@ impl Ranges { /// See [`Ranges`] for the invariants checked. fn check_invariants(self) -> Self { if cfg!(debug_assertions) { - for p in self.segments.as_slice().windows(2) { + for p in self.segments.windows(2) { assert!(end_before_start_with_gap(&p[0].1, &p[1].0)); } for (s, e) in self.segments.iter() {