perf(pep440set): take the release group key from gpp, not from a render

CHANGELOG.md

@@ -13,6 +13,84 @@ served it.
 
 ## [Unreleased]
 
+### Changed
+
+- **`pep440set` no longer renders a version back to text to find its release
+  group.** Warm resolution is **1.19x to 1.39x faster and allocates 1.25x to
+  2.27x less** on the six corpus entries that resolve anything; cold is 1.11x to
+  1.28x. The seventh, `unsatisfiable`, does not move — see below. No resolution
+  this module produces changes.
+
+  `Contains` needs each candidate version's `(epoch, release)` position, and it
+  derived that by calling `BaseVersion()` — which renders the version through a
+  `bytes.Buffer` with one `math/big` decimal conversion **per segment** — and
+  then splitting the result back into digit runs. Once per candidate version per
+  `Contains` call. It now asks go-python-packaging for
+  `version.ReleaseKey` instead, which reads the parsed `Version`'s own epoch and
+  release fields: **no allocation, against 10** for the render-and-split, and
+  roughly an order of magnitude less time.
+
+  ⚠️ The allocation counts are exact and machine-independent; the times are
+  not. gpp's `BenchmarkReleaseKeyVsBaseVersionSplit` reads 16 ns vs 213 ns on
+  an idle M4 Max and 23 ns vs 520 ns on a loaded one — the ratio survives, a
+  quoted nanosecond figure does not. Run the benchmark rather than trusting a
+  number written down here.
+
+  Medians of three interleaved rounds, ten iterations, against the production
+  snapshot (932,861 packages, dated 2026-08-04), on an Apple M4 Max:
+
+  | entry | warm before | warm after | | warm allocs before | after | |
+  |---|---|---|---|---|---|---|
+  | `single-no-deps` | 0.21 ms | 0.15 ms | 1.38x | 3,637 | 2,260 | 1.61x |
+  | `small-tree` | 1.06 ms | 0.80 ms | 1.32x | 19,393 | 10,908 | 1.78x |
+  | `extras` | 1.45 ms | 1.08 ms | 1.35x | 29,391 | 15,102 | 1.95x |
+  | `app-set` | 4.54 ms | 3.27 ms | 1.39x | 96,318 | 42,415 | 2.27x |
+  | `wide-versions` | 8.11 ms | 6.74 ms | 1.20x | 197,228 | 127,659 | 1.54x |
+  | `backtracking` | 2.78 ms | 2.33 ms | 1.19x | 43,304 | 21,015 | 2.06x |
+  | `unsatisfiable` | 0.33 ms | 0.31 ms | 1.07x | 6,448 | 5,168 | 1.25x |
+
+  | entry | cold before | cold after | | cold allocs before | after | |
+  |---|---|---|---|---|---|---|
+  | `single-no-deps` | 0.27 ms | 0.23 ms | 1.17x | 4,365 | 2,982 | 1.46x |
+  | `small-tree` | 1.41 ms | 1.10 ms | 1.28x | 23,609 | 15,107 | 1.56x |
+  | `extras` | 1.82 ms | 1.46 ms | 1.25x | 34,630 | 20,354 | 1.70x |
+  | `app-set` | 5.94 ms | 4.70 ms | 1.26x | 118,465 | 64,533 | 1.84x |
+  | `wide-versions` | 13.14 ms | 11.86 ms | 1.11x | 276,776 | 207,206 | 1.34x |
+  | `backtracking` | 6.22 ms | 5.26 ms | 1.18x | 89,097 | 66,832 | 1.33x |
+  | `unsatisfiable` | 0.55 ms | 0.51 ms | 1.08x | 9,256 | 7,974 | 1.16x |
+
+  **The allocation column is the one to read.** The saving is not the arithmetic
+  — it is the garbage. `app-set` allocates 2.27x less warm while running 1.39x
+  faster.
+
+  In a CPU profile of the warm resolution, `Set.Contains` falls from **41% of
+  `resolver.Resolve` to 24%** on `app-set`, and from 33% to 14% on
+  `wide-versions` — 3.1x and 3.8x fewer samples in absolute terms. Measured
+  against `Resolve` rather than against total samples deliberately: removing
+  this allocation shrinks the profile's garbage-collection share too, so a
+  "% of samples" figure would credit the change with that as well.
+
+  `strings.Split` leaves the profile entirely. `writeRelease` and
+  `math/big.nat.itoa` do **not** — `ensurePub` still renders the public
+  spelling, which this change deliberately does not touch — but they fall from
+  2.3% and 1.6% of samples under `Contains` to below the sampling floor.
+
+  ⚠️ **`unsatisfiable` is not measurably faster.** Its 1.07x is the median of
+  three rounds, and a fourth confirmation round put it at **0.96x** — slower.
+  Read it as no change, not as a small win. That is the predicted result rather
+  than a disappointment: it fails before enumerating many candidates, so it
+  makes the fewest `Contains` calls of the corpus and has the least to gain.
+  Its allocations do fall, by 1.25x, which is the effect showing up where the
+  mechanism says it should.
+
+  ⚠️ The measurement is against `v0.6.0` **without** the parsed-version memo. The
+  two are not additive — after this change the largest remaining warm cost is
+  `version.Parse` inside `index.Versions` (16.9% of samples on `wide-versions`),
+  which is exactly what that memo removes.
+
+- **go-python-packaging is now `v0.7.0`**, for `version.ReleaseKey`. Purely
+  additive; nothing else in this module changed with the pin.
+
 ## [0.6.0] - 2026-08-14
 
 ### Changed

