From a9ba74259bbaa20b9b19036ae319c9595a8aba77 Mon Sep 17 00:00:00 2001 From: Jacob Finkelman Date: Wed, 18 Dec 2024 22:10:32 +0000 Subject: [PATCH 1/2] when priorities are equal do breadth first search --- src/internal/partial_solution.rs | 5 +++-- src/solver.rs | 5 +++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/internal/partial_solution.rs b/src/internal/partial_solution.rs index 2d71e598..ba9750e2 100644 --- a/src/internal/partial_solution.rs +++ b/src/internal/partial_solution.rs @@ -3,6 +3,7 @@ //! A Memory acts like a structured partial solution //! where terms are regrouped by package in a [Map](crate::type_aliases::Map). +use std::cmp::Reverse; use std::fmt::{Debug, Display}; use std::hash::BuildHasherDefault; @@ -67,7 +68,7 @@ pub(crate) struct PartialSolution { /// /// The max heap allows quickly `pop`ing the highest priority package. prioritized_potential_packages: - PriorityQueue, DP::Priority, BuildHasherDefault>, + PriorityQueue, (DP::Priority, Reverse), BuildHasherDefault>, /// Whether we have never backtracked, to enable fast path optimizations. has_ever_backtracked: bool, } @@ -330,7 +331,7 @@ impl PartialSolution { .filter_map(|(&p, pa)| pa.assignments_intersection.potential_package_filter(p)) .for_each(|(p, r)| { let priority = prioritizer(p, r); - prioritized_potential_packages.push(p, priority); + prioritized_potential_packages.push(p, (priority, Reverse(p.into_raw() as u32))); }); self.prioritize_decision_level = self.package_assignments.len(); prioritized_potential_packages.pop().map(|(p, _)| p) diff --git a/src/solver.rs b/src/solver.rs index e6ba6249..d7802656 100644 --- a/src/solver.rs +++ b/src/solver.rs @@ -323,6 +323,11 @@ pub trait DependencyProvider { /// > since these packages will run out of versions to try more quickly. /// > But there's likely room for improvement in these heuristics. /// + /// The `package_conflicts_counts` argument provides access to some other heuristics that + /// are production users have found useful. Although the exact meaning/efficacy of those arguments may change. + /// + /// If two packages have the same priority, PubGrub will biased toward a breadth first search. + /// /// Note: the resolver may call this even when the range has not changed, /// if it is more efficient for the resolvers internal data structures. fn prioritize( From bda2b8178205362ebf3e35abaf68077cc038ef14 Mon Sep 17 00:00:00 2001 From: konstin Date: Thu, 19 Dec 2024 09:43:34 +0100 Subject: [PATCH 2/2] Lints and docs --- src/internal/partial_solution.rs | 4 ++++ src/solver.rs | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/internal/partial_solution.rs b/src/internal/partial_solution.rs index ba9750e2..f1345634 100644 --- a/src/internal/partial_solution.rs +++ b/src/internal/partial_solution.rs @@ -67,6 +67,10 @@ pub(crate) struct PartialSolution { /// The undecided packages order by their `Priority`. /// /// The max heap allows quickly `pop`ing the highest priority package. + /// + /// The `Reverse` is the discovery order of packages used as tiebreaker. Its order is that + /// of a breadth-first search. + #[allow(clippy::type_complexity)] prioritized_potential_packages: PriorityQueue, (DP::Priority, Reverse), BuildHasherDefault>, /// Whether we have never backtracked, to enable fast path optimizations. diff --git a/src/solver.rs b/src/solver.rs index d7802656..7c4ed0f8 100644 --- a/src/solver.rs +++ b/src/solver.rs @@ -325,9 +325,9 @@ pub trait DependencyProvider { /// /// The `package_conflicts_counts` argument provides access to some other heuristics that /// are production users have found useful. Although the exact meaning/efficacy of those arguments may change. - /// + /// /// If two packages have the same priority, PubGrub will biased toward a breadth first search. - /// + /// /// Note: the resolver may call this even when the range has not changed, /// if it is more efficient for the resolvers internal data structures. fn prioritize(