Skip to content

Commit

Permalink
uv-resolver: pre-compute PEP 508 markers from universal markers (#10472)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
BurntSushi authored Jan 10, 2025
1 parent 685a53d commit 503f9a9
Showing 1 changed file with 21 additions and 2 deletions.
23 changes: 21 additions & 2 deletions crates/uv-resolver/src/universal_marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,17 +69,24 @@ pub struct UniversalMarker {
/// stand-point, evaluating it requires uv's custom encoding of extras (and
/// groups).
marker: MarkerTree,
/// The strictly PEP 508 version of `marker`. Basically, `marker`, but
/// without any extras in it. This could be computed on demand (and
/// that's what we used to do), but we do it enough that it was causing a
/// regression in some cases.
pep508: MarkerTree,
}

impl UniversalMarker {
/// A constant universal marker that always evaluates to `true`.
pub(crate) const TRUE: UniversalMarker = UniversalMarker {
marker: MarkerTree::TRUE,
pep508: MarkerTree::TRUE,
};

/// A constant universal marker that always evaluates to `false`.
pub(crate) const FALSE: UniversalMarker = UniversalMarker {
marker: MarkerTree::FALSE,
pep508: MarkerTree::FALSE,
};

/// Creates a new universal marker from its constituent pieces.
Expand All @@ -94,21 +101,26 @@ impl UniversalMarker {
/// Creates a new universal marker from a marker that has already been
/// combined from a PEP 508 and conflict marker.
pub(crate) fn from_combined(marker: MarkerTree) -> UniversalMarker {
UniversalMarker { marker }
UniversalMarker {
marker,
pep508: marker.without_extras(),
}
}

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

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

/// Imbibes the world knowledge expressed by `conflicts` into this marker.
Expand All @@ -121,6 +133,7 @@ impl UniversalMarker {
let self_marker = self.marker;
self.marker = conflicts.marker;
self.marker.implies(self_marker);
self.pep508 = self.marker.without_extras();
}

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

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

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

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

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

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

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

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

0 comments on commit 503f9a9

Please sign in to comment.