perf: precompute intersections (2x speedup) (#37)

* perf: precompute intersections (2x speedup)

* style: fix grammar in function names

Co-authored-by: Alex Tokarev <[email protected]>

Author: Eh2406

src/internal/incompatibility.rs

@@ -225,14 +225,10 @@ impl<P: Package, V: Version> Incompatibility<P, V> {
     }
 
     /// CF definition of Relation enum.
-    pub fn relation<T, I>(&self, terms: impl Fn(&P) -> I) -> Relation<P, V>
-    where
-        T: AsRef<Term<V>>,
-        I: Iterator<Item = T>,
-    {
+    pub fn relation(&self, mut terms: impl FnMut(&P) -> Term<V>) -> Relation<P, V> {
         let mut relation = Relation::Satisfied;
         for (package, incompat_term) in self.package_terms.iter() {
-            match incompat_term.relation_with(terms(package)) {
+            match incompat_term.relation_with(&terms(package)) {
                 term::Relation::Satisfied => {}
                 term::Relation::Contradicted => {
                     relation = Relation::Contradicted(package.clone(), incompat_term.clone());

src/internal/memory.rs

@@ -23,7 +23,8 @@ pub struct Memory<P: Package, V: Version> {
 #[derive(Clone)]
 struct PackageAssignments<V: Version> {
     decision: Option<(V, Term<V>)>,
-    derivations: Vec<Term<V>>,
+    derivations_intersected: Term<V>,
+    derivations_not_intersected_yet: Vec<Term<V>>,
 }
 
 impl<P: Package, V: Version> Memory<P, V> {
@@ -34,12 +35,12 @@ impl<P: Package, V: Version> Memory<P, V> {
         }
     }
 
-    /// Retrieve terms in memory related to package.
-    pub fn terms_for_package(&self, package: &P) -> impl Iterator<Item = &Term<V>> {
-        self.assignments.get(package).into_iter().flat_map(|a| {
-            let decision_iter = a.decision.iter().map(|(_, term)| term);
-            decision_iter.chain(a.derivations.iter())
-        })
+    /// Retrieve intersection of terms in memory related to package.
+    pub fn term_intersection_for_package(&mut self, package: &P) -> Term<V> {
+        match self.assignments.get_mut(package) {
+            None => Term::any(),
+            Some(pa) => pa.assignment_intersection(),
+        }
     }
 
     /// Building step of a Memory from a given assignment.
@@ -72,7 +73,7 @@ impl<P: Package, V: Version> Memory<P, V> {
             .assignments
             .entry(package)
             .or_insert(PackageAssignments::new());
-        pa.derivations.push(term);
+        pa.derivations_not_intersected_yet.push(term);
     }
 
     /// Extract all packages that may potentially be picked next
@@ -81,26 +82,10 @@ impl<P: Package, V: Version> Memory<P, V> {
     /// selected version (no "decision")
     /// and if it contains at least one positive derivation term
     /// in the partial solution.
-    pub fn potential_packages(&self) -> impl Iterator<Item = (&P, &[Term<V>])> {
+    pub fn potential_packages(&mut self) -> impl Iterator<Item = (&P, &Term<V>)> {
         self.assignments
-            .iter()
-            .filter_map(|(p, pa)| Self::potential_package_filter(p, pa))
-    }
-
-    fn potential_package_filter<'a, 'b>(
-        package: &'a P,
-        package_assignments: &'b PackageAssignments<V>,
-    ) -> Option<(&'a P, &'b [Term<V>])> {
-        if &package_assignments.decision == &None
-            && package_assignments
-                .derivations
-                .iter()
-                .any(|t| t.is_positive())
-        {
-            Some((package, package_assignments.derivations.as_slice()))
-        } else {
-            None
-        }
+            .iter_mut()
+            .filter_map(|(p, pa)| pa.potential_package_filter(p))
     }
 
     /// If a partial solution has, for every positive derivation,
@@ -125,7 +110,8 @@ impl<V: Version> PackageAssignments<V> {
     fn new() -> Self {
         Self {
             decision: None,
-            derivations: Vec::new(),
+            derivations_intersected: Term::any(),
+            derivations_not_intersected_yet: Vec::new(),
         }
     }
 
@@ -134,8 +120,55 @@ impl<V: Version> PackageAssignments<V> {
     /// it's a total solution and version solving has succeeded.
     fn is_valid(&self) -> bool {
         match self.decision {
-            None => self.derivations.iter().all(|t| t.is_negative()),
+            None => {
+                self.derivations_intersected.is_negative()
+                    && self
+                        .derivations_not_intersected_yet
+                        .iter()
+                        .all(|t| t.is_negative())
+            }
             Some(_) => true,
         }
     }
+
+    /// Returns intersection of all assignments (decision included).
+    /// Mutates itself to store the intersection result.
+    fn assignment_intersection(&mut self) -> Term<V> {
+        self.derivation_intersection();
+        match &self.decision {
+            None => self.derivations_intersected.clone(),
+            Some((_, decision_term)) => decision_term.intersection(&self.derivations_intersected),
+        }
+    }
+
+    /// Returns intersection of all derivation terms.
+    /// Mutates itself to store the intersection result.
+    fn derivation_intersection(&mut self) -> &Term<V> {
+        for derivation in self.derivations_not_intersected_yet.iter() {
+            self.derivations_intersected = self.derivations_intersected.intersection(derivation);
+        }
+        self.derivations_not_intersected_yet.clear();
+        &self.derivations_intersected
+    }
+
+    /// A package is a potential pick if there isn't an already
+    /// selected version (no "decision")
+    /// and if it contains at least one positive derivation term
+    /// in the partial solution.
+    fn potential_package_filter<'a, 'b, P: Package>(
+        &'a mut self,
+        package: &'b P,
+    ) -> Option<(&'b P, &'a Term<V>)> {
+        if self.decision == None
+            && (self.derivations_intersected.is_positive()
+                || self
+                    .derivations_not_intersected_yet
+                    .iter()
+                    .any(|t| t.is_positive()))
+        {
+            Some((package, self.derivation_intersection()))
+        } else {
+            None
+        }
+    }
 }

src/internal/partial_solution.rs

@@ -80,8 +80,7 @@ impl<P: Package, V: Version> PartialSolution<P, V> {
             .rposition(|(l, _)| *l == decision_level)
             .unwrap_or(self.history.len() - 1);
         *self = Self::from_assignments(
-            self.history
-                .to_owned()
+            std::mem::take(&mut self.history)
                 .into_iter()
                 .take(pos + 1)
                 .map(|(_, a)| a),
@@ -103,16 +102,12 @@ impl<P: Package, V: Version> PartialSolution<P, V> {
     /// This tends to find conflicts earlier if any exist,
     /// since these packages will run out of versions to try more quickly.
     pub fn pick_package(
-        &self,
+        &mut self,
         dependency_provider: &impl DependencyProvider<P, V>,
     ) -> Result<Option<(P, Term<V>)>, PubGrubError<P, V>> {
         let mut out: Option<(P, Term<V>)> = None;
         let mut min_key = usize::MAX;
-        for (p, term) in self
-            .memory
-            .potential_packages()
-            .map(|(p, all_terms)| (p, Term::intersect_all(all_terms.iter())))
-        {
+        for (p, term) in self.memory.potential_packages() {
             let key = dependency_provider
                 .list_available_versions(p)
                 .map_err(|err| PubGrubError::ErrorRetrievingVersions {
@@ -124,7 +119,7 @@ impl<P: Package, V: Version> PartialSolution<P, V> {
                 .count();
             if key < min_key {
                 min_key = key;
-                out = Some((p.clone(), term));
+                out = Some((p.clone(), term.clone()));
             }
         }
         Ok(out)
@@ -167,15 +162,15 @@ impl<P: Package, V: Version> PartialSolution<P, V> {
         self.memory.remove_decision(last_assignment.package());
     }
 
-    fn satisfies_any_of(&self, incompatibilities: &[Incompatibility<P, V>]) -> bool {
+    fn satisfies_any_of(&mut self, incompatibilities: &[Incompatibility<P, V>]) -> bool {
         incompatibilities
             .iter()
             .any(|incompat| self.relation(incompat) == Relation::Satisfied)
     }
 
     /// Check if the terms in the partial solution satisfy the incompatibility.
-    pub fn relation(&self, incompat: &Incompatibility<P, V>) -> Relation<P, V> {
-        incompat.relation(|package| self.memory.terms_for_package(package))
+    pub fn relation(&mut self, incompat: &Incompatibility<P, V>) -> Relation<P, V> {
+        incompat.relation(|package| self.memory.term_intersection_for_package(package))
     }
 
     /// Find satisfier and previous satisfier decision level.

src/term.rs

@@ -92,12 +92,6 @@ impl<V: Version> Term<V> {
         (self.negate().intersection(&other.negate())).negate()
     }
 
-    /// Compute the intersection of multiple terms.
-    /// Return None if the iterator is empty.
-    pub(crate) fn intersect_all<T: AsRef<Term<V>>>(all_terms: impl Iterator<Item = T>) -> Term<V> {
-        all_terms.fold(Self::any(), |acc, term| acc.intersection(term.as_ref()))
-    }
-
     /// Indicate if this term is a subset of another term.
     /// Just like for sets, we say that t1 is a subset of t2
     /// if and only if t1 ∩ t2 = t1.
@@ -131,8 +125,8 @@ impl<'a, V: 'a + Version> Term<V> {
     /// It turns out that this can also be expressed with set operations:
     ///    S satisfies t if and only if  ⋂ S ⊆ t
     #[cfg(test)]
-    fn satisfied_by(&self, terms: impl Iterator<Item = &'a Term<V>>) -> bool {
-        Self::intersect_all(terms).subset_of(self)
+    fn satisfied_by(&self, terms_intersection: &Term<V>) -> bool {
+        terms_intersection.subset_of(self)
     }
 
     /// Check if a set of terms contradicts this term.
@@ -144,19 +138,15 @@ impl<'a, V: 'a + Version> Term<V> {
     ///    S contradicts t if and only if ⋂ S is disjoint with t
     ///    S contradicts t if and only if  (⋂ S) ⋂ t = ∅
     #[cfg(test)]
-    fn contradicted_by(&self, terms: impl Iterator<Item = &'a Term<V>>) -> bool {
-        Self::intersect_all(terms).intersection(self) == Self::empty()
+    fn contradicted_by(&self, terms_intersection: &Term<V>) -> bool {
+        terms_intersection.intersection(self) == Self::empty()
     }
 
     /// Check if a set of terms satisfies or contradicts a given term.
     /// Otherwise the relation is inconclusive.
-    pub(crate) fn relation_with<T: AsRef<Term<V>>>(
-        &self,
-        other_terms: impl Iterator<Item = T>,
-    ) -> Relation {
-        let others_intersection = Self::intersect_all(other_terms);
-        let full_intersection = self.intersection(&others_intersection);
-        if full_intersection == others_intersection {
+    pub(crate) fn relation_with(&self, other_terms_intersection: &Term<V>) -> Relation {
+        let full_intersection = self.intersection(other_terms_intersection.as_ref());
+        if &full_intersection == other_terms_intersection.as_ref() {
             Relation::Satisfied
         } else if full_intersection == Self::empty() {
             Relation::Contradicted
@@ -203,13 +193,13 @@ pub mod tests {
         // Testing relation --------------------------------
 
         #[test]
-        fn relation_with(term in strategy(), set in prop::collection::vec(strategy(), 0..3)) {
-            match term.relation_with(set.iter()) {
-                Relation::Satisfied => assert!(term.satisfied_by(set.iter())),
-                Relation::Contradicted => assert!(term.contradicted_by(set.iter())),
+        fn relation_with(term1 in strategy(), term2 in strategy()) {
+            match term1.relation_with(&term2) {
+                Relation::Satisfied => assert!(term1.satisfied_by(&term2)),
+                Relation::Contradicted => assert!(term1.contradicted_by(&term2)),
                 Relation::Inconclusive => {
-                    assert!(!term.satisfied_by(set.iter()));
-                    assert!(!term.contradicted_by(set.iter()));
+                    assert!(!term1.satisfied_by(&term2));
+                    assert!(!term1.contradicted_by(&term2));
                 }
             }
         }