Skip to content

Commit 4b4b444

Browse files
authored
perf: skip preemptively looking for conflicts befor a backtrack (#252)
* perf: skip preemptively looking for conflicts befor a backtrack * document the new optimization
1 parent 597bf9e commit 4b4b444

File tree

1 file changed

+32
-20
lines changed

1 file changed

+32
-20
lines changed

src/internal/partial_solution.rs

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ pub(crate) struct PartialSolution<DP: DependencyProvider> {
4747
prioritized_potential_packages:
4848
PriorityQueue<DP::P, DP::Priority, BuildHasherDefault<FxHasher>>,
4949
changed_this_decision_level: usize,
50+
has_ever_backtracked: bool,
5051
}
5152

5253
impl<DP: DependencyProvider> Display for PartialSolution<DP> {
@@ -152,6 +153,7 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
152153
package_assignments: FnvIndexMap::default(),
153154
prioritized_potential_packages: PriorityQueue::default(),
154155
changed_this_decision_level: 0,
156+
has_ever_backtracked: false,
155157
}
156158
}
157159

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

343346
/// We can add the version to the partial solution as a decision
@@ -352,28 +355,37 @@ impl<DP: DependencyProvider> PartialSolution<DP> {
352355
new_incompatibilities: std::ops::Range<IncompId<DP::P, DP::VS, DP::M>>,
353356
store: &Arena<Incompatibility<DP::P, DP::VS, DP::M>>,
354357
) {
355-
let exact = Term::exact(version.clone());
356-
let not_satisfied = |incompat: &Incompatibility<DP::P, DP::VS, DP::M>| {
357-
incompat.relation(|p| {
358-
if p == &package {
359-
Some(&exact)
360-
} else {
361-
self.term_intersection_for_package(p)
362-
}
363-
}) != Relation::Satisfied
364-
};
365-
366-
// Check none of the dependencies (new_incompatibilities)
367-
// would create a conflict (be satisfied).
368-
if store[new_incompatibilities].iter().all(not_satisfied) {
369-
log::info!("add_decision: {} @ {}", package, version);
358+
if !self.has_ever_backtracked {
359+
// Nothing has yet gone wrong during this resolution. This call is unlikely to be the first problem.
360+
// So let's live with a little bit of risk and add the decision without checking the dependencies.
361+
// The worst that can happen is we will have to do a full backtrack which only removes this one decision.
362+
log::info!("add_decision: {package} @ {version} without checking dependencies");
370363
self.add_decision(package, version);
371364
} else {
372-
log::info!(
373-
"not adding {} @ {} because of its dependencies",
374-
package,
375-
version
376-
);
365+
// Check if any of the new dependencies preclude deciding on this crate version.
366+
let exact = Term::exact(version.clone());
367+
let not_satisfied = |incompat: &Incompatibility<DP::P, DP::VS, DP::M>| {
368+
incompat.relation(|p| {
369+
if p == &package {
370+
Some(&exact)
371+
} else {
372+
self.term_intersection_for_package(p)
373+
}
374+
}) != Relation::Satisfied
375+
};
376+
377+
// Check none of the dependencies (new_incompatibilities)
378+
// would create a conflict (be satisfied).
379+
if store[new_incompatibilities].iter().all(not_satisfied) {
380+
log::info!("add_decision: {} @ {}", package, version);
381+
self.add_decision(package, version);
382+
} else {
383+
log::info!(
384+
"not adding {} @ {} because of its dependencies",
385+
package,
386+
version
387+
);
388+
}
377389
}
378390
}
379391

0 commit comments

Comments
 (0)