Skip to content

Commit

Permalink
Faster range operations
Browse files Browse the repository at this point in the history
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
```
  • Loading branch information
konstin committed Jan 5, 2024
1 parent 4a48371 commit 8ddd053
Show file tree
Hide file tree
Showing 4 changed files with 221 additions and 7 deletions.
2 changes: 1 addition & 1 deletion src/internal/partial_solution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ impl<P: Package, VS: VersionSet> PackageAssignments<P, VS> {
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);
Expand Down
164 changes: 160 additions & 4 deletions src/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,10 +342,79 @@ fn group_adjacent_locations(
impl<V: Ord + Clone> Range<V> {
/// 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<Interval<V>> = 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.
Expand Down Expand Up @@ -494,6 +563,93 @@ impl<T: Debug + Display + Clone + Eq + Ord> VersionSet for Range<T> {
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 ######################################################################
Expand Down
54 changes: 52 additions & 2 deletions src/term.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<VS: VersionSet> {
/// For example, "1.0.0 <= v < 2.0.0" is a positive expression
/// that is evaluated true if a version is selected
Expand All @@ -19,6 +19,19 @@ pub enum Term<VS: VersionSet> {
Negative(VS),
}

impl<VS: VersionSet> PartialEq for Term<VS> {
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<VS: VersionSet> Eq for Term<VS> {}

/// Base methods.
impl<VS: VersionSet> Term<VS> {
/// A term that is always true.
Expand Down Expand Up @@ -98,10 +111,47 @@ impl<VS: VersionSet> Term<VS> {
}
}

/// 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.
Expand Down
8 changes: 8 additions & 0 deletions src/version_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

0 comments on commit 8ddd053

Please sign in to comment.