Skip to content

Commit

Permalink
perf: specialize range operations for better performances (#177)
Browse files Browse the repository at this point in the history
* Use binary search for `Range::contains`

* Avoid cloning and dropping the term in prior_cause

* Simplify contains

* Specialize union code

* remove a redundant check and reuse helper

* Specialize is_disjoint

* simplify and add tests for is_disjoint

* Specialize subset_of

* simplify and add tests for subset_of

* Specialize range operations for better performance

* add tests and use in more places

* Fix clippy lints

* replace a complement with is_disjoint

* one fewer complement using intersection

* use the symmetry of set functions

---------

Co-authored-by: Jacob Finkelman <jfinkelm@amazon.com>
  • Loading branch information
konstin and Eh2406 committed Mar 13, 2024
1 parent 3549dc5 commit 0d80e9d
Show file tree
Hide file tree
Showing 6 changed files with 356 additions and 54 deletions.
7 changes: 5 additions & 2 deletions src/internal/incompatibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,11 @@ impl<P: Package, VS: VersionSet> Incompatibility<P, VS> {
incompatibility_store: &Arena<Self>,
) -> Self {
let kind = Kind::DerivedFrom(incompat, satisfier_cause);
let mut package_terms = incompatibility_store[incompat].package_terms.clone();
let t1 = package_terms.remove(package).unwrap();
// Optimization to avoid cloning and dropping t1
let (t1, mut package_terms) = incompatibility_store[incompat]
.package_terms
.split_one(package)
.unwrap();
let satisfier_cause_terms = &incompatibility_store[satisfier_cause].package_terms;
package_terms.merge(
satisfier_cause_terms.iter().filter(|(p, _)| p != &package),
Expand Down
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
43 changes: 43 additions & 0 deletions src/internal/small_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,49 @@ impl<K: PartialEq + Eq + Hash, V> SmallMap<K, V> {
}
};
}

/// Returns a reference to the value for one key and a copy of the map without the key.
///
/// This is an optimization over the following, where we only need a reference to `t1`. It
/// avoids cloning and then drop the ranges in each `prior_cause` call.
/// ```ignore
/// let mut package_terms = package_terms.clone();
// let t1 = package_terms.remove(package).unwrap();
/// ```
pub fn split_one(&self, key: &K) -> Option<(&V, Self)>
where
K: Clone,
V: Clone,
{
match self {
Self::Empty => None,
Self::One([(k, v)]) => {
if k == key {
Some((v, Self::Empty))
} else {
None
}
}
Self::Two([(k1, v1), (k2, v2)]) => {
if k1 == key {
Some((v1, Self::One([(k2.clone(), v2.clone())])))
} else if k2 == key {
Some((v2, Self::One([(k1.clone(), v1.clone())])))
} else {
None
}
}
Self::Flexible(map) => {
if let Some(value) = map.get(key) {
let mut map = map.clone();
map.remove(key).unwrap();
Some((value, Self::Flexible(map)))
} else {
None
}
}
}
}
}

impl<K: Clone + PartialEq + Eq + Hash, V: Clone> SmallMap<K, V> {
Expand Down

0 comments on commit 0d80e9d

Please sign in to comment.