From 8ddd053ff9552f13a524f51baf5fd5c396bba7b5 Mon Sep 17 00:00:00 2001 From: konstin Date: Sat, 6 Jan 2024 00:19:51 +0100 Subject: [PATCH] Faster range operations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I replaced some range operations implemented by their mathematical definitions with optimized versions. ``` hyperfine --warmup 1 --runs 3 "target/profiling/main scripts/requirements/bio_embeddings.txt" "target/profiling/branch scripts/requirements/bio_embeddings.txt" Benchmark 1: target/profiling/main scripts/requirements/bio_embeddings.txt Time (mean ± σ): 13.062 s ± 0.620 s [User: 12.626 s, System: 0.344 s] Range (min … max): 12.394 s … 13.619 s 3 runs Benchmark 2: target/profiling/branch scripts/requirements/bio_embeddings.txt Time (mean ± σ): 5.184 s ± 0.405 s [User: 4.732 s, System: 0.391 s] Range (min … max): 4.901 s … 5.648 s 3 runs Summary target/profiling/branch scripts/requirements/bio_embeddings.txt ran 2.52 ± 0.23 times faster than target/profiling/main scripts/requirements/bio_embeddings.txt ``` --- src/internal/partial_solution.rs | 2 +- src/range.rs | 164 ++++++++++++++++++++++++++++++- src/term.rs | 54 +++++++++- src/version_set.rs | 8 ++ 4 files changed, 221 insertions(+), 7 deletions(-) diff --git a/src/internal/partial_solution.rs b/src/internal/partial_solution.rs index cf1d24db..9a4c8841 100644 --- a/src/internal/partial_solution.rs +++ b/src/internal/partial_solution.rs @@ -501,7 +501,7 @@ impl PackageAssignments { let idx = self .dated_derivations .as_slice() - .partition_point(|dd| dd.accumulated_intersection.intersection(start_term) != empty); + .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); return (Some(dd.cause), dd.global_index, dd.decision_level); diff --git a/src/range.rs b/src/range.rs index ab88a0ca..2c6c8c37 100644 --- a/src/range.rs +++ b/src/range.rs @@ -342,10 +342,79 @@ fn group_adjacent_locations( impl Range { /// Computes the union of this `Range` and another. pub fn union(&self, other: &Self) -> Self { - self.complement() - .intersection(&other.complement()) - .complement() - .check_invariants() + let end_before_start_exclusive = |end: &Bound<_>, start: &Bound<_>| match (end, start) { + (_, Unbounded) => false, + (Unbounded, _) => false, + (Included(left), Included(right)) => left < right, + (Included(left), Excluded(right)) => left < right, + (Excluded(left), Included(right)) => left < right, + (Excluded(left), Excluded(right)) => left <= right, + }; + + let mut output: SmallVec> = SmallVec::empty(); + let mut accumulator: Option<(&Bound<_>, &Bound<_>)> = None; + let mut left_iter = self.segments.iter().peekable(); + let mut right_iter = other.segments.iter().peekable(); + loop { + let smaller_interval = match (left_iter.peek(), right_iter.peek()) { + (Some((left_start, left_end)), Some((right_start, right_end))) => { + let left_start_is_smaller = match (left_start, right_start) { + (Unbounded, _) => true, + (_, Unbounded) => false, + (Included(l), Included(r)) => l <= r, + (Excluded(l), Excluded(r)) => l <= r, + (Included(l), Excluded(r)) => l <= r, + (Excluded(l), Included(r)) => l < r, + }; + + if left_start_is_smaller { + left_iter.next(); + (left_start, left_end) + } else { + right_iter.next(); + (right_start, right_end) + } + } + (Some((left_start, left_end)), None) => { + left_iter.next(); + (left_start, left_end) + } + (None, Some((right_start, right_end))) => { + right_iter.next(); + (right_start, right_end) + } + (None, None) => break, + }; + + if let Some(accumulator_) = accumulator { + if end_before_start_exclusive(accumulator_.1, smaller_interval.0) { + output.push((accumulator_.0.clone(), accumulator_.1.clone())); + accumulator = Some(smaller_interval); + } else { + let accumulator_end = match (accumulator_.1, smaller_interval.1) { + (_, Unbounded) | (Unbounded, _) => &Unbounded, + (Included(l), Excluded(r) | Included(r)) if l == r => accumulator_.1, + (Excluded(l) | Included(l), Included(r)) if l == r => smaller_interval.1, + (Included(l) | Excluded(l), Included(r) | Excluded(r)) => { + if l > r { + accumulator_.1 + } else { + smaller_interval.1 + } + } + }; + accumulator = Some((accumulator_.0, accumulator_end)); + } + } else { + accumulator = Some(smaller_interval) + } + } + + if let Some(accumulator) = accumulator { + output.push((accumulator.0.clone(), accumulator.1.clone())); + } + + Self { segments: output }.check_invariants() } /// Computes the intersection of two sets of versions. @@ -494,6 +563,93 @@ impl VersionSet for Range { fn union(&self, other: &Self) -> Self { Range::union(self, other) } + + fn is_disjoint(&self, other: &Self) -> bool { + let end_before_start = |end, start| match (end, start) { + (_, Unbounded) => false, + (Unbounded, _) => false, + (Included(left), Included(right)) => left < right, + (Included(left), Excluded(right)) => left <= right, + (Excluded(left), Included(right)) => left <= right, + (Excluded(left), Excluded(right)) => left <= right, + }; + + // The operation is symmetric + let mut left_iter = self.segments.iter().peekable(); + let mut right_iter = other.segments.iter().peekable(); + + while let (Some(left), Some(right)) = (left_iter.peek(), right_iter.peek()) { + if end_before_start(left.end_bound(), right.start_bound()) { + left_iter.next(); + } else if end_before_start(right.end_bound(), left.start_bound()) { + right_iter.next(); + } else { + return false; + } + } + + // The remaining element(s) can't intersect anymore + true + } + + fn subset_of(&self, other: &Self) -> bool { + let mut containing_iter = other.segments.iter(); + let mut subset_iter = self.segments.iter(); + let Some(mut containing_elem) = containing_iter.next() else { + // As long as we have subset elements, we need containing elements + return subset_iter.next().is_none(); + }; + + let end_before_start = |end, start| match (end, start) { + (_, Unbounded) => false, + (Unbounded, _) => false, + (Included(left), Included(right)) => left < right, + (Included(left), Excluded(right)) => left <= right, + (Excluded(left), Included(right)) => left <= right, + (Excluded(left), Excluded(right)) => left <= right, + }; + + while let Some(subset_elem) = subset_iter.next() { + // Check if the current containing element ends before the subset element. + // There needs to be another containing element for our subset element in this case. + while end_before_start(containing_elem.end_bound(), subset_elem.start_bound()) { + if let Some(containing_elem_) = containing_iter.next() { + containing_elem = containing_elem_; + } else { + return false; + }; + } + + let start_contained = match (containing_elem.start_bound(), subset_elem.start_bound()) { + (Unbounded, _) => true, + (Included(_) | Excluded(_), Unbounded) => false, + // This is the only case where the subset is bound is "wider" than containing bound ... + (Excluded(left), Included(right)) => left < right, + // ... while in the other cases they can share the point + (Included(left), Included(right)) => left <= right, + (Included(left), Excluded(right)) => left <= right, + (Excluded(left), Excluded(right)) => left <= right, + }; + + let end_contained = match (subset_elem.end_bound(), containing_elem.end_bound()) { + (_, Unbounded) => true, + (Unbounded, Included(_) | Excluded(_)) => false, + // This is the only case where the subset is bound is "wider" than containing bound ... + (Included(left), Excluded(right)) => left < right, + // ... while in the other cases they can share the point + (Included(left), Included(right)) => left <= right, + (Excluded(left), Included(right)) => left <= right, + (Excluded(left), Excluded(right)) => left <= right, + }; + + if !(start_contained && end_contained) { + // This subset element is not contained + return false; + } + } + + true + } } // REPORT ###################################################################### diff --git a/src/term.rs b/src/term.rs index 2974da62..e3ce58b2 100644 --- a/src/term.rs +++ b/src/term.rs @@ -7,7 +7,7 @@ use crate::version_set::VersionSet; use std::fmt::{self, Display}; /// A positive or negative expression regarding a set of versions. -#[derive(Debug, Clone, Eq, PartialEq)] +#[derive(Debug, Clone)] pub enum Term { /// For example, "1.0.0 <= v < 2.0.0" is a positive expression /// that is evaluated true if a version is selected @@ -19,6 +19,19 @@ pub enum Term { Negative(VS), } +impl PartialEq for Term { + fn eq(&self, other: &Self) -> bool { + match (self, other) { + (Self::Positive(r1), Self::Positive(r2)) => r1 == r2, + (Self::Positive(r1), Self::Negative(r2)) => r1 == &r2.complement(), + (Self::Negative(r1), Self::Positive(r2)) => &r1.complement() == r2, + (Self::Negative(r1), Self::Negative(r2)) => r1 == r2, + } + } +} + +impl Eq for Term {} + /// Base methods. impl Term { /// A term that is always true. @@ -98,10 +111,47 @@ impl Term { } } + /// Check whether two terms are mutually exclusive. + /// + /// An optimization for the native implementation of checking whether the intersection of two sets is empty. + pub(crate) fn is_disjoint(&self, other: &Self) -> bool { + match (self, other) { + (Self::Positive(r1), Self::Positive(r2)) => r1.is_disjoint(r2), + (Self::Negative(r1), Self::Negative(r2)) => r1.union(r2) == VS::empty().complement(), + // If the positive term is a subset of the negative term, it lies fully in the region that the negative + // term excludes. Inversely, if there is a region outside the negative, they overlap in this region. + (Self::Positive(r1), Self::Negative(r2)) => r1.subset_of(r2), + (Self::Negative(r1), Self::Positive(r2)) => r2.subset_of(r1), + } + } + /// Compute the union of two terms. /// If at least one term is negative, the union is also negative. pub(crate) fn union(&self, other: &Self) -> Self { - (self.negate().intersection(&other.negate())).negate() + match (self, other) { + (Self::Positive(r1), Self::Positive(r2)) => Self::Positive(r1.union(r2)), + (Self::Positive(r1), Self::Negative(r2)) => Self::Positive(r1.union(&r2.complement())), + (Self::Negative(r1), Self::Positive(r2)) => Self::Positive(r1.complement().union(r2)), + (Self::Negative(r1), Self::Negative(r2)) => Self::Negative(r1.intersection(r2)), + } + } + + /// TODO + pub(crate) fn completes(&self, other: &Self) -> bool { + let a = self.union(other) == Self::any(); + let b = match (self, other) { + (Self::Positive(r1), Self::Positive(r2)) => Self::Positive(r1.union(r2)) == Self::any(), + (Self::Positive(r1), Self::Negative(r2)) => r2.subset_of(r1), + (Self::Negative(r1), Self::Positive(r2)) => r1.subset_of(r2), + (Self::Negative(r1), Self::Negative(r2)) => { + Self::Negative(r1.intersection(r2)) == Self::any() + } + }; + if a != b { + dbg!(self, other, a, b); + panic!() + } + return a; } /// Indicate if this term is a subset of another term. diff --git a/src/version_set.rs b/src/version_set.rs index 501ec700..c53d1394 100644 --- a/src/version_set.rs +++ b/src/version_set.rs @@ -57,4 +57,12 @@ pub trait VersionSet: Debug + Display + Clone + Eq { .intersection(&other.complement()) .complement() } + + fn is_disjoint(&self, other: &Self) -> bool { + self.intersection(other) == Self::empty() + } + + fn subset_of(&self, other: &Self) -> bool { + self == &self.intersection(other) + } }