From cd2fb6fd60737090175a13d643dbef598b16622f Mon Sep 17 00:00:00 2001 From: konsti Date: Fri, 19 Jan 2024 10:44:41 +0100 Subject: [PATCH] Box `PrioritizedDistribution` (#948) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On top of https://github.com/astral-sh/puffin/pull/947, we can also box `PrioritizedDistribution`. In a simple benchmark, this seems to slightly improve performance when comparing only this commit to main, even though the benchmark is too noisy to establish significance: ``` $ hyperfine --warmup 30 --runs 300 "target/profiling/main-dev resolve meine_stadt_transparent" "target/profiling/puffin-dev resolve meine_stadt_transparent" Benchmark 1: target/profiling/main-dev resolve meine_stadt_transparent Time (mean ± σ): 83.6 ms ± 2.0 ms [User: 77.7 ms, System: 20.0 ms] Range (min … max): 81.4 ms … 98.2 ms 300 runs Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options. Benchmark 2: target/profiling/puffin-dev resolve meine_stadt_transparent Time (mean ± σ): 80.8 ms ± 2.2 ms [User: 75.4 ms, System: 19.5 ms] Range (min … max): 78.6 ms … 98.6 ms 300 runs Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options. Summary target/profiling/puffin-dev resolve meine_stadt_transparent ran 1.03 ± 0.04 times faster than target/profiling/main-dev resolve meine_stadt_transparent ``` The effect on type sizes however is considerable ([downstack PR](https://gist.github.com/konstin/38e6c774db541db46d61f1d4ea6b498f) vs. [this PR](https://gist.github.com/konstin/003a77fe7d7d246b0d535e3fc843cb36)): ```patch --- branch.txt 2024-01-17 14:26:01.826085176 +0100 +++ boxed-prioritized-dist.txt 2024-01-17 14:25:57.101900963 +0100 @@ -1,19 +1,3 @@ -9264 alloc::collections::btree::node::InternalNode align=8 - 9168 data - 96 edges - -9264 alloc::collections::btree::node::InternalNode align=8 - 9168 data - 96 edges - -9168 alloc::collections::btree::node::LeafNode align=8 - 9064 vals - 88 keys - -9168 alloc::collections::btree::node::LeafNode align=8 - 9064 vals - 88 keys - 8992 tokio::sync::mpsc::block::Block, http::response::Response>> align=8 8960 values 32 header @@ -74,10 +58,23 @@ 40 __tracing_attr_span 64 variant Unresumed, Returned, Panicked +5648 {async fn body@crates/puffin-client/src/registry_client.rs:224:5: 224:30} align=8 + 5647 variant Suspend0 + 5576 __awaitee align=8 + 40 __tracing_attr_span ``` --- .../src/prioritized_distribution.rs | 48 ++++++++++--------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/crates/distribution-types/src/prioritized_distribution.rs b/crates/distribution-types/src/prioritized_distribution.rs index c704391de..164bf8488 100644 --- a/crates/distribution-types/src/prioritized_distribution.rs +++ b/crates/distribution-types/src/prioritized_distribution.rs @@ -12,8 +12,12 @@ pub struct DistRequiresPython { pub requires_python: Option, } +// Boxed because `Dist` is large. #[derive(Debug, Clone)] -pub struct PrioritizedDistribution { +pub struct PrioritizedDistribution(Box); + +#[derive(Debug, Clone)] +struct PrioritizedDistributionInner { /// An arbitrary source distribution for the package version. source: Option, /// The highest-priority, platform-compatible wheel for the package version. @@ -33,7 +37,7 @@ impl PrioritizedDistribution { priority: Option, ) -> Self { if let Some(priority) = priority { - Self { + Self(Box::new(PrioritizedDistributionInner { source: None, compatible_wheel: Some(( DistRequiresPython { @@ -45,9 +49,9 @@ impl PrioritizedDistribution { )), incompatible_wheel: None, hashes: hash.map(|hash| vec![hash]).unwrap_or_default(), - } + })) } else { - Self { + Self(Box::new(PrioritizedDistributionInner { source: None, compatible_wheel: None, incompatible_wheel: Some(DistRequiresPython { @@ -55,7 +59,7 @@ impl PrioritizedDistribution { requires_python, }), hashes: hash.map(|hash| vec![hash]).unwrap_or_default(), - } + })) } } @@ -65,7 +69,7 @@ impl PrioritizedDistribution { requires_python: Option, hash: Option, ) -> Self { - Self { + Self(Box::new(PrioritizedDistributionInner { source: Some(DistRequiresPython { dist, requires_python, @@ -73,7 +77,7 @@ impl PrioritizedDistribution { compatible_wheel: None, incompatible_wheel: None, hashes: hash.map(|hash| vec![hash]).unwrap_or_default(), - } + })) } /// Insert the given built distribution into the [`PrioritizedDistribution`]. @@ -86,9 +90,9 @@ impl PrioritizedDistribution { ) { // Prefer the highest-priority, platform-compatible wheel. if let Some(priority) = priority { - if let Some((.., existing_priority)) = &self.compatible_wheel { + if let Some((.., existing_priority)) = &self.0.compatible_wheel { if priority > *existing_priority { - self.compatible_wheel = Some(( + self.0.compatible_wheel = Some(( DistRequiresPython { dist, requires_python, @@ -97,7 +101,7 @@ impl PrioritizedDistribution { )); } } else { - self.compatible_wheel = Some(( + self.0.compatible_wheel = Some(( DistRequiresPython { dist, requires_python, @@ -105,15 +109,15 @@ impl PrioritizedDistribution { priority, )); } - } else if self.incompatible_wheel.is_none() { - self.incompatible_wheel = Some(DistRequiresPython { + } else if self.0.incompatible_wheel.is_none() { + self.0.incompatible_wheel = Some(DistRequiresPython { dist, requires_python, }); } if let Some(hash) = hash { - self.hashes.push(hash); + self.0.hashes.push(hash); } } @@ -124,24 +128,24 @@ impl PrioritizedDistribution { requires_python: Option, hash: Option, ) { - if self.source.is_none() { - self.source = Some(DistRequiresPython { + if self.0.source.is_none() { + self.0.source = Some(DistRequiresPython { dist, requires_python, }); } if let Some(hash) = hash { - self.hashes.push(hash); + self.0.hashes.push(hash); } } /// Return the highest-priority distribution for the package version, if any. pub fn get(&self) -> Option { match ( - &self.compatible_wheel, - &self.source, - &self.incompatible_wheel, + &self.0.compatible_wheel, + &self.0.source, + &self.0.incompatible_wheel, ) { // Prefer the highest-priority, platform-compatible wheel. (Some((wheel, tag_priority)), _, _) => { @@ -162,17 +166,17 @@ impl PrioritizedDistribution { /// Return the source distribution, if any. pub fn source(&self) -> Option<&DistRequiresPython> { - self.source.as_ref() + self.0.source.as_ref() } /// Return the compatible built distribution, if any. pub fn compatible_wheel(&self) -> Option<&(DistRequiresPython, TagPriority)> { - self.compatible_wheel.as_ref() + self.0.compatible_wheel.as_ref() } /// Return the hashes for each distribution. pub fn hashes(&self) -> &[Hashes] { - &self.hashes + &self.0.hashes } }