Skip to content
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

perf: specialize range operations for better performances #177

Merged
merged 15 commits into from
Mar 13, 2024
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));
Eh2406 marked this conversation as resolved.
Show resolved Hide resolved
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)))
mpizenberg marked this conversation as resolved.
Show resolved Hide resolved
} else {
None
}
}
}
}
}

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