Skip to content

Commit

Permalink
Review
Browse files Browse the repository at this point in the history
  • Loading branch information
konstin committed Dec 16, 2024
1 parent e8cf62e commit c92bf36
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 14 deletions.
21 changes: 17 additions & 4 deletions crates/uv-resolver/src/pubgrub/priority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,12 @@ impl PubGrubPriorities {
}
}

/// Mark a package as prioritized by setting it to [`PubGrubPriority::ConflictEarly`], if it
/// doesn't have a higher priority already.
///
/// Returns whether the priority was changed, i.e., it's the first time we hit this condition
/// for the package.
pub(crate) fn make_conflict_early(&mut self, package: &PubGrubPackage) -> bool {
pub(crate) fn mark_conflict_early(&mut self, package: &PubGrubPackage) -> bool {
let next = self.0.len();
let Some(name) = package.name_no_root() else {
// Not a correctness bug
Expand All @@ -117,7 +120,10 @@ impl PubGrubPriorities {
};
match self.0.entry(name.clone()) {
std::collections::hash_map::Entry::Occupied(mut entry) => {
if matches!(entry.get(), PubGrubPriority::ConflictEarly(_)) {
if matches!(
entry.get(),
PubGrubPriority::ConflictEarly(_) | PubGrubPriority::Singleton(_)
) {
// Already in the right category
return false;
};
Expand All @@ -132,7 +138,12 @@ impl PubGrubPriorities {
}
}

pub(crate) fn make_conflict_late(&mut self, package: &PubGrubPackage) -> bool {
/// Mark a package as prioritized by setting it to [`PubGrubPriority::ConflictLate`], if it
/// doesn't have a higher priority already.
///
/// Returns whether the priority was changed, i.e., it's the first time this package was
/// marked as conflicting above the threshold.
pub(crate) fn mark_conflict_late(&mut self, package: &PubGrubPackage) -> bool {
let next = self.0.len();
let Some(name) = package.name_no_root() else {
// Not a correctness bug
Expand All @@ -147,7 +158,9 @@ impl PubGrubPriorities {
// The ConflictEarly` match avoids infinite loops.
if matches!(
entry.get(),
PubGrubPriority::ConflictLate(_) | PubGrubPriority::ConflictEarly(_)
PubGrubPriority::ConflictLate(_)
| PubGrubPriority::ConflictEarly(_)
| PubGrubPriority::Singleton(_)
) {
// Already in the right category
return false;
Expand Down
18 changes: 9 additions & 9 deletions crates/uv-resolver/src/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -683,10 +683,10 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
///
/// To be called after unit propagation.
fn reprioritize_conflicts(state: &mut ForkState) {
for package in state.conflict_tracker.priotize.drain(..) {
for package in state.conflict_tracker.prioritize.drain(..) {
let changed = state
.priorities
.make_conflict_early(&state.pubgrub.package_store[package]);
.mark_conflict_early(&state.pubgrub.package_store[package]);
if changed {
debug!(
"Package {} has too many conflicts (affected), prioritizing",
Expand All @@ -701,10 +701,10 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
}
}

for package in state.conflict_tracker.depriotize.drain(..) {
for package in state.conflict_tracker.deprioritize.drain(..) {
let changed = state
.priorities
.make_conflict_late(&state.pubgrub.package_store[package]);
.mark_conflict_late(&state.pubgrub.package_store[package]);
if changed {
debug!(
"Package {} has too many conflicts (culprit), deprioritizing and backtracking",
Expand Down Expand Up @@ -2438,7 +2438,7 @@ impl ForkState {
.or_default();
*culprit_count += 1;
if *culprit_count == CONFLICT_THRESHOLD {
self.conflict_tracker.depriotize.push(incompatible);
self.conflict_tracker.deprioritize.push(incompatible);
}
}
// Don't track conflicts between a marker package and the main package, when the
Expand All @@ -2448,7 +2448,7 @@ impl ForkState {
let incompatibility = self.pubgrub.incompatibility_store[incompatibility]
.iter()
.map(|(package, _term)| {
format!("{:?}", self.pubgrub.package_store[package].clone(),)
format!("{}", self.pubgrub.package_store[package].clone(),)
})
.join(", ");
if let Some(version) = version {
Expand All @@ -2467,7 +2467,7 @@ impl ForkState {
let affected_count = self.conflict_tracker.affected.entry(self.next).or_default();
*affected_count += 1;
if *affected_count == CONFLICT_THRESHOLD {
self.conflict_tracker.priotize.push(self.next);
self.conflict_tracker.prioritize.push(self.next);
}
}
}
Expand Down Expand Up @@ -3384,11 +3384,11 @@ struct ConflictTracker {
/// Package(s) to be prioritized after the next unit propagation
///
/// Distilled from `affected` for fast checking in the hot loop.
priotize: Vec<Id<PubGrubPackage>>,
prioritize: Vec<Id<PubGrubPackage>>,
/// How often a package was decided earlier and caused another package to be discarded.
culprit: FxHashMap<Id<PubGrubPackage>, usize>,
/// Package(s) to be de-prioritized after the next unit propagation
///
/// Distilled from `culprit` for fast checking in the hot loop.
depriotize: Vec<Id<PubGrubPackage>>,
deprioritize: Vec<Id<PubGrubPackage>>,
}
2 changes: 1 addition & 1 deletion crates/uv/tests/it/lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14433,7 +14433,7 @@ fn lock_explicit_default_index() -> Result<()> {
DEBUG Adding transitive dependency for project==0.1.0: anyio*
DEBUG Searching for a compatible version of anyio (*)
DEBUG No compatible version found for: anyio
DEBUG Recording unit propagation conflict of anyio from incompatibility of (PubGrubPackage(Package { name: PackageName("project"), extra: None, dev: None, marker: true }))
DEBUG Recording unit propagation conflict of anyio from incompatibility of (project)
DEBUG Searching for a compatible version of project @ file://[TEMP_DIR]/ (<0.1.0 | >0.1.0)
DEBUG No compatible version found for: project
× No solution found when resolving dependencies:
Expand Down

0 comments on commit c92bf36

Please sign in to comment.