Avoid distribution clones in requirements.txt graph (#7210)

This commit is contained in:
Charlie Marsh 2024-09-09 09:31:13 -04:00 committed by GitHub
parent b738b35910
commit dafa3596c5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 26 additions and 30 deletions

View file

@ -39,16 +39,16 @@ pub struct DisplayResolutionGraph<'a> {
} }
#[derive(Debug)] #[derive(Debug)]
enum DisplayResolutionGraphNode { enum DisplayResolutionGraphNode<'dist> {
Root, Root,
Dist(RequirementsTxtDist), Dist(RequirementsTxtDist<'dist>),
} }
impl DisplayResolutionGraphNode { impl DisplayResolutionGraphNode<'_> {
fn markers(&self) -> &MarkerTree { fn markers(&self) -> &MarkerTree {
match self { match self {
DisplayResolutionGraphNode::Root => &MarkerTree::TRUE, DisplayResolutionGraphNode::Root => &MarkerTree::TRUE,
DisplayResolutionGraphNode::Dist(dist) => &dist.markers, DisplayResolutionGraphNode::Dist(dist) => dist.markers,
} }
} }
} }
@ -191,7 +191,7 @@ impl std::fmt::Display for DisplayResolutionGraph<'_> {
// Display the distribution hashes, if any. // Display the distribution hashes, if any.
let mut has_hashes = false; let mut has_hashes = false;
if self.show_hashes { if self.show_hashes {
for hash in &node.hashes { for hash in node.hashes {
has_hashes = true; has_hashes = true;
line.push_str(" \\\n"); line.push_str(" \\\n");
line.push_str(" --hash="); line.push_str(" --hash=");
@ -313,10 +313,11 @@ pub enum AnnotationStyle {
} }
/// We don't need the edge markers anymore since we switched to propagated markers. /// We don't need the edge markers anymore since we switched to propagated markers.
type IntermediatePetGraph = type IntermediatePetGraph<'dist> =
petgraph::graph::Graph<DisplayResolutionGraphNode, (), petgraph::Directed>; petgraph::graph::Graph<DisplayResolutionGraphNode<'dist>, (), petgraph::Directed>;
type RequirementsTxtGraph = petgraph::graph::Graph<RequirementsTxtDist, (), petgraph::Directed>; type RequirementsTxtGraph<'dist> =
petgraph::graph::Graph<RequirementsTxtDist<'dist>, (), petgraph::Directed>;
/// Reduce the graph, such that all nodes for a single package are combined, regardless of /// Reduce the graph, such that all nodes for a single package are combined, regardless of
/// the extras. /// the extras.
@ -325,7 +326,7 @@ type RequirementsTxtGraph = petgraph::graph::Graph<RequirementsTxtDist, (), petg
/// node. /// node.
/// ///
/// We also remove the root node, to simplify the graph structure. /// We also remove the root node, to simplify the graph structure.
fn combine_extras(graph: &IntermediatePetGraph) -> RequirementsTxtGraph { fn combine_extras<'dist>(graph: &IntermediatePetGraph<'dist>) -> RequirementsTxtGraph<'dist> {
let mut next = RequirementsTxtGraph::with_capacity(graph.node_count(), graph.edge_count()); let mut next = RequirementsTxtGraph::with_capacity(graph.node_count(), graph.edge_count());
let mut inverse = FxHashMap::with_capacity_and_hasher(graph.node_count(), FxBuildHasher); let mut inverse = FxHashMap::with_capacity_and_hasher(graph.node_count(), FxBuildHasher);
@ -346,9 +347,6 @@ fn combine_extras(graph: &IntermediatePetGraph) -> RequirementsTxtGraph {
node.extras.extend(dist.extras.iter().cloned()); node.extras.extend(dist.extras.iter().cloned());
node.extras.sort_unstable(); node.extras.sort_unstable();
node.extras.dedup(); node.extras.dedup();
if dist.extras.is_empty() {
node.markers = dist.markers.clone();
}
} else { } else {
let version_id = dist.version_id(); let version_id = dist.version_id();
let dist = dist.clone(); let dist = dist.clone();

View file

@ -17,17 +17,15 @@ use crate::{
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
/// A pinned package with its resolved distribution and all the extras that were pinned for it. /// A pinned package with its resolved distribution and all the extras that were pinned for it.
pub(crate) struct RequirementsTxtDist { pub(crate) struct RequirementsTxtDist<'dist> {
pub(crate) dist: ResolvedDist, pub(crate) dist: &'dist ResolvedDist,
pub(crate) version: Version, pub(crate) version: &'dist Version,
pub(crate) hashes: &'dist [HashDigest],
pub(crate) markers: &'dist MarkerTree,
pub(crate) extras: Vec<ExtraName>, pub(crate) extras: Vec<ExtraName>,
pub(crate) hashes: Vec<HashDigest>,
/// Propagated markers that determine whether this package should be installed on the current
/// platform, without looking at which packages depend on it.
pub(crate) markers: MarkerTree,
} }
impl RequirementsTxtDist { impl<'dist> RequirementsTxtDist<'dist> {
/// Convert the [`RequirementsTxtDist`] to a requirement that adheres to the `requirements.txt` /// Convert the [`RequirementsTxtDist`] to a requirement that adheres to the `requirements.txt`
/// format. /// format.
/// ///
@ -153,29 +151,29 @@ impl RequirementsTxtDist {
if let VersionOrUrlRef::Url(url) = self.version_or_url() { if let VersionOrUrlRef::Url(url) = self.version_or_url() {
RequirementsTxtComparator::Name { RequirementsTxtComparator::Name {
name: self.name(), name: self.name(),
version: &self.version, version: self.version,
url: Some(url.verbatim()), url: Some(url.verbatim()),
} }
} else { } else {
RequirementsTxtComparator::Name { RequirementsTxtComparator::Name {
name: self.name(), name: self.name(),
version: &self.version, version: self.version,
url: None, url: None,
} }
} }
} }
pub(crate) fn from_annotated_dist(annotated: &AnnotatedDist) -> Self { pub(crate) fn from_annotated_dist(annotated: &'dist AnnotatedDist) -> Self {
Self { Self {
dist: annotated.dist.clone(), dist: &annotated.dist,
version: annotated.version.clone(), version: &annotated.version,
hashes: &annotated.hashes,
markers: &annotated.marker,
extras: if let Some(extra) = annotated.extra.clone() { extras: if let Some(extra) = annotated.extra.clone() {
vec![extra] vec![extra]
} else { } else {
vec![] vec![]
}, },
hashes: annotated.hashes.clone(),
markers: annotated.marker.clone(),
} }
} }
} }
@ -192,19 +190,19 @@ pub(crate) enum RequirementsTxtComparator<'a> {
}, },
} }
impl Name for RequirementsTxtDist { impl Name for RequirementsTxtDist<'_> {
fn name(&self) -> &PackageName { fn name(&self) -> &PackageName {
self.dist.name() self.dist.name()
} }
} }
impl DistributionMetadata for RequirementsTxtDist { impl DistributionMetadata for RequirementsTxtDist<'_> {
fn version_or_url(&self) -> VersionOrUrlRef { fn version_or_url(&self) -> VersionOrUrlRef {
self.dist.version_or_url() self.dist.version_or_url()
} }
} }
impl Display for RequirementsTxtDist { impl Display for RequirementsTxtDist<'_> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
Display::fmt(&self.dist, f) Display::fmt(&self.dist, f)
} }