From 4e6c07665cf30809c0402ce48da9add6cd49dcb3 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 16 Feb 2025 15:55:27 -0500 Subject: [PATCH] Remove clone from marker evaluation (#11562) ## Summary Something I noticed while looking at https://github.com/astral-sh/uv/issues/11548. --- crates/uv-resolver/src/graph_ops.rs | 12 +++++------ crates/uv-resolver/src/universal_marker.rs | 25 ++++++++++++---------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/crates/uv-resolver/src/graph_ops.rs b/crates/uv-resolver/src/graph_ops.rs index 9db0f5aad..62899dba5 100644 --- a/crates/uv-resolver/src/graph_ops.rs +++ b/crates/uv-resolver/src/graph_ops.rs @@ -1,10 +1,10 @@ +use std::collections::hash_map::Entry; + use petgraph::graph::{EdgeIndex, NodeIndex}; use petgraph::visit::EdgeRef; use petgraph::{Direction, Graph}; use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet}; -use std::collections::hash_map::Entry; -use uv_normalize::{ExtraName, GroupName, PackageName}; use uv_pep508::MarkerTree; use uv_pypi_types::{ConflictItem, Conflicts}; @@ -240,18 +240,18 @@ pub(crate) fn simplify_conflict_markers( if !inf.included { return None; } - Some((inf.item.package().clone(), inf.item.extra()?.clone())) + Some((inf.item.package(), inf.item.extra()?)) }) - .collect::>(); + .collect::>(); let groups = set .iter() .filter_map(|inf| { if !inf.included { return None; } - Some((inf.item.package().clone(), inf.item.group()?.clone())) + Some((inf.item.package(), inf.item.group()?)) }) - .collect::>(); + .collect::>(); graph[edge_index].conflict().evaluate(&extras, &groups) }); if !all_paths_satisfied { diff --git a/crates/uv-resolver/src/universal_marker.rs b/crates/uv-resolver/src/universal_marker.rs index 193a77285..e147720a3 100644 --- a/crates/uv-resolver/src/universal_marker.rs +++ b/crates/uv-resolver/src/universal_marker.rs @@ -421,11 +421,12 @@ impl ConflictMarker { /// Returns true if this conflict marker is satisfied by the given /// list of activated extras and groups. - pub(crate) fn evaluate( - self, - extras: &[(PackageName, ExtraName)], - groups: &[(PackageName, GroupName)], - ) -> bool { + pub(crate) fn evaluate(self, extras: &[(P, E)], groups: &[(P, G)]) -> bool + where + P: Borrow, + E: Borrow, + G: Borrow, + { static DUMMY: std::sync::LazyLock = std::sync::LazyLock::new(|| { MarkerEnvironment::try_from(MarkerEnvironmentBuilder { implementation_name: "", @@ -444,10 +445,10 @@ impl ConflictMarker { }); let extras = extras .iter() - .map(|(package, extra)| encode_package_extra(package, extra)); + .map(|(package, extra)| encode_package_extra(package.borrow(), extra.borrow())); let groups = groups .iter() - .map(|(package, group)| encode_package_group(package, group)); + .map(|(package, group)| encode_package_group(package.borrow(), group.borrow())); self.marker .evaluate(&DUMMY, &extras.chain(groups).collect::>()) } @@ -597,9 +598,10 @@ mod tests { .iter() .copied() .map(|name| (create_package("pkg"), create_extra(name))) - .collect::>(); + .collect::>(); + let groups = Vec::<(PackageName, GroupName)>::new(); assert!( - !cm.evaluate(&extras, &[]), + !cm.evaluate(&extras, &groups), "expected `{extra_names:?}` to evaluate to `false` in `{cm:?}`" ); } @@ -619,9 +621,10 @@ mod tests { .iter() .copied() .map(|name| (create_package("pkg"), create_extra(name))) - .collect::>(); + .collect::>(); + let groups = Vec::<(PackageName, GroupName)>::new(); assert!( - cm.evaluate(&extras, &[]), + cm.evaluate(&extras, &groups), "expected `{extra_names:?}` to evaluate to `true` in `{cm:?}`" ); }