perf(pep440set): take the release group key from gpp, not from a render - #46
Merged
Conversation
Contains needs each candidate version's (epoch, release) position, and it got it by calling BaseVersion() -- which renders the version through a bytes.Buffer with one math/big decimal conversion PER SEGMENT -- then split the text back into digit runs. Once per candidate version per Contains call. version.ReleaseKey, new in go-python-packaging v0.7.0, derives the same key from the parsed Version's own epoch and release fields: 16 ns and no allocation against 220 ns and 10 allocations. releaseKey, canonDigits, cmpDigits and cmpSegments go with it. isDigits stays; construct.go uses it. Warm resolution against the production snapshot 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 allocation column is the one to read -- what the render was buying was garbage. Contains/cross-group, the common case, drops from 304 ns and 8 allocations to 104 ns and none, and Set.Contains falls from 41% of resolver.Resolve to 24% on app-set. Full tables in resolver/bench_test.go.⚠️ That share is against Resolve, not against total samples: removing this allocation shrinks the profile's GC share too, so "% of samples" would credit this change with that as well. And strings.Split leaves the profile while writeRelease and math/big.nat.itoa do NOT -- ensurePub still renders the public spelling, which this change does not touch. Both were stated wrongly in an earlier draft of these comments. TestBoundOrderingGrid gains a pair differing only past the SIXTH release segment, where gpp's packed fast path gives way to arbitrary precision. Truncating gpp's packer there instead of refusing makes that row fail; without it, nothing in this module would notice.⚠️ unsatisfiable is NOT measurably faster. Its 1.07x is a median of three and a fourth confirmation round put it at 0.96x, slower; read it as no change. That is the prediction met rather than a disappointment -- it fails before enumerating many candidates and so makes the fewest Contains calls in the corpus. Its allocations do fall by 1.25x, which is the effect appearing where the mechanism says it should.⚠️ This is the THIRD change to this cost and the one that removes it. #33 derived a bound's key once instead of per comparison and #39 replaced the probe bound with a stack-held verPos, but both left the derivation itself a string render, in newPosKey and in verPos.init. Fixing releaseKey fixes both and leaves no fourth site. A memo over the old function was measured and rejected: it would have made memory larger where this makes it smaller. TestCanonDigits is replaced rather than deleted. It existed because the key was built from DIGIT RUNS compared length-first, where "007" would sort above "7". The key now comes from math/big integers, where "007" is 7 before a key exists and there is no run to strip -- the concern is gone rather than moved. But the behaviour it protected is not optional, so TestLeadingZeroSpellings asserts it through cmpBound at widths either side of int64 and uint64, and also asserts the spelling has not flattened a real difference: 1.007 is 1.7, not 1.70 and not 1.0.7. Held by the production differential at PEP440SET_CORPUS_PACKAGES=20000 -- 33,918,235 (specifier, version) pairs, 0 mismatches against Check -- the rendering differential over the same 20,000 packages, FuzzFromSpecifiers for 120 s with no new input, the ordering suites under -race, and on the gpp side a migration differential against this very algorithm across 348,752 corpus versions.
…ation counts are not An unbriefed review seat measured 23 ns / 520 ns for gpp's ReleaseKey benchmark where this entry quotes 16 ns / 220 ns. Both are real: mine was an idle machine, theirs was running mutation tests. The ratio survives either way and the allocation counts -- 0 against 10 -- are identical on both, so those are what the entry now leads with. A reader who needs a time should run the benchmark.
jonyoder
added a commit
that referenced
this pull request
Aug 14, 2026
RSFIndex.Versions memoized the sorted order of a package's version keys and re-parsed every one of them on every call. It held strings because it had to: until go-python-packaging v0.6.0 a version.Version could not be shared between goroutines, since Version.Compare padded the shorter operand's release segment with append into spare capacity a by-value copy shares. v0.6.0 fixed that -- packable versions never touch part.Parts, and the fallback pads by copying -- and this takes the memo the fix unblocks. Warm resolution is 1.13x to 3.23x faster and allocates 41% to 94% fewer objects against the production snapshot, measured at base 11da678 over nine interleaved rounds. Cold does not improve, as expected: the first call already parsed the keys in order to sort them. candvers, metadata and the pin set are identical on every corpus entry, and 4,007 resolutions produce byte-identical transcripts with zero one-sided timeout exclusions.⚠️ These figures are larger than the same change measured against 6c13230 (1.24x and 2.27x). #46 shrank the denominator rather than the parse: parseKeys went from 47.4% of wide-versions' Resolve to 69.1%. Neither figure is quotable against the other's base. The returned slice is a COPY of the memo's, because cmd/pyresolve's `versions` subcommand sorts the result in place. Measured at -0.3% to +9.7% warm against a variant that shares -- higher than the "under 2%" this was scoped with, for the same denominator reason. It is also redundant on this module's own resolution path, since provider hands the result straight to candidate.Rank, which copies again; it protects the exported contract, and an internal accessor that skips it is a follow-up worth taking at that price. The cost is retained heap rather than churn, measured rather than estimated: a warmed index keeps roughly three times as much alive after a resolve (app-set 0.35 -> 0.99 MB, wide-versions 1.31 -> 4.41 MB), identical at both bases. The memo is left unbounded to match the blob cache's rationale, and the code now says plainly that a bound is a prerequisite for embedding this in a long-lived server. index/shared_memo_test.go shares one warmed memo across eight goroutines over fixtures chosen for the padding hazard's actual shape, covering both of v0.6.0's safety arguments and also driving version.ReleaseKey, which #46 made a second reader of a shared parsed version. Every fixture reports WARNING: DATA RACE under a v0.5.0 pin, 20 fresh processes each.⚠️ That guard is inert until the -race step in #44 lands: gpr CI runs plain `go test`, under which the equivalent pre-existing check catches a deleted defensive copy 0 times in 20. Sweeps the rationales that asserted the memo was impossible, in index/rsfindex.go, index/mock.go, provider/provider.go and resolver/bench_test.go. The CHANGELOG diff is pure insertion; no dated section is touched.
jonyoder
added a commit
that referenced
this pull request
Aug 14, 2026
RSFIndex.Versions memoized the sorted order of a package's version keys and re-parsed every one of them on every call. It held strings because it had to: until go-python-packaging v0.6.0 a version.Version could not be shared between goroutines, since Version.Compare padded the shorter operand's release segment with append into spare capacity a by-value copy shares. v0.6.0 fixed that -- packable versions never touch part.Parts, and the fallback pads by copying -- and this takes the memo the fix unblocks. Warm resolution is 1.13x to 3.23x faster and allocates 41% to 94% fewer objects against the production snapshot, measured at base 11da678 over nine interleaved rounds. Cold does not improve, as expected: the first call already parsed the keys in order to sort them. candvers, metadata and the pin set are identical on every corpus entry, and 4,007 resolutions produce byte-identical transcripts with zero one-sided timeout exclusions.⚠️ These figures are larger than the same change measured against 6c13230 (1.24x and 2.27x). #46 shrank the denominator rather than the parse: parseKeys went from 47.4% of wide-versions' Resolve to 69.1%. Neither figure is quotable against the other's base. The returned slice is a COPY of the memo's, because cmd/pyresolve's `versions` subcommand sorts the result in place. Measured at -0.3% to +9.7% warm against a variant that shares -- higher than the "under 2%" this was scoped with, for the same denominator reason. It is also redundant on this module's own resolution path, since provider hands the result straight to candidate.Rank, which copies again; it protects the exported contract, and an internal accessor that skips it is a follow-up worth taking at that price. The cost is retained heap rather than churn, measured rather than estimated: a warmed index keeps roughly three times as much alive after a resolve (app-set 0.35 -> 0.99 MB, wide-versions 1.31 -> 4.41 MB), identical at both bases. The memo is left unbounded to match the blob cache's rationale, and the code now says plainly that a bound is a prerequisite for embedding this in a long-lived server. index/shared_memo_test.go shares one warmed memo across eight goroutines over fixtures chosen for the padding hazard's actual shape, covering both of v0.6.0's safety arguments and also driving version.ReleaseKey, which #46 made a second reader of a shared parsed version. Every fixture reports WARNING: DATA RACE under a v0.5.0 pin, 20 fresh processes each. That guard runs in CI on the -race step #44 added, which this is rebased onto. Without it these tests are a green no-op: measured on a tree with the defensive copy deleted, the pre-existing concurrency check catches it 8 times in 20 under -race and 0 times in 20 without. Sweeps the rationales that asserted the memo was impossible, in index/rsfindex.go, index/mock.go, provider/provider.go and resolver/bench_test.go. The CHANGELOG diff is pure insertion; no dated section is touched.
jonyoder
added a commit
that referenced
this pull request
Aug 14, 2026
RSFIndex.Versions memoized the sorted order of a package's version keys and re-parsed every one of them on every call. It held strings because it had to: until go-python-packaging v0.6.0 a version.Version could not be shared between goroutines, since Version.Compare padded the shorter operand's release segment with append into spare capacity a by-value copy shares. v0.6.0 fixed that -- packable versions never touch part.Parts, and the fallback pads by copying -- and this takes the memo the fix unblocks. Warm resolution is 1.13x to 3.23x faster and allocates 41% to 94% fewer objects against the production snapshot, measured at base 11da678 over nine interleaved rounds. Cold does not improve, as expected: the first call already parsed the keys in order to sort them. candvers, metadata and the pin set are identical on every corpus entry, and 4,007 resolutions produce byte-identical transcripts with zero one-sided timeout exclusions.⚠️ These figures are larger than the same change measured against 6c13230 (1.24x and 2.27x). #46 shrank the denominator rather than the parse: parseKeys went from 47.4% of wide-versions' Resolve to 69.1%. Neither figure is quotable against the other's base. The returned slice is a COPY of the memo's, because cmd/pyresolve's `versions` subcommand sorts the result in place. Measured at -0.3% to +9.7% warm against a variant that shares -- higher than the "under 2%" this was scoped with, for the same denominator reason. It is also redundant on this module's own resolution path, since provider hands the result straight to candidate.Rank, which copies again; it protects the exported contract, and an internal accessor that skips it is a follow-up worth taking at that price. The cost is retained heap rather than churn, measured rather than estimated: a warmed index keeps roughly three times as much alive after a resolve (app-set 0.35 -> 0.99 MB, wide-versions 1.31 -> 4.41 MB), identical at both bases. The memo is left unbounded to match the blob cache's rationale, and the code now says plainly that a bound is a prerequisite for embedding this in a long-lived server. index/shared_memo_test.go shares one warmed memo across eight goroutines over fixtures chosen for the padding hazard's actual shape, covering both of v0.6.0's safety arguments and also driving version.ReleaseKey, which #46 made a second reader of a shared parsed version. Every fixture reports WARNING: DATA RACE under a v0.5.0 pin, 20 fresh processes each. That guard runs in CI on the -race step #44 added, which this is rebased onto. Without it these tests are a green no-op: measured on a tree with the defensive copy deleted, the pre-existing concurrency check catches it 8 times in 20 under -race and 0 times in 20 without. Sweeps the rationales that asserted the memo was impossible, in index/rsfindex.go, index/mock.go, provider/provider.go and resolver/bench_test.go. The CHANGELOG diff is pure insertion; no dated section is touched.
jonyoder
added a commit
that referenced
this pull request
Aug 14, 2026
RSFIndex.Versions memoized the sorted order of a package's version keys and re-parsed every one of them on every call. It held strings because it had to: until go-python-packaging v0.6.0 a version.Version could not be shared between goroutines, since Version.Compare padded the shorter operand's release segment with append into spare capacity a by-value copy shares. v0.6.0 fixed that -- packable versions never touch part.Parts, and the fallback pads by copying -- and this takes the memo the fix unblocks. Warm resolution is 1.13x to 3.23x faster and allocates 41% to 94% fewer objects against the production snapshot, measured at base 11da678 over nine interleaved rounds. Cold does not improve, as expected: the first call already parsed the keys in order to sort them. candvers, metadata and the pin set are identical on every corpus entry, and 4,007 resolutions produce byte-identical transcripts with zero one-sided timeout exclusions.⚠️ These figures are larger than the same change measured against 6c13230 (1.24x and 2.27x). #46 shrank the denominator rather than the parse: parseKeys went from 47.4% of wide-versions' Resolve to 69.1%. Neither figure is quotable against the other's base. The returned slice is a COPY of the memo's, because cmd/pyresolve's `versions` subcommand sorts the result in place. Measured at -0.3% to +9.7% warm against a variant that shares -- higher than the "under 2%" this was scoped with, for the same denominator reason. It is also redundant on this module's own resolution path, since provider hands the result straight to candidate.Rank, which copies again; it protects the exported contract, and an internal accessor that skips it is a follow-up worth taking at that price. The cost is retained heap rather than churn, measured rather than estimated: a warmed index keeps roughly three times as much alive after a resolve (app-set 0.35 -> 0.99 MB, wide-versions 1.31 -> 4.41 MB), identical at both bases. The memo is left unbounded to match the blob cache's rationale, and the code now says plainly that a bound is a prerequisite for embedding this in a long-lived server. index/shared_memo_test.go shares one warmed memo across eight goroutines over fixtures chosen for the padding hazard's actual shape, covering both of v0.6.0's safety arguments and also driving version.ReleaseKey, which #46 made a second reader of a shared parsed version. Every fixture reports WARNING: DATA RACE under a v0.5.0 pin, 20 fresh processes each. That guard runs in CI on the -race step #44 added, which this is rebased onto. Without it these tests are a green no-op: measured on a tree with the defensive copy deleted, the pre-existing concurrency check catches it 8 times in 20 under -race and 0 times in 20 without. Sweeps the rationales that asserted the memo was impossible, in index/rsfindex.go, index/mock.go, provider/provider.go and resolver/bench_test.go. The CHANGELOG diff is pure insertion; no dated section is touched.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
pep440setno longer renders a version back to text to find its release group.Containsneeds each candidate version's(epoch, release)position. It derived that by callingBaseVersion()— which renders the version through abytes.Bufferwith onemath/bigdecimal conversion per segment — and then split the result back into digit runs, re-derivingcanonDigits/cmpDigits/cmpSegmentsover the text. Once per candidate version perContainscall.It now asks go-python-packaging for
version.ReleaseKey(new in gpp v0.7.0), which reads the parsedVersion's own epoch and release fields: ~16 ns and no allocation, against ~220 ns and 10 allocations for the render-and-split.releaseKey,canonDigits,cmpDigitsandcmpSegmentsare deleted.isDigitsstays —construct.gostill uses it.Why this is a root fix and not a third one
There is history. #33 derived a bound's key once instead of per comparison; #39 replaced the probe bound with a stack-held
verPos. Both left the derivation itself a string render, in two places:newPosKeyandverPos.init. FixingreleaseKeyfixes both, and leaves no fourth site — a memo over the old function was measured and deliberately rejected, because it would have made memory larger where this makes it smaller.Measured
Medians of three interleaved rounds (base, after, base, after, … so ambient load lands on both), ten iterations, against the production snapshot — 932,861 packages, dated 2026-08-04 — on an Apple M4 Max. Base is
6c13230(v0.6.0). Per-round spread was under 6% on every entry.single-no-depssmall-treeextrasapp-setwide-versionsbacktrackingunsatisfiablesingle-no-depssmall-treeextrasapp-setwide-versionsbacktrackingunsatisfiableThe allocation column is the one to read. What the render was buying was garbage:
app-setallocates 2.27x less warm while running 1.39x faster.Directly, from
pep440set's own micro-benchmarks (medians of three interleaved rounds):Contains/cross-groupContains/same-groupThree interleaved rounds of
-count 3each, on an otherwise idle machine; every run within ±2%. Cross-group — probing a span that anchors some other release, the common case — becomes allocation-free. Same-group still allocates 8 times, because it descends past the group key into the public-spelling comparison, which this change deliberately does not touch.In a CPU profile of the warm resolution,
Set.Containsfalls from 41% ofresolver.Resolveto 24% onapp-setand from 33% to 14% onwide-versions— 3.1x and 3.8x fewer samples in absolute terms.Resolve, not against total samples. The denominator has to be something the change does not move, and total samples is not: removing this allocation shrinks the profile's garbage-collection share too, so a "% of samples" figure would credit this change with that as well. The same binary'sContainsshare swings by 7x purely by changing-benchtime.strings.Splitleaves the profile entirely, butmath/big.nat.itoaandversion.writeReleasedo not —ensurePubstill renders the public spelling, which this change does not touch. They fall from 2.3% and 1.6% of samples underContainsto below the sampling floor, which is not the same as gone.unsatisfiableis not measurably faster. Its 1.07x is the median of three rounds, and a fourth confirmation round — run against the published gpp v0.7.0 rather than a localreplace— put it at 0.96x, slower. Read it as no change. That is the prediction met rather than a disappointment: it fails before enumerating many candidates, so it makes the fewestContainscalls in the corpus and has the least to gain. Its allocations do fall, by 1.25x, which is the effect appearing where the mechanism says it should. An entry gaining where it has no mechanism to would have meant the measurement was doing the work.That fourth round reproduced every other entry within noise (
app-set1.40x,wide-versions1.24x,extras1.32x,backtracking1.21x), which is also what confirms the published module behaves as the localreplacedid.version.Parseinsideindex.Versions— 16.9% of samples onwide-versions— which is exactly what that memo removes. Whichever lands second must re-measure against the first rather than carrying a column forward, for the same reason #39 and the ranking change needed a 2x2.Correctness
The two derivations are identical on every input reachable from a parsed
Version, and that is proved rather than argued:ReleaseKeyagainst the render-and-split algorithm this PR deletes, component by component, over 348,752 corpus versions plus 980,000 ordering pairs.PEP440SET_CORPUS_PACKAGES=20000— 20,000 packages, 305,548 specifier sets, 33,918,235 (specifier, version) pairs, 0 mismatches againstCheck. The rendering differential passes too: 611,096 renderings over the same 20,000 packages.FuzzFromSpecifiers, 120 s, no new failing input.TestBoundOrdering,TestBoundOrderingGrid,TestBoundOrderingTransitive,TestBoundOrderingPastInt64,TestCmpVerBoundAgreesWithCmpBound,TestContainsAgreesWithContainsBoundall green, and green under-race.ReleaseKeyto pypa/packaging 26.2's own release key (Version._key[0:2]) via a frozen fixture covering epochs and segments past a machine word, eight-segment releases, and leading zeros.Two arbitrary-precision invariants specifically survive, because both were shipped bugs here once: a release segment at or above 2^63 is still ordered by value (
TestBoundOrderingPastInt64), and1.0/1.0.0/1.0.0.0/1.00still occupy one position, whichEqualdepends on.One row added for a gap this change creates
TestBoundOrderingGridgains1.0.1.2.3.4.5vs1.0.1.2.3.4.6— a pair differing only past the sixth release segment. gpp packs a release into six 32-bit fields and falls back to arbitrary precision beyond that, so this pair straddles the fast path's edge. Mutation-checked: truncating gpp's packer at six segments instead of refusing makes this row fail withcmpBound(at(1.0.1.2.3.4.5), at(1.0.1.2.3.4.6)) = 0, want -1. Without the row, nothing in this module notices — gpp has its own layout tests, but a resolver picking the wrong version is the consequence here.isDigitsmoves toconstruct.go, which is now its only caller.The one test that changed shape
TestCanonDigitsis replaced byTestLeadingZeroSpellings. The old test existed because the key was built from digit runs compared length-first, where"007"would have sorted above"7", so the stripping was defensive code worth pinning. The key now comes from gpp's parsed release segments, which aremath/bigintegers —"007"and"7"are the same integer before a key exists, and there is no run to strip. The concern is gone rather than moved, but the behaviour it protected is not optional, so the replacement asserts it through the real path (cmpBound) at widths on both sides ofint64anduint64, and additionally asserts that the spelling has not flattened a real difference (1.007is1.7, not1.70and not1.0.7).Docs
resolver/bench_test.go's profile commentary namedreleaseKeyas a live frame in two places; both now say it no longer exists. Its "CONSTRUCTING the bound … 83% ofContains" diagnosis is marked superseded — #39 removed the bound, this removes what was under it, and the follow-up it proposed has nothing left to buy.🤖 Generated with Claude Code