Skip to content

Commit

Permalink
Use a single buffer for hints on resolver errors (#7497)
Browse files Browse the repository at this point in the history
This changes the structure of the hints generator in the resolver when
encountering solution errors, so that it re-uses a single output buffer
owned by the caller.
It avoids repeated allocations of a temporary buffer within each
recursive function call.
  • Loading branch information
lucab authored Sep 18, 2024
1 parent 039b683 commit 67cfb4a
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 14 deletions.
8 changes: 6 additions & 2 deletions crates/uv-resolver/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::collections::{BTreeMap, BTreeSet};
use std::fmt::Formatter;
use std::sync::Arc;

use indexmap::IndexSet;
use pubgrub::{DefaultStringReporter, DerivationTree, Derived, External, Range, Reporter};
use rustc_hash::FxHashMap;

Expand Down Expand Up @@ -248,15 +249,18 @@ impl std::fmt::Display for NoSolutionError {
write!(f, "{report}")?;

// Include any additional hints.
for hint in formatter.hints(
let mut additional_hints = IndexSet::default();
formatter.generate_hints(
&self.error,
&self.selector,
&self.index_locations,
&self.unavailable_packages,
&self.incomplete_packages,
&self.fork_urls,
&self.markers,
) {
&mut additional_hints,
);
for hint in additional_hints {
write!(f, "\n\n{hint}")?;
}

Expand Down
30 changes: 18 additions & 12 deletions crates/uv-resolver/src/pubgrub/report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,7 @@ impl PubGrubReportFormatter<'_> {
///
/// The [`PubGrubHints`] help users resolve errors by providing additional context or modifying
/// their requirements.
pub(crate) fn hints(
pub(crate) fn generate_hints(
&self,
derivation_tree: &ErrorTree,
selector: &CandidateSelector,
Expand All @@ -508,8 +508,8 @@ impl PubGrubReportFormatter<'_> {
incomplete_packages: &FxHashMap<PackageName, BTreeMap<Version, IncompletePackage>>,
fork_urls: &ForkUrls,
markers: &ResolverMarkers,
) -> IndexSet<PubGrubHint> {
let mut hints = IndexSet::default();
output_hints: &mut IndexSet<PubGrubHint>,
) {
match derivation_tree {
DerivationTree::External(
External::Custom(package, set, _) | External::NoVersions(package, set),
Expand All @@ -518,7 +518,12 @@ impl PubGrubReportFormatter<'_> {
// Check for no versions due to pre-release options.
if !fork_urls.contains_key(name) {
self.prerelease_available_hint(
package, name, set, selector, markers, &mut hints,
package,
name,
set,
selector,
markers,
output_hints,
);
}
}
Expand All @@ -532,7 +537,7 @@ impl PubGrubReportFormatter<'_> {
index_locations,
unavailable_packages,
incomplete_packages,
&mut hints,
output_hints,
);
}
}
Expand All @@ -547,7 +552,7 @@ impl PubGrubReportFormatter<'_> {
&**dependency,
PubGrubPackageInner::Python(PubGrubPython::Target)
) {
hints.insert(PubGrubHint::RequiresPython {
output_hints.insert(PubGrubHint::RequiresPython {
source: self.python_requirement.source(),
requires_python: self.python_requirement.target().clone(),
package: package.clone(),
Expand All @@ -558,27 +563,28 @@ impl PubGrubReportFormatter<'_> {
}
DerivationTree::External(External::NotRoot(..)) => {}
DerivationTree::Derived(derived) => {
hints.extend(self.hints(
self.generate_hints(
&derived.cause1,
selector,
index_locations,
unavailable_packages,
incomplete_packages,
fork_urls,
markers,
));
hints.extend(self.hints(
output_hints,
);
self.generate_hints(
&derived.cause2,
selector,
index_locations,
unavailable_packages,
incomplete_packages,
fork_urls,
markers,
));
output_hints,
);
}
}
hints
};
}

fn index_hints(
Expand Down

0 comments on commit 67cfb4a

Please sign in to comment.