perf(pep440set): probe containment with a stack-held position, not a bound - #39
Merged
Conversation
…bound Set.Contains materialized atBound(v) -- a public-spelling render, a possible re-parse, and a heap-allocated posKey -- purely to test the membership of a version the caller already holds. The resolver calls Contains once per candidate version per Candidates call, which made that derivation a measurable slice of warm resolution time. Contains now probes the spans with a verPos: the group key is derived up front (every comparison needs it first), and the public spelling waits until a comparison actually descends into a release group, which cross-group probes -- the common case -- never do. cmpVerBound is cmpBound with the left side specialized to edgeAt, and two agreement tests hold the fast path to the reference path: cmpVerBound against cmpBound over the full ordering grid, and Contains against containsBound over every span shape construct.go produces. Also carries the spike's measurement scaffolding: a struct-size logger and a retained-heap harness, from the span-size investigation this branch started as (752-byte spans are ~1% of warm resolve time on a complete index; the measured negative is recorded in the spike report). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The verPos path is single-owner by construction, but the bound side of every comparison is a SHARED posKey, and rstudio/go-version v0.0.2's Parts.Padding appends into spare capacity in place -- the reason a version.Version cannot be shared across goroutines for reads (go-version PR #5, unmerged). Every probe here ends in .0 so its release carries that spare capacity, and shares the bounds' release group so the ladder descends to pub.Compare rather than stopping at the group key. Both Contains and the containsBound reference path run, so a race in either is attributed correctly. Green today because two same-group versions have equal normalized release lengths, which never pads; the test exists so a change that lets cross-group probes reach pub.Compare cannot land silently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An unbriefed review seat demonstrated the latent hazard: init set v, epoch and release but left pubDone/public/pub/pubOK from the previous version, so re-initializing a used verPos kept the stale public spelling -- init(1.0.post1), compare (fills pub), init(1.0.dev0), compare returned +1 where the reference returns -1. No caller re-inits today, but the name promises it works, and hoisting one verPos out of a per-candidate loop is exactly the micro-optimization someone will try. init now clears the whole struct first, and TestVerPosReinit reuses one verPos across every (version, bound) pair -- verified to FAIL against the old body. Also from the same review: the retained-heap harness logs its delta in signed arithmetic (a shrinking live heap read as ~1.8e19), and the concurrent test now asserts that at least one probe actually reaches the pub.Compare arm instead of trusting the comment that says so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The 2-6% by-entry claim was measured against baa7af1's corpus, whose backtracking entry did not backtrack (#38 replaced it). Re-measured interleaved against 00e608e: the allocation cut is deterministic on all seven entries (5-9% fewer bytes per resolve); wall clock is 3-5% on the allocation-heavy entries and flat within noise on the rest, including the corrected backtracking entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jonyoder
force-pushed
the
spike/span-size
branch
from
August 14, 2026 11:50
96f6b8d to
4518741
Compare
Spans are canonical -- sorted by lo, disjoint -- so a probe below one span's floor is below every later span too, and the scan can stop. The pre-verPos containsBound had the same gap, so this is an added improvement, not a regression fix: below-range probes on a 15-span set drop 570 -> 224 ns and !=-hole probes 888 -> 665 ns (reviewer-measured), and the shape is exactly what a backtracking solve's != holes produce. Also from review: the concurrent test's comment now states in the file that the go-version padding race is UNREACHABLE from this path (green here must not be read as covering it); BenchmarkContains gains a same-group case so the lazy derivation is quoted from both ends (cross-group never renders, same-group pays the full ladder); containsBound is marked as the test-only reference path; the retained- heap harness warns its global MemStats reading is only meaningful solo; two test comments now say what the tests actually do. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The micro figures were medians of 5 from an earlier session; the entry described them with the warm A/B's methodology. They are now re-measured same-session pairs, quoted from both ends of the lazy derivation (cross-group never renders; same-group pays the full ladder), and the early exit's end-to-end contribution is stated as below the noise floor rather than folded into the headline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jonyoder
added a commit
that referenced
this pull request
Aug 14, 2026
… and re-measure on 73d820a Review findings, and a re-measurement forced by the base moving under the branch.⚠️ THE MEMO DIFFERENTIAL DID NOT OBSERVE THE MEMO. It reported "N calls served from a warm memo" as calls-minus-packages, which is arithmetic over its own loop shape and nothing else. Neuter the memo lookup so every call misses and the test still passed, still printing the identical figure -- and that figure had been quoted in CHANGELOG.md and the PR description as a measurement. It is now read through a counting index and asserts one Versions() call per DISTINCT package, which is not derivable from the loop: 14,379 calls for 14,379 packages, so 68,255 of 82,634 (82.6%) came from the memo. Verified by the mutation the old one slept through -- always-miss now fails it at 940 reads for 136 packages. Also adds the test for the property the memo's whole argument is phrased in, and did not have. Candidates rests on candidate.Rank being a STABLE sort, and stability says nothing unless elements TIE -- but the default Newest policy is total on real index output (Versions collapses each PEP 440 equality class) and the only other Policy in these tests, oldestFirst, is total too. An UNSTABLE sort passed every existing provider test. TestCandidatesUnderATiedPolicy... drives a deliberately non-total policy (rank by major version only) through Candidates against ExactCandidates over 17 ranges, 13 of them choosing a version inside a tie group of more than one.⚠️ My first draft of the companion stability test hardcoded the expected order from an assumption about MockIndex's ordering. The assumption was wrong and it failed against correct code -- a hardcoded expectation had turned a stability test into an index-ordering test. It now asserts the property (tied versions keep the index's relative order, whatever that order is) and is stable over -count=8.⚠️ The transitivity test's equivalence half was ~99.997% degenerate while its guard certified it. Classes held at most 2 spellings, so a "mutually equivalent triple" had to repeat an element: either a == c, where the conclusion is reflexive and cannot fail, or the antecedent restated. 749,465 satisfied antecedents contained 2 genuine witnesses. Injecting a second alternate spelling takes that to 96,989, and the guard now counts WITNESSES rather than antecedents. Stale and wrong numbers in production doc comments, all introduced by the first commit and never updated when later commits re-measured: * provider.go said the corpus fell "1.3x to 9.6x"; it is 1.4x to 11.8x. * policy.go said the pass took wide-versions "from 80.0 ms to 8.2 ms". 8.2 ms matches nothing for wide-versions -- 8.14 ms was APP-SET's number. Replaced with the comparison counts it is actually about. * policy.go claimed "at most 2(n-1) Less calls" and "two Less calls on the pair that settles it". The loop makes exactly ONE call per pair: at most n-1. * Candidates still said the change keeps cost "proportional to the packages actually decided rather than to every version in range". True of METADATA reads only; the walk is O(all versions) by construction, which is why Contains is 28.6% of the call. * BenchmarkSelectFirst's "O(1) versus O(n)" is a claim about ranking, not about a Candidates call, which does not break out of its walk. Documents three things the memo narrows that nothing recorded: retained memory traded for churn (and NOT measured at peak), the loss of a cancellation checkpoint on a call with nothing in range, and a transient Versions() failure becoming unobservable after a first success. Re-measured against 73d820a, since #39 changed Contains, which sits inside Candidates. Medians of three INTERLEAVED runs -- one round was contended and put the baseline's small-tree at 8.10 ms against 4.31 and 4.22, which is what medians are for. entry base after single-no-deps 1.77 -> 0.28 6.3x small-tree 4.31 -> 1.39 3.1x extras 9.68 -> 1.90 5.1x app-set 67.10 -> 5.67 11.8x wide-versions 80.33 -> 15.02 5.3x backtracking 24.51 -> 3.29 7.5x unsatisfiable 0.73 -> 0.51 1.4x⚠️ And a 2x2, because the two changes are NOT separable by subtraction. All four cells interleaved in one session: #39 alone is 1.02x/1.12x/1.02x on the three large entries, this change alone is 9.19x/5.43x/6.90x, and #39 ON TOP of this change is 1.29x/1.12x/1.11x. #39 got more valuable, mechanically: Contains was 4.4% of a Candidates call before this work and 28.6% after. Neither change may be credited with the other's contribution. Discloses that every corpus figure comes from a local full-snapshot run; CI falls back to the 139-package excerpt, and its anti-skip guard covers ./index/ only. Records the transcript harness's exact invocation (the committed default deadline is 10s; the published runs used 8s) and that its excluded set is BIASED toward the hardest resolutions, since the faster side times out less often.
jonyoder
added a commit
that referenced
this pull request
Aug 14, 2026
…tead of sorting (#40) * spike(candidate,provider): rank once per package, and detect order instead of sorting A spike, not a proposal: measure where a Candidates call actually spends its time now that the found/rank change cut metadata reads by up to 190x, and prototype whatever the profile names. The profile names the sort, unambiguously. Warm against the production snapshot, candidate.Rank is 83% of Candidates on the benchmark's app-set entry and 85% on wide-versions; the usability walk that the found/rank change had just optimized is 1.7% and 0.6%, index.Versions is ~10%, and pep440set.Contains -- the whole "intersect the version list" step -- is 4-5%. Nothing outside the sort is worth attacking. Two independent causes, both fixed here: * The same lists are re-ranked constantly. The solver re-asks about a package on every round it reconsiders it, and each call sorted from scratch. Provider now memoizes candidate.Rank over the package's FULL version list for the life of the resolution, and each call walks that order filtering by allowed. Ranking the superset rather than the in-range list is what makes it memoizable: the in-range list is a function of caller-supplied input. The memo lives on the Provider and not on the index deliberately. A version.Version cannot be shared across goroutines even for reads -- index/rsfindex.go documents that upstream defect at length and declines to memoize parsed versions because of it -- and a Provider serves exactly one resolution and is documented as unsafe for concurrent use, so nothing here is read from two goroutines. On a shared index this would reintroduce the race in full. * index.RSFIndex returns versions ASCENDING and the default Newest policy wants them descending, so sort.SliceStable was being handed its worst case on essentially every call. candidate.Rank now classifies the input in one linear pass and reverses it, or returns it untouched, when it can; otherwise it sorts as before. Detection, not assumption -- MetadataIndex promises no ordering, and the branch a real list takes is measured rather than assumed. Warm resolution, production snapshot, 10 iterations, before -> after: single-no-deps 1.63 ms -> 0.33 ms 5.0x small-tree 4.28 ms -> 1.59 ms 2.7x extras 9.76 ms -> 2.32 ms 4.2x app-set 68.98 ms -> 7.22 ms 9.6x wide-versions 81.17 ms -> 16.54 ms 4.9x backtracking 6.36 ms -> 1.36 ms 4.7x unsatisfiable 0.70 ms -> 0.52 ms 1.3x candvers, the metric the found/rank change did not move at all, finally moves: app-set 6040 -> 943, wide-versions 7206 -> 4647. Equivalence, checked rather than argued: * candidate: the fast path against a sort-only reference, on 13 hand-built shapes (including the tie cases the reversed branch claims are impossible), 20,000 random lists, and every version list of 200,000 production packages (1,623,115 versions; 103,358 lists take the reversed branch, 42,609 the ordered one, 0 fall through to the sort). * candidate: Newest is a genuine strict weak ordering on production data -- 124,918 ordered triples and 749,465 equivalence triples, no violation.⚠️ The equivalence half needs both injected equal spellings and biased sampling to be non-vacuous, because Versions() dedupes PEP 440 equality classes: on real index output the order is TOTAL and stability is vacuous. * provider: the existing differential against ExactCandidates over 60,000 production packages, 42,355 with something available, 16 of them discriminating. ExactCandidates no longer calls candidate.Rank, since Rank is now part of what is under test. * provider: a NEW differential for the memo, which the existing one cannot reach -- it asks each package once, always with All(), and the memo only does anything on the second call with a DIFFERENT allowed set. 82,634 calls over 14,540 production packages, 68,094 of them served from a warm memo. Not pursued, with the reason: a cheaper intersection (binary search over a sorted list rather than a walk) is 4-5% of the call at most, and pre-release admission breaks the contiguity it would need. * spike(provider): measure contiguity, and make the harness lint-clean Two additions to the spike, both measurement rather than mechanism. TestInRangeIsNotContiguous decides the "intersect with two binary searches instead of a walk" question with data instead of an argument. Over 72,976 production packages and 807,795 versions: 12.38% of versions are pre-releases, 10.37% of packages publish at least one, and 4.18% of packages have their admitted set SPLIT by one -- so for those, no pair of binary searches over a version-ordered list can delimit the admitted set, because a pre-release sits between two finals in version order and removing it breaks the run. 95.8% are contiguous, so the idea is viable WITH a fallback, not without one.⚠️ It is also aimed at the wrong half of the cost. Set.Contains is 28.6% of a Candidates call after this spike, and 83% of THAT is pep440set.atBound -- building the probe bound for the version, not walking the spans. Memoizing the probe alongside the ranked list would take the same cost out with no contiguity assumption at all; it needs a pep440set API that does not exist yet. TestRSFIndexVersionsAreAscending pins the property the fast path was built for, across 51,521 multi-version packages: 0 adjacent inversions, 0 adjacent PEP 440-equal pairs.⚠️ It is a check on the IMPLEMENTATION, not the contract -- MetadataIndex promises no ordering, which is why Rank detects the shape rather than assuming it. It exists so that if RSFIndex ever stops being sorted, the reason the fast path went quiet is discoverable rather than mysterious. Mutation-tested, because a differential that cannot fail is decoration: * Delete the reverse from Rank's reversed branch and the synthetic shapes, the 20,000 random lists, the real-index comparison, the memo differential and four pre-existing policy tests all fail. * Poison the memo with the RANGE-FILTERED list -- the "keyed by caller-supplied input" bug -- and exactly ONE test in the whole module fails: the new memo differential. The pre-existing exact-count differential passes, because it asks each package once with All(); so does the entire resolver suite. Also silences errcheck on the transcript harness. The bufio.Writer defers every error to Flush, so the per-call returns carried nothing; Flush is now checked and fails the run, which the deferred one could not do. * spike(candidate,resolver): measure the alternatives, and record the result BenchmarkSelectFirst measures the three ways of answering the question Candidates actually asks -- which version to try first -- rather than assuming which is best. sort (what every call used to do), scan (one linear pass, the partial-selection idea), and memo (the list is already ranked, take [0]).⚠️ scan answers a STRICTLY EASIER question than the other two: it finds the single best element and cannot produce the second-best, which Candidates needs whenever the best version is unusable. Its number is a LOWER bound on a real partial-selection implementation, not an estimate of one -- and it is still O(n) comparisons per call where the memo is O(1), against a solver that asks about the same package 2.4 to 4.8 times per resolution. Measured anyway, because "the memo wins" is a claim about a ratio and the ratio is what decides whether the memo's retained memory is worth paying for. TestFirstByScanAgreesWithTheSort keeps that benchmark honest over 20,000 random lists: an alternative that returned a different element would be timing the wrong algorithm. resolver/bench_test.go records the result and, more importantly, corrects itself. The previous section closed with "the remaining cost is in walking and intersecting those lists -- set algebra and version comparison". It named two suspects and the profile convicted exactly one: version comparison inside the sort, 83-86% of a Candidates call. The set algebra it also blamed -- pep440set.Set.Contains, the entire intersection step -- was 4.4% on app-set and 5.2% on wide-versions. Also corrects a figure I had wrong in three places: app-set's 87 Versions() calls span 18 distinct packages, not 27. It is measurable rather than estimatable now, because after the memo `versions/op` IS the distinct-name count -- each name triggers exactly one call. * spike(resolver): the final numbers, and the equivalence transcript result Re-measured both sides back to back on an idle machine, median of three runs of ten iterations each, and replaced the single-run figures with those. Warm: single-no-deps 1.64 ms -> 0.30 ms 5.5x small-tree 4.29 ms -> 1.42 ms 3.0x extras 9.74 ms -> 2.31 ms 4.2x app-set 68.49 ms -> 7.19 ms 9.5x wide-versions 82.50 ms -> 16.38 ms 5.0x backtracking 6.67 ms -> 1.29 ms 5.2x unsatisfiable 0.77 ms -> 0.51 ms 1.5x Equivalence is now measured end to end rather than at the Candidates seam. 4,007 resolutions against the production snapshot -- the seven corpus entries plus 4,000 sampled package names -- produce BYTE-IDENTICAL transcripts before and after: same pins, same decision ORDER, same activated extras, and the same failure report text on the 1,605 cases that fail. 48 cases where either side hit an 8-second deadline are excluded and counted, because a wall-clock deadline is not a property two builds share.⚠️ That closes a gap provider/unusable.go names explicitly: "the differential compares found, best and rank, not Unusable() and not rendered reports". The transcript compares rendered reports, on 1,605 of them. The mechanism numbers are isolated too, from candidate/'s comparison-counting benchmarks: * On ascending input -- what RSFIndex returns -- detection replaces ~9.8 comparisons per element with one: 13,999 against 137,387 at n=14,000. *⚠️ On shuffled input, where detection fails and the sort runs anyway, it costs 2-3 extra comparisons TOTAL (243,629 against 243,626), because the pass breaks at the first pair that rules both shapes out. The regression case is free, not a tradeoff. * Partial selection, measured rather than dismissed: a linear max-scan is 9.8-10.0x faster than sorting and allocates nothing. Real, and strictly dominated -- the memo answers in 2.7-3.8 ns with no comparisons, against a solver that asks 2.4 to 4.8 times per package. Also corrects the previous section in place. It closed by blaming "set algebra and version comparison"; the profile convicted version comparison inside the sort and cleared the set algebra, which was 4.4% and 5.2%. * perf(provider,candidate): rank once per package, and detect order instead of sorting Re-measured against the CORRECTED backtracking entry from #38 and added the changelog entry. The numbers in the previous commit were taken against the retired `pandas, numpy<2` entry and are not comparable; these replace them.⚠️ The backtracking row is the one to read. It is the only corpus entry whose purpose is that the solver reconsiders the same package repeatedly, which is exactly what the memo targets, and it improves MORE than the retired entry did: 6.7x against 5.2x, with the largest candvers drop of the corpus (2,495 -> 351, 7.1x) and the largest Versions() drop (30 calls -> 6). entry warm ms candvers Metadata before after before after single-no-deps 1.68 0.32 130 65 3 3 5.2x small-tree 4.38 1.58 984 254 30 30 2.8x extras 9.77 2.55 1658 321 43 43 3.8x app-set 72.44 8.14 6040 943 105 105 8.9x wide-versions 83.16 17.13 7206 4647 24 24 4.9x backtracking 25.27 3.78 2495 351 42 42 6.7x unsatisfiable 0.75 0.54 124 124 3 3 1.4x Medians of three runs per side, both sides back to back in one session on an idle machine.⚠️ Medians rather than means because one app-set baseline run came in at 118.79 ms against 72.44 and 69.22 -- a transient, and a mean would have carried it. #38's TestEveryReasonIsRecordedWhenNOTHINGIsUsable passes unchanged, which is the check that matters for the rebase: the not-found path still walks every in-range version, so every reason a failure report needs still survives. * fix(provider,candidate): make the memo differential observe the memo, and re-measure on 73d820a Review findings, and a re-measurement forced by the base moving under the branch.⚠️ THE MEMO DIFFERENTIAL DID NOT OBSERVE THE MEMO. It reported "N calls served from a warm memo" as calls-minus-packages, which is arithmetic over its own loop shape and nothing else. Neuter the memo lookup so every call misses and the test still passed, still printing the identical figure -- and that figure had been quoted in CHANGELOG.md and the PR description as a measurement. It is now read through a counting index and asserts one Versions() call per DISTINCT package, which is not derivable from the loop: 14,379 calls for 14,379 packages, so 68,255 of 82,634 (82.6%) came from the memo. Verified by the mutation the old one slept through -- always-miss now fails it at 940 reads for 136 packages. Also adds the test for the property the memo's whole argument is phrased in, and did not have. Candidates rests on candidate.Rank being a STABLE sort, and stability says nothing unless elements TIE -- but the default Newest policy is total on real index output (Versions collapses each PEP 440 equality class) and the only other Policy in these tests, oldestFirst, is total too. An UNSTABLE sort passed every existing provider test. TestCandidatesUnderATiedPolicy... drives a deliberately non-total policy (rank by major version only) through Candidates against ExactCandidates over 17 ranges, 13 of them choosing a version inside a tie group of more than one.⚠️ My first draft of the companion stability test hardcoded the expected order from an assumption about MockIndex's ordering. The assumption was wrong and it failed against correct code -- a hardcoded expectation had turned a stability test into an index-ordering test. It now asserts the property (tied versions keep the index's relative order, whatever that order is) and is stable over -count=8.⚠️ The transitivity test's equivalence half was ~99.997% degenerate while its guard certified it. Classes held at most 2 spellings, so a "mutually equivalent triple" had to repeat an element: either a == c, where the conclusion is reflexive and cannot fail, or the antecedent restated. 749,465 satisfied antecedents contained 2 genuine witnesses. Injecting a second alternate spelling takes that to 96,989, and the guard now counts WITNESSES rather than antecedents. Stale and wrong numbers in production doc comments, all introduced by the first commit and never updated when later commits re-measured: * provider.go said the corpus fell "1.3x to 9.6x"; it is 1.4x to 11.8x. * policy.go said the pass took wide-versions "from 80.0 ms to 8.2 ms". 8.2 ms matches nothing for wide-versions -- 8.14 ms was APP-SET's number. Replaced with the comparison counts it is actually about. * policy.go claimed "at most 2(n-1) Less calls" and "two Less calls on the pair that settles it". The loop makes exactly ONE call per pair: at most n-1. * Candidates still said the change keeps cost "proportional to the packages actually decided rather than to every version in range". True of METADATA reads only; the walk is O(all versions) by construction, which is why Contains is 28.6% of the call. * BenchmarkSelectFirst's "O(1) versus O(n)" is a claim about ranking, not about a Candidates call, which does not break out of its walk. Documents three things the memo narrows that nothing recorded: retained memory traded for churn (and NOT measured at peak), the loss of a cancellation checkpoint on a call with nothing in range, and a transient Versions() failure becoming unobservable after a first success. Re-measured against 73d820a, since #39 changed Contains, which sits inside Candidates. Medians of three INTERLEAVED runs -- one round was contended and put the baseline's small-tree at 8.10 ms against 4.31 and 4.22, which is what medians are for. entry base after single-no-deps 1.77 -> 0.28 6.3x small-tree 4.31 -> 1.39 3.1x extras 9.68 -> 1.90 5.1x app-set 67.10 -> 5.67 11.8x wide-versions 80.33 -> 15.02 5.3x backtracking 24.51 -> 3.29 7.5x unsatisfiable 0.73 -> 0.51 1.4x⚠️ And a 2x2, because the two changes are NOT separable by subtraction. All four cells interleaved in one session: #39 alone is 1.02x/1.12x/1.02x on the three large entries, this change alone is 9.19x/5.43x/6.90x, and #39 ON TOP of this change is 1.29x/1.12x/1.11x. #39 got more valuable, mechanically: Contains was 4.4% of a Candidates call before this work and 28.6% after. Neither change may be credited with the other's contribution. Discloses that every corpus figure comes from a local full-snapshot run; CI falls back to the 139-package excerpt, and its anti-skip guard covers ./index/ only. Records the transcript harness's exact invocation (the committed default deadline is 10s; the published runs used 8s) and that its excluded set is BIASED toward the hardest resolutions, since the faster side times out less often. * fix(provider): stop the memo differential wedging in its own fixture builder⚠️ THE TEST COULD NOT BE RUN THE WAY ITS OWN DOC INVITED. Pointed at a full snapshot with no sample cap -- the obvious way to reproduce the figures quoted for it -- it swept 680,711 packages and ran for 34 minutes without reaching an assertion, failing the whole provider package on the 40-minute limit. The hang was in the TEST'S OWN FIXTURE BUILDER, not in anything under test. A profile of it put the stack in rangesOver -> Union -> newSet -> cmpBound -> version.Compare -> version.String on a package with 905 versions. rangesOver built its allowed sets by unioning per-version singletons, which re-canonicalizes a span list that has grown by one each time: quadratic in cmpBound, and cmpBound reaches the go-python-packaging v0.5.0 hot spot that resolver/bench_test.go already points at. So scaling the corpus knob up bought cmpBound cost rather than coverage. Ranges are now built through specifiers -- >=, <, and != for a two-span set -- so each is one or two spans regardless of how many versions a package has. The union-built gappy shape is kept, because it is the one no pair of binary searches can express and therefore the one worth having, but only for lists of 64 or fewer. And the default sample is capped at 20,000 rather than "all", with GPR_SAMPLE=0 still meaning all for anyone who wants it. A bare full-snapshot run now takes 57 seconds and covers MORE: 109,564 calls against the previous 82,634, because the specifier-built ranges are more varied than the windows they replace. Also corrects corpus figures that were quoted as though they were full-corpus and came from a ~10% sample, understating the corpus about 9x. Re-run with GPR_SAMPLE=0: contiguity 72,976 packages / 807,795 versions -> 680,711 / 7,666,753 split by a pre-release 4.18% -> 4.23% ascending 51,521 multi-version packages -> 481,998 The percentages moved by ~0.05pp, so the conclusion they support is unchanged -- but the absolute counts were wrong and read as authoritative. And records the configuration behind the transitivity figures (GPR_SAMPLE=3000 GPR_TRIPLES=3000000), which the default run does not reproduce. Same reproducibility gap as the transcript harness's, and the same fix: write the invocation next to the number or the number is not checkable. * test(resolver): measure peak heap, which falls rather than rising Closes the review findings, and replaces a documented prediction with a measurement that contradicts it.⚠️ THE RETAINED-MEMORY CAVEAT WAS WRONG, in the safe direction. The memo holds the parsed version list of every package in the closure for the whole resolve, where those were previously garbage, so the obvious reading is that it trades allocation churn for a higher high-water mark -- and B/op cannot settle it, because B/op is cumulative rather than peak. The doc asserted that trade-off as though it were established. It was a prediction, and peak heap goes the other way: entry peak over baseline base after single-no-deps 1.3 -> 0.3 MB small-tree 4.6 -> 1.3 MB extras 9.5 -> 2.0 MB app-set 53.9 -> 7.4 MB 7.3x lower wide-versions 56.2 -> 18.6 MB 3.0x backtracking 23.4 -> 5.7 MB 4.1x unsatisfiable 0.4 -> 0.3 MB Reproducible across three runs a side, within 4%. The reason is that the churn removed was never short-lived: the old path allocated a fresh in-range slice AND a fresh Rank copy on each of app-set's 87 calls, which pile up inside one GC cycle, against 18 retained lists now and no in-range slice at all. Fewer live bytes, not more. TestPeakHeapDuringOneResolve samples runtime.ReadMemStats during one resolution and keeps the maximum.⚠️ It is a SAMPLED maximum -- ReadMemStats stops the world, so the interval bounds how sharp a spike it can see -- which makes it a floor on the true peak, not the true peak. It also perturbs GC timing, so only the before/after comparison means anything and both sides are sampled the same way. Skipped unless GPR_PEAK is set. Remaining review findings: * A stale ratio in bench_test.go: prose said the backtracking entry's "6.7x is above the 5.2x the retired entry showed" while the table 15 lines up already read 7.5x -- left over from the pre-re-measurement draft, in the paragraph that tells the reader this is the row to read. * Three different runtimes for one run recorded in three places (30s, 57s, 85s; the 85s predates the specifier-built ranges). Replaced with "well under a minute" and a note on why a precise figure invites this. * realVersions fataled on a version it merely cannot use. Appending ".0" is PEP 440-equal for a release segment and NOT for a base carrying a local label -- "1.0+abc" gives "1.0+abc.0", which parses and compares unequal. PyPI rejects local versions on upload so it cannot fire today, but a fixture that aborts on a data property is a red test waiting to happen. It now skips the class; the vacuity guard catches "too few classes". And the equivalence transcript is re-run at the CURRENT base, which the earlier ones were not -- they were taken at baa7af1 and 00e608e, both before 73d820a changed Set.Contains inside Candidates. 1,199 of 1,199 compared cases identical, 500 of them full failure reports, 8 excluded for deadline.
jonyoder
added a commit
that referenced
this pull request
Aug 14, 2026
The bump entry asserted the packed key and #40 do not compose multiplicatively. That was reasoning, not a measurement. This records the 2x2 that establishes it, in one interleaved session, warm, medians of 3. The packed key is worth 1.99-5.93x BEFORE #40 and 1.23-1.88x after; #40 is worth 1.43-12.34x at gpp v0.5.0 and 1.07-2.95x at v0.6.0. Both directions shrink, so the two are substitutes: on app-set, multiplying the isolated headlines predicts 65.8x against a measured 15.7x end to end. That is the opposite sign to the #39/#40 interaction, where Contains became MORE valuable after the rank memo, so the interaction cannot be guessed. It also resolves an apparent conflict with go-python-packaging's own release notes, which claim 2.1-5.2x warm on this benchmark: those were measured at 73d820a, before #40 landed 46 minutes later. The pre-#40 column reproduces them (1.99-5.93x). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jonyoder
added a commit
that referenced
this pull request
Aug 14, 2026
#41) Bumps go-python-packaging v0.5.0 -> v0.6.0. No code in this repository changes; the diff is go.mod, go.sum, a CHANGELOG entry and two doc corrections. v0.6.0 compares versions through a packed 4xuint64 order-preserving key rather than a field-by-field walk, for 97.3% of real versions. Against the production snapshot (932,861 packages), medians of five interleaved rounds: cold resolution 2.0x to 13.4x faster, warm 1.2x to 1.9x. Cold gains far exceed warm, and that asymmetry is the finding: building a package's sorted version order is comparison-bound and happens once per package per index, so it lands in cold (wide-versions 167.35 -> 12.46 ms). Warm reuses that order through 0.5.0's memo, so the packed key only reaches the residual comparisons inside pep440set containment and candidate filtering. The interaction with #40 is measured, not asserted: the two CONTEND. The packed key is worth 1.99-5.93x before #40 and 1.23-1.88x after; #40 is worth 1.43-12.34x at v0.5.0 and 1.07-2.95x at v0.6.0. On app-set, multiplying the isolated headlines predicts 65.8x against a measured 15.7x end to end. That is the opposite sign to the #39/#40 interaction, so these cannot be guessed. Equivalence: 4,007 resolutions against the production snapshot (7 corpus entries plus 4,000 sampled names, seed 1) produce byte-identical transcripts before and after -- identical pins, decision order, activated extras, and failure report text on the 1,607 that fail -- with 36 wall-clock-deadline cases excluded. candvers, metadata and the pin set are identical everywhere. Also inherits an upstream data-race fix: a parsed Version is now safe to share across goroutines. That restriction was documented on the exported PackageMetadata.SupportsPython, which told callers to give each goroutine its own version.Parse; that doc and the canonical account in RSFIndex.Versions are corrected. Taking the now-unblocked parsed-version memo is left to #42. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 14, 2026
jonyoder
added a commit
that referenced
this pull request
Aug 14, 2026
…er (#46) * perf(pep440set): take the release group key from gpp, not from a render 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. * docs(changelog): the nanosecond figures are load-sensitive, the allocation 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.
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
Set.Containsno longer materializesatBound(v)to test membership. It probes the spans with a stack-heldverPos: the group key is derived up front (every comparison needs it first), and the public spelling — a render plus, for a version carrying a local label, a re-parse — is derived lazily, only when a comparison descends into a release group. Cross-group probes, the common case, never pay it.cmpVerBoundiscmpBoundwith the left side specialized toedgeAt;containsBoundremains as the reference path.Base:
00e608e(origin/main, rebased). The branch was first cut from a stale localmainfour commits behindbaa7af1, then rebased ontobaa7af1, then rebased again onto00e608eafter #38 merged (the CHANGELOG conflict from that merge is what silently suppressed every CI run — see the thread). The warm end-to-end A/B below was re-measured on the rebased base, including #38's correctedbacktrackingentry; the micro and correctness numbers from thebaa7af1session are labeled where they appear.Why
The resolver tests membership once per candidate version per
Candidatescall (provider.go's in-range filter). On the warm profile that madeSet.Contains2.0% of samples, essentially all of it innewPosKey. With thecandidate.Rankfast path in progress elsewhere expected to remove the current dominant cost, this was on track to be roughly a third of the residual; after this changeContainsis 0.54% of samples.Review follow-up:
Containsnow also stops at the first span whose floor is above the probe — spans are canonical (sorted, disjoint), so nothing later can match. The pre-PRcontainsBoundhad the same gap, so this is an added improvement, not a regression fix. Reviewer-measured micro on a 15-span set: below-range probes 570 → 224 ns (−61%),!=-hole probes 888 → 665 ns (−25%) — the shapes a backtracking solve's!=holes produce. Its end-to-end contribution is below this machine's noise floor (see below); it is kept for the micro win.Measured (Apple M4 Max, go1.26.4, prod snapshot 932,861 pkgs)
Microbenchmark (
BenchmarkContains, same-session before/after pairs, medians of 5). The two cases bound the lazy derivation from both ends:Warm end-to-end, re-measured on the rebased base
00e608e(BenchmarkResolveWarm, interleaved A/B ×3 rounds, medians), including #38's corrected genuinely-backtracking entry:00e608e)The allocation cut is deterministic on all seven entries; the wall-clock benefit is 3–5% on the allocation-heavy entries and flat within noise on the rest. (An earlier session on
baa7af1read −2.3% to −6.5% across all seven; the rebased session is the one this PR claims. Noise floor on this shared machine is roughly ±1.5%, and was visibly worse in the final session.) Retained heap perSetunchanged (2829.7 → 2829.6 B).A third interleaved A/B (×3 rounds) after adding the early exit: the branch won all 21 of 21 within-round entry comparisons against
00e608e(sign test alone puts the direction beyond doubt), with per-entry medians in the same −2.5% to −9% band — but that session's baseline rounds ran hot relative to the identical commit the day before, so the magnitudes are not upgraded on its evidence and the 3–5% claim above stands. The early exit itself moved nothing end-to-end that survives the noise floor.Correctness evidence
TestBoundOrdering,TestBoundOrderingGrid,TestBoundOrderingTransitive,TestBoundEqualPositions,TestEqualSpellingsCanonicalizeAlike: pass — the strict total order and the1.0/1.0.0/1.0.0.0/1.00single-position invariant are untouched (releaseKeyis shared, not reimplemented).TestCmpVerBoundAgreesWithCmpBound(full ordering grid × 34 version spellings, oneverPosreused per row so the lazy derivation runs in every order) andTestContainsAgreesWithContainsBound(every span shapeconstruct.goproduces, each also complemented).PEP440SET_CORPUS_PACKAGES=20000: 33,918,235 (specifier, version) pairs over 20,000 packages, zero mismatches. The differential assertsContains— this path — againstversion.Specifiers.Check.FuzzFromSpecifiers120 s: 8.63M execs, 0 failures (the target also assertsContainsvsCheck).go test ./...andgo test -race ./...green; pypa/packaging 26.2 spot-checks agree.Concurrency
No new sharing:
verPosis call-local and mutated only by its owning goroutine; the shared object in every comparison remains the bound-sideposKey, exactly as before. NewTestContainsConcurrent(8 goroutines × 500 iterations against one sharedSet, probes ending in.0so their releases carry the spare capacity the go-version padding hazard needs, sharing the bounds' release group so the ladder reachespub.Compare) is green under-raceon both paths. Structural note: same-group comparisons have equal normalized release lengths, soParts.Paddingnever appends in place on this path today — that is why it is clean, not evidence that go-version v0.0.2 is safe to share. go-version PR #5 remains the real fix and is not merged.Recorded negative: shrinking
spandoes not pay (complete-index regime)This branch began as the span-size spike, and the negative belongs on the record:
unsafe.Sizeof(committedsize_test.go):version.Version352 B,posKey416 B,bound376 B,span752 B.baa7af1, the entire set algebra that copies those spans —Intersect,Complement,newSet,appendSpan,versionset.Difference— totals ~1% of warm resolve time against the complete production index. A pointer-boundshrink (752 → ~32 B per span) has no room to produce a meaningful end-to-end win there, and the prototype was deliberately not built.Scaffolding carried on the branch
size_test.go(struct-size logger),heap_test.go(retained-heap harness),verpos_race_test.go(concurrent probe). All assert nothing beyond their purpose and run in normalgo test.🤖 Generated with Claude Code