Reduce AssignmentsIntersection size

Author: x-hgg-x

src/internal/partial_solution.rs

@@ -133,19 +133,41 @@ impl<M: Eq + Clone + Debug + Display> Display for DatedDerivation<M> {
     }
 }
 
-#[derive(Clone, Debug)]
-enum AssignmentsIntersection {
-    Decision((u32, VersionIndex, Term)),
-    Derivations(Term),
+#[derive(Clone, Debug, Default)]
+struct AssignmentsIntersection {
+    term: Term,
+    is_decision: bool,
+    decision_global_index: u32,
+    decision_version_index: VersionIndex,
+}
+
+impl AssignmentsIntersection {
+    fn decision(global_index: u32, version_index: VersionIndex) -> Self {
+        Self {
+            term: Term::exact(version_index),
+            is_decision: true,
+            decision_global_index: global_index,
+            decision_version_index: version_index,
+        }
+    }
+
+    fn derivations(term: Term) -> Self {
+        Self {
+            term,
+            ..Default::default()
+        }
+    }
 }
 
 impl Display for AssignmentsIntersection {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            Self::Decision((lvl, version_index, _)) => {
-                write!(f, "Decision: level {}, v = {}", lvl, version_index.get())
-            }
-            Self::Derivations(term) => write!(f, "Derivations term: {}", term),
+        if self.is_decision {
+            let lvl = self.decision_global_index;
+            let version = self.decision_version_index.get();
+            write!(f, "Decision: level {lvl}, v = {version}")
+        } else {
+            let term = self.term;
+            write!(f, "Derivations term: {term}")
         }
     }
 }
@@ -192,25 +214,26 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
     pub(crate) fn add_decision(&mut self, package_id: PackageId, version_index: VersionIndex) {
         // Check that add_decision is never used in the wrong context.
         if cfg!(debug_assertions) {
-            match self.package_assignments.get_mut(&package_id) {
-                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"),
-                    // Cannot be called if the versions is not contained in the terms' intersection.
-                    AssignmentsIntersection::Derivations(term) => {
-                        debug_assert!(
-                            term.contains(version_index),
-                            "{} was expected to be contained in {}",
-                            version_index.get(),
-                            term,
-                        )
-                    }
-                },
-            }
+            let pa = self
+                .package_assignments
+                .get_mut(&package_id)
+                .expect("Derivations must already exist");
+            // Cannot be called when a decision has already been taken.
+            assert!(
+                !pa.assignments_intersection.is_decision,
+                "Already existing decision",
+            );
+            // Cannot be called if the versions is not contained in the terms' intersection.
+            let term = pa.assignments_intersection.term;
+            assert!(
+                term.contains(version_index),
+                "{} was expected to be contained in {}",
+                version_index.get(),
+                term,
+            );
             assert_eq!(
                 self.changed_this_decision_level,
-                self.package_assignments.len()
+                self.package_assignments.len(),
             );
         }
         let new_idx = self.current_decision_level.0 as usize;
@@ -220,11 +243,8 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
             .get_full_mut(&package_id)
             .expect("Derivations must already exist");
         pa.highest_decision_level = self.current_decision_level;
-        pa.assignments_intersection = AssignmentsIntersection::Decision((
-            self.next_global_index,
-            version_index,
-            Term::exact(version_index),
-        ));
+        pa.assignments_intersection =
+            AssignmentsIntersection::decision(self.next_global_index, version_index);
         // 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);
@@ -253,21 +273,19 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
                 let idx = occupied.index();
                 let pa = occupied.get_mut();
                 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(_) => {
-                        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;
-                        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);
-                        }
-                    }
+                // Check that add_derivation is never called in the wrong context.
+                assert!(
+                    !pa.assignments_intersection.is_decision,
+                    "add_derivation should not be called after a decision",
+                );
+                let t = &mut pa.assignments_intersection.term;
+                *t = t.intersection(dated_derivation.accumulated_intersection);
+                dated_derivation.accumulated_intersection = *t;
+                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);
                 }
                 pa.dated_derivations.push(dated_derivation);
             }
@@ -281,7 +299,7 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
                     smallest_decision_level: self.current_decision_level,
                     highest_decision_level: self.current_decision_level,
                     dated_derivations: SmallVec::One([dated_derivation]),
-                    assignments_intersection: AssignmentsIntersection::Derivations(term),
+                    assignments_intersection: AssignmentsIntersection::derivations(term),
                 });
             }
         }
@@ -327,11 +345,12 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
             .package_assignments
             .iter()
             .take(self.current_decision_level.0 as usize)
