Skip to content

perf: optimize term::relation_with() #59

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/internal/incompatibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ use std::fmt;
use crate::internal::arena::{Arena, Id};
use crate::internal::small_map::SmallMap;
use crate::package::Package;
use crate::range::Range;
use crate::range::{self, Range};
use crate::report::{DefaultStringReporter, DerivationTree, Derived, External};
use crate::term::{self, Term};
use crate::term::Term;
use crate::version::Version;

/// An incompatibility is a set of terms for different packages
Expand Down Expand Up @@ -177,11 +177,11 @@ impl<P: Package, V: Version> Incompatibility<P, V> {
let mut relation = Relation::Satisfied;
for (package, incompat_term) in self.package_terms.iter() {
match terms(package).map(|term| incompat_term.relation_with(&term)) {
Some(term::Relation::Satisfied) => {}
Some(term::Relation::Contradicted) => {
Some(range::Relation::Satisfied) => {}
Some(range::Relation::Contradicted) => {
return Relation::Contradicted((package.clone(), incompat_term.clone()));
}
None | Some(term::Relation::Inconclusive) => {
None | Some(range::Relation::Inconclusive) => {
// If a package is not present, the intersection is the same as [Term::any].
// According to the rules of satisfactions, the relation would be inconclusive.
// It could also be satisfied if the incompatibility term was also [Term::any],
Expand Down
2 changes: 1 addition & 1 deletion src/internal/small_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ impl<T> SmallVec<T> {
}
}

pub fn iter(&self) -> impl Iterator<Item = &T> {
pub fn iter(&self) -> impl DoubleEndedIterator<Item = &T> {
self.as_slice().iter()
}
}
Expand Down
129 changes: 129 additions & 0 deletions src/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,88 @@ impl<V: Version> Range<V> {
}
}

/// Describe a relation between a set of terms S and another term t.
///
/// As a shorthand, we say that a term v
/// satisfies or contradicts a term t if {v} satisfies or contradicts it.
#[derive(Eq, PartialEq, Debug)]
pub(crate) enum Relation {
/// We say that a set of terms S "satisfies" a term t
/// if t must be true whenever every term in S is true.
Satisfied,
/// Conversely, S "contradicts" t if t must be false
/// whenever every term in S is true.
Contradicted,
/// If neither of these is true we say that S is "inconclusive" for t.
Inconclusive,
}

// Set operations.
impl<V: Version> Range<V> {
/// Compute the relation of two sets of versions.
pub(crate) fn relation(&self, other: &Self) -> Relation {
let mut state = None;
for s in self.segments.iter() {
match other.contains_interval(s) {
Relation::Satisfied => match state {
None | Some(Relation::Satisfied) => state = Some(Relation::Satisfied),
_ => return Relation::Inconclusive,
},
Relation::Inconclusive => return Relation::Inconclusive,
Relation::Contradicted => match state {
None | Some(Relation::Contradicted) => state = Some(Relation::Contradicted),
_ => return Relation::Inconclusive,
},
};
}
state.unwrap_or(Relation::Satisfied)
}

fn contains_interval(&self, other: &Interval<V>) -> Relation {
match other {
(o1, Some(o2)) => {
for seg in self.segments.iter() {
match seg {
(s1, Some(s2)) => {
if o2 < s1 {
break;
}
if s1 <= o1 && o2 <= s2 {
return Relation::Satisfied;
}
if !(s1 >= o2 || o1 >= s2) {
return Relation::Inconclusive;
}
}
(s1, None) => {
if s1 <= o1 {
return Relation::Satisfied;
}
if s1 < o2 {
return Relation::Inconclusive;
}
}
}
}
}
(o1, None) => {
if let Some((s1, None)) = self.segments.iter().rev().next() {
if s1 <= o1 {
return Relation::Satisfied;
}
}
if self.segments.iter().any(|seg| match seg {
(_, Some(s2)) => o1 < s2,
_ => true,
}) {
return Relation::Inconclusive;
}
}
};
Relation::Contradicted
}
}

// Other useful functions.
impl<V: Version> Range<V> {
/// Check if a range contains a given version.
Expand Down Expand Up @@ -381,6 +463,53 @@ pub mod tests {
assert_eq!(r1.intersection(&r2).contains(&version), r1.contains(&version) && r2.contains(&version));
}

// Testing relation -----------------------------------

#[test]
fn relation_with_any_is_satisfied(range in strategy()) {
prop_assert_eq!(range.relation(&Range::any()), Relation::Satisfied);
}

#[test]
fn relation_with_none_is_contradicted(range in strategy()) {
prop_assert_eq!(range.relation(&Range::none()), if range == Range::none() {
Relation::Satisfied
} else {
Relation::Contradicted
});
}

#[test]
fn relation_with_self_is_satisfied(range in strategy()) {
prop_assert_eq!(range.relation(&range), Relation::Satisfied);
}

#[test]
fn relation_matchs_intersection(r1 in strategy(), r2 in strategy()) {
let full_intersection = r1.intersection(&r2);
let by_intersection = if full_intersection == r2 {
Relation::Satisfied
} else if full_intersection == Range::none() {
Relation::Contradicted
} else {
Relation::Inconclusive
};
prop_assert_eq!(by_intersection, r2.relation(&r1));
}

#[test]
fn relation_contains_both(r1 in strategy(), r2 in strategy(), version in version_strat()) {
match r1.relation(&r2) {
Relation::Satisfied => {
prop_assert!(r2.contains(&version) || !r1.contains(&version));
}
Relation::Contradicted => {
prop_assert!(!(r1.contains(&version) && r2.contains(&version)));
}
_ => {}
}
}

// Testing union -----------------------------------

#[test]
Expand Down
45 changes: 20 additions & 25 deletions src/term.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//! A term is the fundamental unit of operation of the PubGrub algorithm.
//! It is a positive or negative expression regarding a set of versions.

use crate::range::Range;
use crate::range::{Range, Relation};
use crate::version::Version;
use std::fmt;

Expand All @@ -28,7 +28,7 @@ impl<V: Version> Term<V> {
}

/// A term that is never true.
pub(crate) fn empty() -> Self {
pub fn empty() -> Self {
Self::Positive(Range::none())
}

Expand Down Expand Up @@ -104,21 +104,6 @@ impl<V: Version> Term<V> {
}
}

/// Describe a relation between a set of terms S and another term t.
///
/// As a shorthand, we say that a term v
/// satisfies or contradicts a term t if {v} satisfies or contradicts it.
pub(crate) enum Relation {
/// We say that a set of terms S "satisfies" a term t
/// if t must be true whenever every term in S is true.
Satisfied,
/// Conversely, S "contradicts" t if t must be false
/// whenever every term in S is true.
Contradicted,
/// If neither of these is true we say that S is "inconclusive" for t.
Inconclusive,
}

/// Relation between terms.
impl<'a, V: 'a + Version> Term<V> {
/// Check if a set of terms satisfies this term.
Expand Down Expand Up @@ -148,14 +133,24 @@ impl<'a, V: 'a + Version> Term<V> {

/// Check if a set of terms satisfies or contradicts a given term.
/// Otherwise the relation is inconclusive.
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 {
Relation::Satisfied
} else if full_intersection == Self::empty() {
Relation::Contradicted
} else {
Relation::Inconclusive
pub(crate) fn relation_with(&self, other: &Term<V>) -> Relation {
match (self, other) {
(Self::Positive(r1), Self::Positive(r2)) => r2.relation(r1),
(Self::Positive(r1), Self::Negative(r2)) => match r2.negate().relation(r1) {
Relation::Satisfied => {
if r2 == &Range::any() {
Relation::Contradicted
} else {
Relation::Inconclusive
}
}
x => x,
},
(Self::Negative(r1), Self::Positive(r2)) => r2.relation(&r1.negate()),
(Self::Negative(r1), Self::Negative(r2)) => match r1.relation(r2) {
Relation::Contradicted => Relation::Inconclusive,
x => x,
},
}
}
}
Expand Down