Skip to content

Commit 8375f23

Browse files
committed
perf: more efficient relation_with
1 parent 9c7d0fb commit 8375f23

File tree

5 files changed

+157
-34
lines changed

5 files changed

+157
-34
lines changed

Diff for: src/internal/core.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -179,8 +179,7 @@ impl<P: Package, V: Version> State<P, V> {
179179
fn find_shared_ids(&self, incompat: &Incompatibility<P, V>) -> Set<usize> {
180180
let mut all_ids = Set::new();
181181
let mut shared_ids = Set::new();
182-
let mut stack = Vec::new();
183-
stack.push(incompat);
182+
let mut stack = vec![incompat];
184183
while let Some(i) = stack.pop() {
185184
if let Some((id1, id2)) = i.causes() {
186185
if all_ids.contains(&i.id) {

Diff for: src/internal/incompatibility.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@ use std::fmt;
88

99
use crate::internal::small_map::SmallMap;
1010
use crate::package::Package;
11-
use crate::range::Range;
11+
use crate::range::{self, Range};
1212
use crate::report::{DefaultStringReporter, DerivationTree, Derived, External};
1313
use crate::solver::DependencyConstraints;
14-
use crate::term::{self, Term};
14+
use crate::term::Term;
1515
use crate::version::Version;
1616

1717
/// An incompatibility is a set of terms for different packages
@@ -193,11 +193,11 @@ impl<P: Package, V: Version> Incompatibility<P, V> {
193193
let mut relation = Relation::Satisfied;
194194
for (package, incompat_term) in self.package_terms.iter() {
195195
match terms(package).map(|term| incompat_term.relation_with(&term)) {
196-
Some(term::Relation::Satisfied) => {}
197-
Some(term::Relation::Contradicted) => {
196+
Some(range::Relation::Satisfied) => {}
197+
Some(range::Relation::Contradicted) => {
198198
return Relation::Contradicted((package.clone(), incompat_term.clone()));
199199
}
200-
None | Some(term::Relation::Inconclusive) => {
200+
None | Some(range::Relation::Inconclusive) => {
201201
// If a package is not present, the intersection is the same as [Term::any].
202202
// According to the rules of satisfactions, the relation would be inconclusive.
203203
// It could also be satisfied if the incompatibility term was also [Term::any],
@@ -324,7 +324,7 @@ pub mod tests {
324324

325325
let i2 = Incompatibility {
326326
id: 0,
327-
package_terms: SmallMap::Two([("p2", t2.clone()), ("p3", t3.clone())]),
327+
package_terms: SmallMap::Two([("p2", t2), ("p3", t3.clone())]),
328328
kind: Kind::DerivedFrom(0,0)
329329
};
330330

Diff for: src/internal/small_vec.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ impl<T> SmallVec<T> {
3838
}
3939
}
4040

41-
pub fn iter(&self) -> impl Iterator<Item = &T> {
41+
pub fn iter(&self) -> impl DoubleEndedIterator<Item = &T> {
4242
self.as_slice().iter()
4343
}
4444
}

Diff for: src/range.rs

+129
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,88 @@ impl<V: Version> Range<V> {
237237
}
238238
}
239239

240+
/// Describe a relation between a set of terms S and another term t.
241+
///
242+
/// As a shorthand, we say that a term v
243+
/// satisfies or contradicts a term t if {v} satisfies or contradicts it.
244+
#[derive(Eq, PartialEq, Debug)]
245+
pub(crate) enum Relation {
246+
/// We say that a set of terms S "satisfies" a term t
247+
/// if t must be true whenever every term in S is true.
248+
Satisfied,
249+
/// Conversely, S "contradicts" t if t must be false
250+
/// whenever every term in S is true.
251+
Contradicted,
252+
/// If neither of these is true we say that S is "inconclusive" for t.
253+
Inconclusive,
254+
}
255+
256+
// Set operations.
257+
impl<V: Version> Range<V> {
258+
/// Compute the relation of two sets of versions.
259+
pub(crate) fn relation(&self, other: &Self) -> Relation {
260+
let mut state = None;
261+
for s in self.segments.iter() {
262+
match other.contains_interval(s) {
263+
Relation::Satisfied => match state {
264+
None | Some(Relation::Satisfied) => state = Some(Relation::Satisfied),
265+
_ => return Relation::Inconclusive,
266+
},
267+
Relation::Inconclusive => return Relation::Inconclusive,
268+
Relation::Contradicted => match state {
269+
None | Some(Relation::Contradicted) => state = Some(Relation::Contradicted),
270+
_ => return Relation::Inconclusive,
271+
},
272+
};
273+
}
274+
state.unwrap_or(Relation::Satisfied)
275+
}
276+
277+
fn contains_interval(&self, other: &Interval<V>) -> Relation {
278+
match other {
279+
(o1, Some(o2)) => {
280+
for seg in self.segments.iter() {
281+
match seg {
282+
(s1, Some(s2)) => {
283+
if o2 < s1 {
284+
break;
285+
}
286+
if s1 <= o1 && o2 <= s2 {
287+
return Relation::Satisfied;
288+
}
289+
if !(s1 >= o2 || o1 >= s2) {
290+
return Relation::Inconclusive;
291+
}
292+
}
293+
(s1, None) => {
294+
if s1 <= o1 {
295+
return Relation::Satisfied;
296+
}
297+
if s1 < o2 {
298+
return Relation::Inconclusive;
299+
}
300+
}
301+
}
302+
}
303+
}
304+
(o1, None) => {
305+
if let Some((s1, None)) = self.segments.iter().rev().next() {
306+
if s1 <= o1 {
307+
return Relation::Satisfied;
308+
}
309+
}
310+
if self.segments.iter().any(|seg| match seg {
311+
(_, Some(s2)) => o1 < s2,
312+
_ => true,
313+
}) {
314+
return Relation::Inconclusive;
315+
}
316+
}
317+
};
318+
Relation::Contradicted
319+
}
320+
}
321+
240322
// Other useful functions.
241323
impl<V: Version> Range<V> {
242324
/// Check if a range contains a given version.
@@ -381,6 +463,53 @@ pub mod tests {
381463
assert_eq!(r1.intersection(&r2).contains(&version), r1.contains(&version) && r2.contains(&version));
382464
}
383465

466+
// Testing relation -----------------------------------
467+
468+
#[test]
469+
fn relation_with_any_is_satisfied(range in strategy()) {
470+
prop_assert_eq!(range.relation(&Range::any()), Relation::Satisfied);
471+
}
472+
473+
#[test]
474+
fn relation_with_none_is_contradicted(range in strategy()) {
475+
prop_assert_eq!(range.relation(&Range::none()), if range == Range::none() {
476+
Relation::Satisfied
477+
} else {
478+
Relation::Contradicted
479+
});
480+
}
481+
482+
#[test]
483+
fn relation_with_self_is_satisfied(range in strategy()) {
484+
prop_assert_eq!(range.relation(&range), Relation::Satisfied);
485+
}
486+
487+
#[test]
488+
fn relation_matchs_intersection(r1 in strategy(), r2 in strategy()) {
489+
let full_intersection = r1.intersection(&r2);
490+
let by_intersection = if full_intersection == r2 {
491+
Relation::Satisfied
492+
} else if full_intersection == Range::none() {
493+
Relation::Contradicted
494+
} else {
495+
Relation::Inconclusive
496+
};
497+
prop_assert_eq!(by_intersection, r2.relation(&r1));
498+
}
499+
500+
#[test]
501+
fn relation_contains_both(r1 in strategy(), r2 in strategy(), version in version_strat()) {
502+
match r1.relation(&r2) {
503+
Relation::Satisfied => {
504+
prop_assert!(r2.contains(&version) || !r1.contains(&version));
505+
}
506+
Relation::Contradicted => {
507+
prop_assert!(!(r1.contains(&version) && r2.contains(&version)));
508+
}
509+
_ => {}
510+
}
511+
}
512+
384513
// Testing union -----------------------------------
385514

386515
#[test]

Diff for: src/term.rs

+20-25
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
//! A term is the fundamental unit of operation of the PubGrub algorithm.
44
//! It is a positive or negative expression regarding a set of versions.
55
6-
use crate::range::Range;
6+
use crate::range::{Range, Relation};
77
use crate::version::Version;
88
use std::fmt;
99

@@ -28,7 +28,7 @@ impl<V: Version> Term<V> {
2828
}
2929

3030
/// A term that is never true.
31-
pub(crate) fn empty() -> Self {
31+
pub fn empty() -> Self {
3232
Self::Positive(Range::none())
3333
}
3434

@@ -104,21 +104,6 @@ impl<V: Version> Term<V> {
104104
}
105105
}
106106

107-
/// Describe a relation between a set of terms S and another term t.
108-
///
109-
/// As a shorthand, we say that a term v
110-
/// satisfies or contradicts a term t if {v} satisfies or contradicts it.
111-
pub(crate) enum Relation {
112-
/// We say that a set of terms S "satisfies" a term t
113-
/// if t must be true whenever every term in S is true.
114-
Satisfied,
115-
/// Conversely, S "contradicts" t if t must be false
116-
/// whenever every term in S is true.
117-
Contradicted,
118-
/// If neither of these is true we say that S is "inconclusive" for t.
119-
Inconclusive,
120-
}
121-
122107
/// Relation between terms.
123108
impl<'a, V: 'a + Version> Term<V> {
124109
/// Check if a set of terms satisfies this term.
@@ -148,14 +133,24 @@ impl<'a, V: 'a + Version> Term<V> {
148133

149134
/// Check if a set of terms satisfies or contradicts a given term.
150135
/// Otherwise the relation is inconclusive.
151-
pub(crate) fn relation_with(&self, other_terms_intersection: &Term<V>) -> Relation {
152-
let full_intersection = self.intersection(other_terms_intersection.as_ref());
153-
if &full_intersection == other_terms_intersection {
154-
Relation::Satisfied
155-
} else if full_intersection == Self::empty() {
156-
Relation::Contradicted
157-
} else {
158-
Relation::Inconclusive
136+
pub(crate) fn relation_with(&self, other: &Term<V>) -> Relation {
137+
match (self, other) {
138+
(Self::Positive(r1), Self::Positive(r2)) => r2.relation(r1),
139+
(Self::Positive(r1), Self::Negative(r2)) => match r2.negate().relation(r1) {
140+
Relation::Satisfied => {
141+
if r2 == &Range::any() {
142+
Relation::Contradicted
143+
} else {
144+
Relation::Inconclusive
145+
}
146+
}
147+
x => x,
148+
},
149+
(Self::Negative(r1), Self::Positive(r2)) => r2.relation(&r1.negate()),
150+
(Self::Negative(r1), Self::Negative(r2)) => match r1.relation(r2) {
151+
Relation::Contradicted => Relation::Inconclusive,
152+
x => x,
153+
},
159154
}
160155
}
161156
}

0 commit comments

Comments
 (0)