Skip to content

Commit 0d80e9d

Browse files
konstinEh2406
andauthored
perf: specialize range operations for better performances (pubgrub-rs#177)
* 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>
1 parent 3549dc5 commit 0d80e9d

File tree

6 files changed

+356
-54
lines changed

6 files changed

+356
-54
lines changed

src/internal/incompatibility.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,11 @@ impl<P: Package, VS: VersionSet> Incompatibility<P, VS> {
173173
incompatibility_store: &Arena<Self>,
174174
) -> Self {
175175
let kind = Kind::DerivedFrom(incompat, satisfier_cause);
176-
let mut package_terms = incompatibility_store[incompat].package_terms.clone();
177-
let t1 = package_terms.remove(package).unwrap();
176+
// Optimization to avoid cloning and dropping t1
177+
let (t1, mut package_terms) = incompatibility_store[incompat]
178+
.package_terms
179+
.split_one(package)
180+
.unwrap();
178181
let satisfier_cause_terms = &incompatibility_store[satisfier_cause].package_terms;
179182
package_terms.merge(
180183
satisfier_cause_terms.iter().filter(|(p, _)| p != &package),

src/internal/partial_solution.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -501,7 +501,7 @@ impl<P: Package, VS: VersionSet> PackageAssignments<P, VS> {
501501
let idx = self
502502
.dated_derivations
503503
.as_slice()
504-
.partition_point(|dd| dd.accumulated_intersection.intersection(start_term) != empty);
504+
.partition_point(|dd| !dd.accumulated_intersection.is_disjoint(start_term));
505505
if let Some(dd) = self.dated_derivations.get(idx) {
506506
debug_assert_eq!(dd.accumulated_intersection.intersection(start_term), empty);
507507
return (Some(dd.cause), dd.global_index, dd.decision_level);

src/internal/small_map.rs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,49 @@ impl<K: PartialEq + Eq + Hash, V> SmallMap<K, V> {
9999
}
100100
};
101101
}
102+
103+
/// Returns a reference to the value for one key and a copy of the map without the key.
104+
///
105+
/// This is an optimization over the following, where we only need a reference to `t1`. It
106+
/// avoids cloning and then drop the ranges in each `prior_cause` call.
107+
/// ```ignore
108+
/// let mut package_terms = package_terms.clone();
109+
// let t1 = package_terms.remove(package).unwrap();
110+
/// ```
111+
pub fn split_one(&self, key: &K) -> Option<(&V, Self)>
112+
where
113+
K: Clone,
114+
V: Clone,
115+
{
116+
match self {
117+
Self::Empty => None,
118+
Self::One([(k, v)]) => {
119+
if k == key {
120+
Some((v, Self::Empty))
121+
} else {
122+
None
123+
}
124+
}
125+
Self::Two([(k1, v1), (k2, v2)]) => {
126+
if k1 == key {
127+
Some((v1, Self::One([(k2.clone(), v2.clone())])))
128+
} else if k2 == key {
129+
Some((v2, Self::One([(k1.clone(), v1.clone())])))
130+
} else {
131+
None
132+
}
133+
}
134+
Self::Flexible(map) => {
135+
if let Some(value) = map.get(key) {
136+
let mut map = map.clone();
137+
map.remove(key).unwrap();
138+
Some((value, Self::Flexible(map)))
139+
} else {
140+
None
141+
}
142+
}
143+
}
144+
}
102145
}
103146

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

0 commit comments

Comments
 (0)