Skip to content

Commit 503f9a9

Browse files
authored
uv-resolver: pre-compute PEP 508 markers from universal markers (#10472)
It turns out that we use `UniversalMarker::pep508` quite a bit. To the point that it makes sense to pre-compute it when constructing a `UniversalMarker`. This still isn't necessarily the fastest thing we can do, but this results in a major speed-up and `without_extras` no longer shows up for me in a profile. Motivating benchmarks. First, from #10430: ``` $ hyperfine 'rm -f uv.lock && uv lock' 'rm -f uv.lock && uv-ag-optimize-without-extras lock' Benchmark 1: rm -f uv.lock && uv lock Time (mean ± σ): 408.3 ms ± 276.6 ms [User: 333.6 ms, System: 111.1 ms] Range (min … max): 316.9 ms … 1195.3 ms 10 runs Warning: The first benchmarking run for this command was significantly slower than the rest (1.195 s). This could be caused by (filesystem) caches that were not filled until after the first run. You should consider using the '--warmup' option to fill those caches before the actual benchmark. Alternatively, use the '--prepare' option to clear the caches before each timing run. Benchmark 2: rm -f uv.lock && uv-ag-optimize-without-extras lock Time (mean ± σ): 209.4 ms ± 2.2 ms [User: 209.8 ms, System: 103.8 ms] Range (min … max): 206.1 ms … 213.4 ms 14 runs Summary rm -f uv.lock && uv-ag-optimize-without-extras lock ran 1.95 ± 1.32 times faster than rm -f uv.lock && uv lock ``` And now from #10438: ``` $ hyperfine 'uv pip compile requirements.in -c constraints.txt --universal --no-progress --python-version 3.8 --offline > /dev/null' 'uv-ag-optimize-without-extras pip compile requirements.in -c constraints.txt --universal --no-progress --python-version 3.8 --offline > /dev/null' Benchmark 1: uv pip compile requirements.in -c constraints.txt --universal --no-progress --python-version 3.8 --offline > /dev/null Time (mean ± σ): 12.718 s ± 0.052 s [User: 12.818 s, System: 0.140 s] Range (min … max): 12.650 s … 12.815 s 10 runs Benchmark 2: uv-ag-optimize-without-extras pip compile requirements.in -c constraints.txt --universal --no-progress --python-version 3.8 --offline > /dev/null Time (mean ± σ): 419.5 ms ± 6.7 ms [User: 434.7 ms, System: 100.6 ms] Range (min … max): 412.7 ms … 434.3 ms 10 runs Summary uv-ag-optimize-without-extras pip compile requirements.in -c constraints.txt --universal --no-progress --python-version 3.8 --offline > /dev/null ran 30.32 ± 0.50 times faster than uv pip compile requirements.in -c constraints.txt --universal --no-progress --python-version 3.8 --offline > /dev/null ``` Fixes #10430, Fixes #10438
1 parent 685a53d commit 503f9a9

File tree

1 file changed

+21
-2
lines changed

1 file changed

+21
-2
lines changed

crates/uv-resolver/src/universal_marker.rs

+21-2
Original file line numberDiff line numberDiff line change
@@ -69,17 +69,24 @@ pub struct UniversalMarker {
6969
/// stand-point, evaluating it requires uv's custom encoding of extras (and
7070
/// groups).
7171
marker: MarkerTree,
72+
/// The strictly PEP 508 version of `marker`. Basically, `marker`, but
73+
/// without any extras in it. This could be computed on demand (and
74+
/// that's what we used to do), but we do it enough that it was causing a
75+
/// regression in some cases.
76+
pep508: MarkerTree,
7277
}
7378

7479
impl UniversalMarker {
7580
/// A constant universal marker that always evaluates to `true`.
7681
pub(crate) const TRUE: UniversalMarker = UniversalMarker {
7782
marker: MarkerTree::TRUE,
83+
pep508: MarkerTree::TRUE,
7884
};
7985

8086
/// A constant universal marker that always evaluates to `false`.
8187
pub(crate) const FALSE: UniversalMarker = UniversalMarker {
8288
marker: MarkerTree::FALSE,
89+
pep508: MarkerTree::FALSE,
8390
};
8491

8592
/// Creates a new universal marker from its constituent pieces.
@@ -94,21 +101,26 @@ impl UniversalMarker {
94101
/// Creates a new universal marker from a marker that has already been
95102
/// combined from a PEP 508 and conflict marker.
96103
pub(crate) fn from_combined(marker: MarkerTree) -> UniversalMarker {
97-
UniversalMarker { marker }
104+
UniversalMarker {
105+
marker,
106+
pep508: marker.without_extras(),
107+
}
98108
}
99109

100110
/// Combine this universal marker with the one given in a way that unions
101111
/// them. That is, the updated marker will evaluate to `true` if `self` or
102112
/// `other` evaluate to `true`.
103113
pub(crate) fn or(&mut self, other: UniversalMarker) {
104114
self.marker.or(other.marker);
115+
self.pep508.or(other.pep508);
105116
}
106117

107118
/// Combine this universal marker with the one given in a way that
108119
/// intersects them. That is, the updated marker will evaluate to `true` if
109120
/// `self` and `other` evaluate to `true`.
110121
pub(crate) fn and(&mut self, other: UniversalMarker) {
111122
self.marker.and(other.marker);
123+
self.pep508.and(other.pep508);
112124
}
113125

114126
/// Imbibes the world knowledge expressed by `conflicts` into this marker.
@@ -121,6 +133,7 @@ impl UniversalMarker {
121133
let self_marker = self.marker;
122134
self.marker = conflicts.marker;
123135
self.marker.implies(self_marker);
136+
self.pep508 = self.marker.without_extras();
124137
}
125138

126139
/// Assumes that a given extra/group for the given package is activated.
@@ -132,6 +145,7 @@ impl UniversalMarker {
132145
ConflictPackage::Extra(ref extra) => self.assume_extra(item.package(), extra),
133146
ConflictPackage::Group(ref group) => self.assume_group(item.package(), group),
134147
}
148+
self.pep508 = self.marker.without_extras();
135149
}
136150

137151
/// Assumes that a given extra/group for the given package is not
@@ -144,6 +158,7 @@ impl UniversalMarker {
144158
ConflictPackage::Extra(ref extra) => self.assume_not_extra(item.package(), extra),
145159
ConflictPackage::Group(ref group) => self.assume_not_group(item.package(), group),
146160
}
161+
self.pep508 = self.marker.without_extras();
147162
}
148163

149164
/// Assumes that a given extra for the given package is activated.
@@ -155,6 +170,7 @@ impl UniversalMarker {
155170
self.marker = self
156171
.marker
157172
.simplify_extras_with(|candidate| *candidate == extra);
173+
self.pep508 = self.marker.without_extras();
158174
}
159175

160176
/// Assumes that a given extra for the given package is not activated.
@@ -166,6 +182,7 @@ impl UniversalMarker {
166182
self.marker = self
167183
.marker
168184
.simplify_not_extras_with(|candidate| *candidate == extra);
185+
self.pep508 = self.marker.without_extras();
169186
}
170187

171188
/// Assumes that a given group for the given package is activated.
@@ -177,6 +194,7 @@ impl UniversalMarker {
177194
self.marker = self
178195
.marker
179196
.simplify_extras_with(|candidate| *candidate == extra);
197+
self.pep508 = self.marker.without_extras();
180198
}
181199

182200
/// Assumes that a given group for the given package is not activated.
@@ -188,6 +206,7 @@ impl UniversalMarker {
188206
self.marker = self
189207
.marker
190208
.simplify_not_extras_with(|candidate| *candidate == extra);
209+
self.pep508 = self.marker.without_extras();
191210
}
192211

193212
/// Returns true if this universal marker will always evaluate to `true`.
@@ -259,7 +278,7 @@ impl UniversalMarker {
259278
/// always use a universal marker since it accounts for all possible ways
260279
/// for a package to be installed.
261280
pub fn pep508(self) -> MarkerTree {
262-
self.marker.without_extras()
281+
self.pep508
263282
}
264283

265284
/// Returns the non-PEP 508 marker expression that represents conflicting

0 commit comments

Comments
 (0)