go.mod

@@ -5,7 +5,7 @@ go 1.25.0
 require (
 	github.com/klauspost/compress v1.19.2
 	github.com/posit-dev/go-pubgrub v0.2.0
-	github.com/posit-dev/go-python-packaging v0.6.0
+	github.com/posit-dev/go-python-packaging v0.7.0
 	github.com/rstudio/repository-snapshot-format v0.12.0
 )
 

go.sum

@@ -6,8 +6,8 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/posit-dev/go-pubgrub v0.2.0 h1:RWGbZl0x1s1T/7kF1e1Q4CKyy+gMBqEh1rCaZwIzdLs=
 github.com/posit-dev/go-pubgrub v0.2.0/go.mod h1:5ks/krPJaOu6GmbBqaeOiFR+cBa4lh5lkYIxun6cWW4=
-github.com/posit-dev/go-python-packaging v0.6.0 h1:bqiHnvfeFN7TV7QWxwgCyTrZ75oJhg+hr0K41S6l67c=
-github.com/posit-dev/go-python-packaging v0.6.0/go.mod h1:NkELXOh/eRDT7c+LKFxJd+HkyGpKFsUIxpTYaq5xdkU=
+github.com/posit-dev/go-python-packaging v0.7.0 h1:pxyI2MmsP2iLqcwHkjZiXMPvvagywbslXid35czAh3o=
+github.com/posit-dev/go-python-packaging v0.7.0/go.mod h1:NkELXOh/eRDT7c+LKFxJd+HkyGpKFsUIxpTYaq5xdkU=
 github.com/rstudio/go-version v0.0.2 h1:ihU0xaF+Yuya0p2J6C8dfyB4gu/YehM1d6cTpabf/7A=
 github.com/rstudio/go-version v0.0.2/go.mod h1:Xfuma+m4R9L0P+Hof8iDDiL4/TJ8VjpdDXWq8qGXHL4=
 github.com/rstudio/repository-snapshot-format v0.12.0 h1:a6gRfXPLOR+5YxN7irO097lpWGOPMe8ZoATGkC9hcWE=

pep440set/bound.go

