mirror of https://github.com/astral-sh/uv
pep440-rs: switch Version::release to smallvec
This commit attempts an optimization that switches a version's `release`
field over to a `smallvec` optimization. The idea is that most versions
are very small and can be stored inline.
Interestingly, I was unable to observe any obvious benefit:
$ hyperfine \
"./target/profiling/puffin-dev-u32 resolve-many --cache-dir cache-docker-no-build --no-build pypi_top_8k_flat.txt --limit 1000 2> /dev/null" \
"./target/profiling/puffin-dev-smallvec-release resolve-many --cache-dir cache-docker-no-build --no-build pypi_top_8k_flat.txt --limit 1000 2> /dev/null"
Benchmark 1: ./target/profiling/puffin-dev-u32 resolve-many --cache-dir cache-docker-no-build --no-build pypi_top_8k_flat.txt --limit 1000 2> /dev/null
Time (mean ± σ): 872.2 ms ± 26.5 ms [User: 14646.0 ms, System: 2516.0 ms]
Range (min … max): 833.0 ms … 912.0 ms 10 runs
Benchmark 2: ./target/profiling/puffin-dev-smallvec-release resolve-many --cache-dir cache-docker-no-build --no-build pypi_top_8k_flat.txt --limit 1000 2> /dev/null
Time (mean ± σ): 882.3 ms ± 17.4 ms [User: 14764.4 ms, System: 2520.9 ms]
Range (min … max): 859.7 ms … 912.7 ms 10 runs
Summary
'./target/profiling/puffin-dev-u32 resolve-many --cache-dir cache-docker-no-build --no-build pypi_top_8k_flat.txt --limit 1000 2> /dev/null' ran
1.01 ± 0.04 times faster than './target/profiling/puffin-dev-smallvec-release resolve-many --cache-dir cache-docker-no-build --no-build pypi_top_8k_flat.txt --limit 1000 2> /dev/null'
My hypothesis is that because of an earlier commit that switched the
global allocator to jemalloc, the cost of allocation had precipitously
decreased. To the point that the reduction in allocs from the smallvec
becomes a wash. To test my hypothesis, I dropped the jemalloc commit and
measured the perf of the smallvec optimization against main:
$ hyperfine \
"./target/profiling/puffin-dev-main resolve-many --cache-dir cache-docker-no-build --no-build pypi_top_8k_flat.txt --limit 1000 2> /dev/null" \
"./target/profiling/puffin-dev-smallvec-release-no-jemalloc resolve-many --cache-dir cache-docker-no-build --no-build pypi_top_8k_flat.txt --limit 1000 2> /dev/null"
Benchmark 1: ./target/profiling/puffin-dev-main resolve-many --cache-dir cache-docker-no-build --no-build pypi_top_8k_flat.txt --limit 1000 2> /dev/null
Time (mean ± σ): 968.0 ms ± 20.0 ms [User: 17637.4 ms, System: 2151.9 ms]
Range (min … max): 940.2 ms … 1005.3 ms 10 runs
Benchmark 2: ./target/profiling/puffin-dev-smallvec-release-no-jemalloc resolve-many --cache-dir cache-docker-no-build --no-build pypi_top_8k_flat.txt --limit 1000 2> /dev/null
Time (mean ± σ): 958.4 ms ± 15.7 ms [User: 17119.7 ms, System: 2246.1 ms]
Range (min … max): 944.7 ms … 993.3 ms 10 runs
Summary
'./target/profiling/puffin-dev-smallvec-release-no-jemalloc resolve-many --cache-dir cache-docker-no-build --no-build pypi_top_8k_flat.txt --limit 1000 2> /dev/null' ran
1.01 ± 0.03 times faster than './target/profiling/puffin-dev-main resolve-many --cache-dir cache-docker-no-build --no-build pypi_top_8k_flat.txt --limit 1000 2> /dev/null'
Fiddlesticks. Even when allocation is (presumably) more expensive, the
smallvec optimization didn't help. This suggests something is off about
my mental model of the code. So there are more avenues to explore here!
This commit is contained in:
parent
259a835ab6
commit
f9528b4ecd
|
|
@ -2034,6 +2034,7 @@ dependencies = [
|
|||
"pyo3",
|
||||
"regex",
|
||||
"serde",
|
||||
"smallvec",
|
||||
"tracing",
|
||||
"unicode-width",
|
||||
]
|
||||
|
|
@ -2064,6 +2065,7 @@ dependencies = [
|
|||
"regex",
|
||||
"serde",
|
||||
"serde_json",
|
||||
"smallvec",
|
||||
"testing_logger",
|
||||
"thiserror",
|
||||
"tracing",
|
||||
|
|
@ -3275,9 +3277,9 @@ dependencies = [
|
|||
|
||||
[[package]]
|
||||
name = "smallvec"
|
||||
version = "1.11.1"
|
||||
version = "1.11.2"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "942b4a808e05215192e39f4ab80813e599068285906cc91aa64f923db842bd5a"
|
||||
checksum = "4dccd0940a2dcdf68d092b8cbab7dc0ad8fa938bf95787e1b916b0e3d0e8e970"
|
||||
|
||||
[[package]]
|
||||
name = "smawk"
|
||||
|
|
|
|||
|
|
@ -60,6 +60,7 @@ seahash = { version = "4.1.0" }
|
|||
serde = { version = "1.0.190" }
|
||||
serde_json = { version = "1.0.108" }
|
||||
sha2 = { version = "0.10.8" }
|
||||
smallvec = { version = "1.11.2" }
|
||||
tar = { version = "0.4.40" }
|
||||
target-lexicon = { version = "0.12.12" }
|
||||
tempfile = { version = "3.8.1" }
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ crate-type = ["rlib", "cdylib"]
|
|||
once_cell = { workspace = true }
|
||||
regex = { workspace = true }
|
||||
serde = { workspace = true, features = ["derive"], optional = true }
|
||||
smallvec = { workspace = true }
|
||||
tracing = { workspace = true, optional = true }
|
||||
unicode-width = { workspace = true }
|
||||
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ use regex::Captures;
|
|||
use regex::Regex;
|
||||
#[cfg(feature = "serde")]
|
||||
use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
|
||||
use smallvec::SmallVec;
|
||||
use std::cmp::{max, Ordering};
|
||||
#[cfg(feature = "pyo3")]
|
||||
use std::collections::hash_map::DefaultHasher;
|
||||
|
|
@ -269,7 +270,7 @@ pub struct Version {
|
|||
/// such a `1.2.3` in `4!1.2.3-a8.post9.dev1`
|
||||
///
|
||||
/// Note that we drop the * placeholder by moving it to `Operator`
|
||||
pub release: Vec<u32>,
|
||||
pub release: SmallVec<[u32; 8]>,
|
||||
/// The [prerelease](https://peps.python.org/pep-0440/#pre-releases), i.e. alpha, beta or rc
|
||||
/// plus a number
|
||||
///
|
||||
|
|
@ -328,7 +329,7 @@ impl PyVersion {
|
|||
/// Note that we drop the * placeholder by moving it to `Operator`
|
||||
#[getter]
|
||||
pub fn release(&self) -> Vec<u32> {
|
||||
self.0.release.clone()
|
||||
self.0.release.to_vec()
|
||||
}
|
||||
/// The [prerelease](https://peps.python.org/pep-0440/#pre-releases), i.e. alpha, beta or rc
|
||||
/// plus a number
|
||||
|
|
@ -445,7 +446,7 @@ impl Version {
|
|||
pub fn from_release(release: Vec<u32>) -> Self {
|
||||
Self {
|
||||
epoch: 0,
|
||||
release,
|
||||
release: SmallVec::from(release),
|
||||
pre: None,
|
||||
post: None,
|
||||
dev: None,
|
||||
|
|
@ -785,7 +786,7 @@ impl Version {
|
|||
.as_str()
|
||||
.split('.')
|
||||
.map(|segment| segment.parse::<u32>().map_err(|err| err.to_string()))
|
||||
.collect::<Result<Vec<u32>, String>>()?;
|
||||
.collect::<Result<SmallVec<[u32; 8]>, String>>()?;
|
||||
|
||||
let star = captures.name("trailing_dot_star").is_some();
|
||||
if star {
|
||||
|
|
|
|||
|
|
@ -566,6 +566,7 @@ pub fn parse_version_specifiers(spec: &str) -> Result<Vec<VersionSpecifier>, Pep
|
|||
mod test {
|
||||
use crate::{Operator, Version, VersionSpecifier, VersionSpecifiers};
|
||||
use indoc::indoc;
|
||||
use smallvec::smallvec;
|
||||
use std::cmp::Ordering;
|
||||
use std::str::FromStr;
|
||||
|
||||
|
|
@ -1060,7 +1061,7 @@ mod test {
|
|||
operator: Operator::TildeEqual,
|
||||
version: Version {
|
||||
epoch: 0,
|
||||
release: vec![0, 9],
|
||||
release: smallvec![0, 9],
|
||||
pre: None,
|
||||
post: None,
|
||||
dev: None,
|
||||
|
|
@ -1071,7 +1072,7 @@ mod test {
|
|||
operator: Operator::GreaterThanEqual,
|
||||
version: Version {
|
||||
epoch: 0,
|
||||
release: vec![1, 0],
|
||||
release: smallvec![1, 0],
|
||||
pre: None,
|
||||
post: None,
|
||||
dev: None,
|
||||
|
|
@ -1082,7 +1083,7 @@ mod test {
|
|||
operator: Operator::NotEqualStar,
|
||||
version: Version {
|
||||
epoch: 0,
|
||||
release: vec![1, 3, 4],
|
||||
release: smallvec![1, 3, 4],
|
||||
pre: None,
|
||||
post: None,
|
||||
dev: None,
|
||||
|
|
@ -1093,7 +1094,7 @@ mod test {
|
|||
operator: Operator::LessThan,
|
||||
version: Version {
|
||||
epoch: 0,
|
||||
release: vec![2, 0],
|
||||
release: smallvec![2, 0],
|
||||
pre: None,
|
||||
post: None,
|
||||
dev: None,
|
||||
|
|
|
|||
|
|
@ -37,6 +37,7 @@ indoc = "2.0.4"
|
|||
log = "0.4.20"
|
||||
testing_logger = "0.1.1"
|
||||
serde_json = "1.0.108"
|
||||
smallvec = { workspace = true }
|
||||
|
||||
[features]
|
||||
pyo3 = ["dep:pyo3", "pep440_rs/pyo3", "pyo3-log"]
|
||||
|
|
|
|||
|
|
@ -888,6 +888,7 @@ mod tests {
|
|||
use std::str::FromStr;
|
||||
|
||||
use indoc::indoc;
|
||||
use smallvec::smallvec;
|
||||
use url::Url;
|
||||
|
||||
use pep440_rs::{Operator, Version, VersionSpecifier};
|
||||
|
|
@ -956,7 +957,7 @@ mod tests {
|
|||
Operator::GreaterThanEqual,
|
||||
Version {
|
||||
epoch: 0,
|
||||
release: vec![2, 8, 1],
|
||||
release: smallvec![2, 8, 1],
|
||||
pre: None,
|
||||
post: None,
|
||||
dev: None,
|
||||
|
|
@ -969,7 +970,7 @@ mod tests {
|
|||
Operator::Equal,
|
||||
Version {
|
||||
epoch: 0,
|
||||
release: vec![2, 8],
|
||||
release: smallvec![2, 8],
|
||||
pre: None,
|
||||
post: None,
|
||||
dev: None,
|
||||
|
|
|
|||
Loading…
Reference in New Issue