From 1a34f10aac994165ae681f696c7ac1246e9a1432 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Tue, 3 Nov 2020 11:20:39 -0500 Subject: [PATCH] perf: more efficient relation_with --- src/internal/incompatibility.rs | 10 +-- src/internal/small_vec.rs | 2 +- src/range.rs | 129 ++++++++++++++++++++++++++++++++ src/term.rs | 45 +++++------ 4 files changed, 155 insertions(+), 31 deletions(-) diff --git a/src/internal/incompatibility.rs b/src/internal/incompatibility.rs index 5bb4c577..f046309f 100644 --- a/src/internal/incompatibility.rs +++ b/src/internal/incompatibility.rs @@ -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 @@ -177,11 +177,11 @@ impl Incompatibility { 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], diff --git a/src/internal/small_vec.rs b/src/internal/small_vec.rs index 98c0dd1e..cba45f0a 100644 --- a/src/internal/small_vec.rs +++ b/src/internal/small_vec.rs @@ -38,7 +38,7 @@ impl SmallVec { } } - pub fn iter(&self) -> impl Iterator { + pub fn iter(&self) -> impl DoubleEndedIterator { self.as_slice().iter() } } diff --git a/src/range.rs b/src/range.rs index cc4ea432..aa859ac8 100644 --- a/src/range.rs +++ b/src/range.rs @@ -237,6 +237,88 @@ impl Range { } } +/// 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 Range { + /// 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) -> 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 Range { /// Check if a range contains a given version. @@ -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] diff --git a/src/term.rs b/src/term.rs index 63c4da7d..cea52b33 100644 --- a/src/term.rs +++ b/src/term.rs @@ -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; @@ -28,7 +28,7 @@ impl Term { } /// A term that is never true. - pub(crate) fn empty() -> Self { + pub fn empty() -> Self { Self::Positive(Range::none()) } @@ -104,21 +104,6 @@ impl Term { } } -/// 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 { /// Check if a set of terms satisfies this term. @@ -148,14 +133,24 @@ impl<'a, V: 'a + Version> Term { /// 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) -> 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) -> 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, + }, } } }