@@ -50,21 +50,22 @@ type bound struct {
 
 // posKey is everything cmpBound needs from a bound's version.
 //
-// ⚠️ DERIVE IT AT CONSTRUCTION, NOT PER COMPARISON. Every field here costs at
-// least one string render: version.Version holds its release as big.Ints and
-// its public spelling only as something String() builds, and Public() then
-// needs a re-parse to be comparable. cmpBound runs in the innermost loop of the
-// set algebra, which is itself in the solver's hot loop, so it is called orders
-// of magnitude more often than a bound is built. One resolution against a
-// curated-shaped index -- packages present, transitive dependencies absent --
-// took 17.5 s and allocated 25 GB CUMULATIVELY with this work on the comparison
-// side. Cumulatively, not concurrently: peak heap stayed under 120 MB, so what
-// the churn bought was garbage collection. The cost is latency, not footprint.
+// ⚠️ DERIVE IT AT CONSTRUCTION, NOT PER COMPARISON. version.Version holds its
+// public spelling only as something String() builds, and Public() then needs a
+// re-parse to be comparable, so the public fields below cost a render each.
+// cmpBound runs in the innermost loop of the set algebra, which is itself in
+// the solver's hot loop, so it is called orders of magnitude more often than a
+// bound is built. One resolution against a curated-shaped index -- packages
+// present, transitive dependencies absent -- took 17.5 s and allocated 25 GB
+// CUMULATIVELY with this work on the comparison side. Cumulatively, not
+// concurrently: peak heap stayed under 120 MB, so what the churn bought was
+// garbage collection. The cost is latency, not footprint.
 type posKey struct {
-	// epoch and release are the canonical (leading-zero-free, trailing-zero-
-	// stripped) decimal digit runs releaseKey produces.
-	epoch   string
-	release []string
+	// rel is v's release group: its epoch and its trailing-zero-stripped
+	// release segments, ignoring the pre/post/dev/local suffix. gpp derives it
+	// from the parsed fields, so unlike the rest of this struct it costs no
+	// render -- see the note above cmpBound.
+	rel version.ReleaseKey
 	// public is v.Public(), and pub is that spelling parsed, which is v with
 	// any local label removed. pubOK is false when the spelling does not parse,
 	// which leaves the public comparison out entirely, exactly as it was when
@@ -76,8 +77,7 @@ type posKey struct {
 
 // newPosKey derives the key for v.
 func newPosKey(v version.Version) *posKey {
-	epoch, release := releaseKey(v)
-	k := &posKey{epoch: epoch, release: release, public: v.Public()}
+	k := &posKey{rel: v.ReleaseKey(), public: v.Public()}
 	if k.public == "" {
 		// An uninitialized Version. Parsing its (empty) public spelling failed
 		// when cmpBound did it inline, so it stays out of the comparison.
@@ -134,102 +134,50 @@ func (b bound) tier() int {
 	}
 }
 
-// isDigits reports whether s is a non-empty run of ASCII digits.
-func isDigits(s string) bool {
-	if s == "" {
-		return false
-	}
-	for i := 0; i < len(s); i++ {
-		if s[i] < '0' || s[i] > '9' {
-			return false
-		}
-	}
-	return true
-}
-
-// canonDigits strips leading zeros from a digit run, keeping one digit for an
-// all-zero segment, so "007" and "7" produce the same key component.
-func canonDigits(s string) string {
-	i := 0
-	for i < len(s)-1 && s[i] == '0' {
-		i++
-	}
-	return s[i:]
-}
-
-// releaseKey returns the (epoch, release) of b's version, with trailing zeros
-// stripped so 1.0 and 1.0.0.0 share a key. BaseVersion renders "1!3.4.5" for
-// an epoch, so the epoch is split off here.
+// cmpBound reports whether a is before (-1), at (0) or after (+1) b.
+//
+// # Where the release group key comes from, and why it is not derived here
+//
+// The (epoch, release) group key is version.ReleaseKey, derived by gpp from the
+// parsed Version's own epoch and release fields.
 //
-// ⚠️ THE KEY COMPONENTS ARE DECIMAL STRINGS, NOT ints. DO NOT "SIMPLIFY" THIS
-// BACK TO strconv.Atoi.
+// ⚠️ DO NOT REDERIVE IT FROM A RENDERED STRING. This package used to: it called
+// BaseVersion(), which renders "1!3.4.5" through a bytes.Buffer with one
+// math/big decimal conversion PER SEGMENT, and then split the result back into
+// digit runs -- once per candidate version per Contains call.
 //
-// PEP 440 puts no ceiling on an epoch or a release segment -- the grammar is
-// `[0-9]+` -- and gpp stores both as arbitrary-precision part.BigInt, so it
-// orders 1.99999999999999999999 above 1.5 correctly. The earlier key parsed
-// each segment with strconv.Atoi and BROKE out of the loop on error, so a
-// segment at or above 2^63 was dropped along with every segment AFTER it: the
-// key became a PREFIX of the real release, and 99999999999999999999.0 keyed as
-// the empty release, sorting below every version in existence. That made
+// That derivation was 82% of Set.Contains on the warm app-set resolution and
+// 74% on wide-versions, measured with pprof against the production snapshot.
+//
+// ⚠️ Measured as a share of resolver.Resolve, NOT as a share of total samples.
+// The denominator has to be something the change does not move: removing this
+// allocation shrinks the profile's garbage-collection share too, so "% of
+// samples" would credit this change with that as well. Against Resolve,
+// Contains falls from 41% to 24% on app-set and from 33% to 14% on
+// wide-versions -- 3.1x and 3.8x fewer samples in absolute terms.
+//
+// End to end that is 1.39x warm on app-set and 1.20x on wide-versions, with
+// 2.27x fewer allocations. BenchmarkContains/cross-group -- the common case,
+// probing a span that anchors some other release -- went from 304 ns and 8
+// allocations to 104 ns and none.
+//
+// ⚠️ strings.Split leaves the profile entirely, but writeRelease and
+// math/big.nat.itoa DO NOT: ensurePub still renders the public spelling, which
+// this change deliberately does not touch. They drop from 2.3% and 1.6% of
+// samples under Contains to below the sampling floor, which is not the same as
+// gone. What is left inside Contains is mostly verPos.init, and most of that is
+// the two Version copies it makes rather than the key.
+//
+// ⚠️ AND DO NOT REPLACE IT WITH ints. An even earlier key parsed each segment
+// with strconv.Atoi and BROKE out of the loop on error, so a segment at or
+// above 2^63 was dropped along with every segment AFTER it: the key became a
+// PREFIX of the real release, and 99999999999999999999.0 keyed as the empty
+// release, sorting below every version in existence. That made
 // `>99999999999999999999.0` admit everything while Check admitted nothing, and
-// `<1.5` admit 1.99999999999999999999. Comparing the digit runs directly
-// (length first, then byte-wise) is exact at every magnitude and needs no
-// math/big.
-func releaseKey(v version.Version) (epoch string, release []string) {
-	base := v.BaseVersion()
-	epoch = "0"
-	if i := strings.Index(base, "!"); i >= 0 {
-		// A non-numeric epoch cannot come out of BaseVersion; if one ever did,
-		// treating it as 0 keeps this a total order rather than a panic.
-		if isDigits(base[:i]) {
-			epoch = canonDigits(base[:i])
-		}
-		base = base[i+1:]
-	}
-	for _, part := range strings.Split(base, ".") {
-		if !isDigits(part) {
-			break
-		}
-		release = append(release, canonDigits(part))
-	}
-	for len(release) > 0 && release[len(release)-1] == "0" {
-		release = release[:len(release)-1]
-	}
-	return epoch, release
-}
-
-// cmpDigits orders two canonical (leading-zero-free) digit runs by value. The
-// shorter run is the smaller number, and equal-length runs compare byte-wise,
-// which for ASCII digits is the same as comparing values.
-func cmpDigits(a, b string) int {
-	switch {
-	case len(a) < len(b):
-		return -1
-	case len(a) > len(b):
-		return 1
-	}
-	return strings.Compare(a, b)
-}
-
-// cmpSegments orders two release keys segment by segment, the shorter being
-// smaller when it is a prefix of the longer. releaseKey has already stripped
-// trailing zeros, so a shorter key means a genuinely shorter release.
-func cmpSegments(a, b []string) int {
-	for i := 0; i < len(a) && i < len(b); i++ {
-		if c := cmpDigits(a[i], b[i]); c != 0 {
-			return c
-		}
-	}
-	switch {
-	case len(a) < len(b):
-		return -1
-	case len(a) > len(b):
-		return 1
-	}
-	return 0
-}
-
-// cmpBound reports whether a is before (-1), at (0) or after (+1) b.
+// `<1.5` admit 1.99999999999999999999. PEP 440 puts no ceiling on an epoch or a
+// release segment -- the grammar is `[0-9]+` -- and ReleaseKey is exact at
+// every magnitude, comparing arbitrary-precision integers for the keys that do
+// not fit its packed fast path.
 func cmpBound(a, b bound) int {
 	if a.inf != 0 || b.inf != 0 {
 		switch {
@@ -242,10 +190,7 @@ func cmpBound(a, b bound) int {
 	}
 
 	ak, bk := a.pos(), b.pos()
-	if c := cmpDigits(ak.epoch, bk.epoch); c != 0 {
-		return c
-	}
-	if c := cmpSegments(ak.release, bk.release); c != 0 {
+	if c := ak.rel.Compare(bk.rel); c != 0 {
 		return c
 	}
 

pep440set/bound_invariants_test.go

@@ -83,6 +83,18 @@ func ascendingPositions(t *testing.T) []orderEntry {
 		below("1.0.1"),
 		at("1.0.1"),
 		aboveRelease("1.0.1"),
+
+		// ⚠️ A pair differing ONLY past the sixth release segment. gpp packs a
+		// release into six 32-bit fields and falls back to arbitrary-precision
+		// comparison beyond that, so these two straddle the fast path's edge:
+		// a packer that silently truncated at six segments, or whose fields
+		// overlapped, would order them EQUAL and nothing else in this module
+		// would notice. gpp has its own tests for the layout; this is the row
+		// that makes a regression in it fail HERE, where the consequence is a
+		// resolver picking the wrong version.
+		at("1.0.1.2.3.4.5"),
+		at("1.0.1.2.3.4.6"),
+
 		below("1.1"),
 		at("1.1"),
 		below("1.5"),
@@ -177,15 +189,14 @@ func TestBoundEqualPositions(t *testing.T) {
 			bound{v: v("1!1.0"), edge: edgeAt}, bound{v: v("01!1.0"), edge: edgeAt}},
 		{"at(0!1.0) == at(1.0)",
 			bound{v: v("0!1.0"), edge: edgeAt}, bound{v: v("1.0"), edge: edgeAt}},
-		// ⚠️ THESE TWO DO NOT REACH canonDigits' STRIPPING, so do not read them
-		// as its guard. releaseKey derives from BaseVersion(), which renders
-		// each segment through big.Int.String() and is therefore already
-		// leading-zero-free: "1.00000000000000000001" arrives here as "1.1".
-		// What they guard is the layer above -- that a segment past 2^63
-		// survives the parse at full precision, and that cmpDigits orders two
-		// runs by VALUE (length first) rather than truncating to an int or
-		// comparing rendered text. canonDigits is covered directly by
-		// TestCanonDigits.
+		// ⚠️ THESE TWO ARE NOT ABOUT LEADING ZEROS, so do not read them as that
+		// guard. The group key comes from gpp's parsed release segments, which
+		// are math/big integers: "1.00000000000000000001" is the integer 1
+		// before a key exists, and no zero-stripping runs at all. What they
+		// guard is the layer above -- that a segment past 2^63 survives the
+		// parse at full precision, and that the key orders it by VALUE rather
+		// than truncating to an int or comparing rendered text. Leading-zero
+		// spellings are covered directly by TestLeadingZeroSpellings.
 		{"at(1.00000000000000000001) == at(1.1)",
 			bound{v: v("1.00000000000000000001"), edge: edgeAt},
 			bound{v: v("1.1"), edge: edgeAt}},