Skip to content

Commit

Permalink
perf: skip preemptively looking for conflicts befor a backtrack (#252)
Browse files Browse the repository at this point in the history
* perf: skip preemptively looking for conflicts befor a backtrack

* document the new optimization
  • Loading branch information
Eh2406 authored Aug 28, 2024
1 parent 597bf9e commit 4b4b444
Showing 1 changed file with 32 additions and 20 deletions.
52 changes: 32 additions & 20 deletions src/internal/partial_solution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ pub(crate) struct PartialSolution<DP: DependencyProvider> {
prioritized_potential_packages:
PriorityQueue<DP::P, DP::Priority, BuildHasherDefault<FxHasher>>,
changed_this_decision_level: usize,
has_ever_backtracked: bool,
}

impl<DP: DependencyProvider> Display for PartialSolution<DP> {
Expand Down Expand Up @@ -152,6 +153,7 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
package_assignments: FnvIndexMap::default(),
prioritized_potential_packages: PriorityQueue::default(),
changed_this_decision_level: 0,
has_ever_backtracked: false,
}
}

Expand Down Expand Up @@ -338,6 +340,7 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
// Throw away all stored priority levels, And mark that they all need to be recomputed.
self.prioritized_potential_packages.clear();
self.changed_this_decision_level = self.current_decision_level.0.saturating_sub(1) as usize;
self.has_ever_backtracked = true;
}

/// We can add the version to the partial solution as a decision
Expand All @@ -352,28 +355,37 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
new_incompatibilities: std::ops::Range<IncompId<DP::P, DP::VS, DP::M>>,
store: &Arena<Incompatibility<DP::P, DP::VS, DP::M>>,
) {
let exact = Term::exact(version.clone());
let not_satisfied = |incompat: &Incompatibility<DP::P, DP::VS, DP::M>| {
incompat.relation(|p| {
if p == &package {
Some(&exact)
} else {
self.term_intersection_for_package(p)
}
}) != Relation::Satisfied
};

// Check none of the dependencies (new_incompatibilities)
// would create a conflict (be satisfied).
if store[new_incompatibilities].iter().all(not_satisfied) {
log::info!("add_decision: {} @ {}", package, version);
if !self.has_ever_backtracked {
// Nothing has yet gone wrong during this resolution. This call is unlikely to be the first problem.
// So let's live with a little bit of risk and add the decision without checking the dependencies.
// The worst that can happen is we will have to do a full backtrack which only removes this one decision.
log::info!("add_decision: {package} @ {version} without checking dependencies");
self.add_decision(package, version);
} else {
log::info!(
"not adding {} @ {} because of its dependencies",
package,
version
);
// Check if any of the new dependencies preclude deciding on this crate version.
let exact = Term::exact(version.clone());
let not_satisfied = |incompat: &Incompatibility<DP::P, DP::VS, DP::M>| {
incompat.relation(|p| {
if p == &package {
Some(&exact)
} else {
self.term_intersection_for_package(p)
}
}) != Relation::Satisfied
};

// Check none of the dependencies (new_incompatibilities)
// would create a conflict (be satisfied).
if store[new_incompatibilities].iter().all(not_satisfied) {
log::info!("add_decision: {} @ {}", package, version);
self.add_decision(package, version);
} else {
log::info!(
"not adding {} @ {} because of its dependencies",
package,
version
);
}
}
}

Expand Down

0 comments on commit 4b4b444

Please sign in to comment.