-            .map(|(&pid, pa)| match pa.assignments_intersection {
-                AssignmentsIntersection::Decision((_, version_index, _)) => (pid, version_index),
-                AssignmentsIntersection::Derivations(_) => {
-                    panic!("Derivations in the Decision part")
-                }
+            .map(|(&pid, pa)| {
+                assert!(
+                    pa.assignments_intersection.is_decision,
+                    "Derivations in the Decision part",
+                );
+                (pid, pa.assignments_intersection.decision_version_index)
             })
             .collect::<Map<_, _>>();
 
@@ -374,7 +393,7 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
 
                 // Reset the assignments intersection.
                 pa.assignments_intersection =
-                    AssignmentsIntersection::Derivations(last.accumulated_intersection);
+                    AssignmentsIntersection::derivations(last.accumulated_intersection);
                 true
             }
         });
@@ -462,7 +481,7 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
     pub(crate) fn term_intersection_for_package(&self, package_id: PackageId) -> Option<Term> {
         self.package_assignments
             .get(&package_id)
-            .map(|pa| pa.assignments_intersection.term())
+            .map(|pa| pa.assignments_intersection.term)
     }
 
     /// Figure out if the satisfier and previous satisfier are of different decision levels.
@@ -544,10 +563,11 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
         let accum_term = if let &Some(cause) = satisfier_cause {
             store[cause].get(satisfier_package_id).unwrap().negate()
         } else {
-            match satisfier_pa.assignments_intersection {
-                AssignmentsIntersection::Derivations(_) => panic!("must be a decision"),
-                AssignmentsIntersection::Decision((_, _, term)) => term,
-            }
+            assert!(
+                satisfier_pa.assignments_intersection.is_decision,
+                "must be a decision",
+            );
+            satisfier_pa.assignments_intersection.term
         };
 
         let incompat_term = incompat
@@ -589,49 +609,37 @@ impl<M: Eq + Clone + Debug + Display> PackageAssignments<M> {
             debug_assert_eq!(dd.accumulated_intersection.intersection(start_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).
-        match &self.assignments_intersection {
-            AssignmentsIntersection::Decision((global_index, _, _)) => {
-                (None, *global_index, self.highest_decision_level)
-            }
-            AssignmentsIntersection::Derivations(accumulated_intersection) => {
-                let p = package_store.pkg(package_id).unwrap();
-                unreachable!(
-                    "while processing package {p}: \
-                    accum_term = {accumulated_intersection} has overlap with incompat_term = {start_term}, \
-                    which means the last assignment should have been a decision, \
-                    but instead it was a derivation. This shouldn't be possible! \
-                    (Maybe your Version ordering is broken?)"
-                )
-            }
+        // If it wasn't found in the derivations, it must be the decision which is last (if called in the right context).
+        let AssignmentsIntersection {
+            term,
+            is_decision,
+            decision_global_index,
+            ..
+        } = self.assignments_intersection;
+        if !is_decision {
+            let p = package_store.pkg(package_id).unwrap();
+            unreachable!(
+                "while processing package {p}: \
+                accum_term = {term} has overlap with incompat_term = {start_term}, \
+                which means the last assignment should have been a decision, \
+                but instead it was a derivation. This shouldn't be possible! \
+                (Maybe your Version ordering is broken?)"
+            )
         }
+        (None, decision_global_index, self.highest_decision_level)
     }
 }
 
 impl AssignmentsIntersection {
-    /// Returns the term intersection of all assignments (decision included).
-    fn term(&self) -> Term {
-        match *self {
-            Self::Decision((_, _, term)) => term,
-            Self::Derivations(term) => term,
-        }
-    }
-
     /// 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(&self, package_id: PackageId) -> Option<(PackageId, VersionSet)> {
-        match self {
-            Self::Decision(_) => None,
-            Self::Derivations(term_intersection) => {
-                if term_intersection.is_positive() {
-                    Some((package_id, term_intersection.unwrap_positive()))
-                } else {
-                    None
-                }
-            }
+        if !self.is_decision && self.term.is_positive() {
+            Some((package_id, self.term.unwrap_positive()))
+        } else {
+            None
         }
     }
 }

src/term.rs

@@ -18,7 +18,7 @@ use crate::{VersionIndex, VersionSet};
 /// Specifically, `Term::positive(VersionSet::empty())` means that there was a conflict
 /// (we need to select a version for the package but can't pick any),
 /// while `Term::negative(VersionSet::full())` would mean it is fine as long as we don't select the package.
-#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
+#[derive(Debug, Default, Copy, Clone, Eq, PartialEq, Hash)]
 #[repr(transparent)]
 pub struct Term(u64);