Memoize parsed versions in index.RSFIndex, not just version keys

[jonyoder]

Closes #42. Base: 61029b6 (#44). The performance figures below were measured at 11da678, the commit immediately before it; #44 is test-and-CI only and changes no code on any measured path, so they carry. Everything before that was re-measured from scratch after #46 moved the base — see Measurement provenance.

index.RSFIndex memoized the sorted order of a package's version keys and re-parsed every one of them on every Versions call. It held strings because it had to: until go-python-packaging v0.6.0 a parsed version.Version could not be shared between goroutines, because 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 #41 deliberately left the memo for its own measurement. This is that change.

Roughly fifteen lines in RSFIndex.Versions. The rest is measurement, concurrency coverage, and the sweep of the internal rationales that asserted the memo was impossible.

What it buys

Profiled at the base commit as a share of resolver.Resolve's own cumulative cost, parseKeys was 22.7% of a warm app-set resolution and 69.1% of a warm wide-versions one, and 58.4% and 94.8% of the objects those resolutions allocated.

Medians of nine interleaved rounds, 500 iterations, production snapshot (932,861 packages), Apple M4 Max:

entry	warm before	warm after		allocs/op before	after
single-no-deps	0.164 ms	0.090 ms	1.82x	2,258	542	−76.0%
small-tree	0.795 ms	0.555 ms	1.43x	10,901	4,634	−57.5%
extras	1.110 ms	0.836 ms	1.33x	15,114	7,143	−52.7%
app-set	3.164 ms	2.318 ms	1.37x	42,400	18,906	−55.4%
wide-versions	6.271 ms	1.944 ms	3.23x	127,661	7,181	−94.4%
backtracking	2.235 ms	1.985 ms	1.13x	21,020	12,395	−41.0%
unsatisfiable	0.325 ms	0.221 ms	1.47x	5,170	2,054	−60.3%

⚠️ These are LARGER than the same change measured against the old base, and neither number is wrong. Against 6c13230 this was 1.24x on app-set and 2.27x on wide-versions. #46 removed a render-and-split from pep440set.Contains, which does not make the parse cheaper — it makes the parse a bigger share of what is left. parseKeys went from 47.4% of wide-versions' Resolve to 69.1% on exactly that account. The two changes are not additive in either direction, and a figure from one base is not quotable against the other.

Amdahl reconciles almost exactly where the parse dominates: removing 69.1% predicts 3.24x, wide-versions measures 3.23x. On app-set it under-predicts — 22.7% predicts 1.29x against 1.37x measured — because the change also removes 58.4% of the objects allocated.

Cold does not improve: 0.96x to 1.01x, allocation flat to four significant figures. Expected — the first call per package already parsed its keys in order to sort them, so there is nothing to save, and what is left is the copy this now makes on the way out.

candvers, metadata and the pin set are identical on every entry.

The RFD 0001 Phase 3 warm gate (under 1 ms) is now met by 4 of 7 against 3 of 7 at the base — extras crosses at 0.836 ms.

⚠️ #46's caveat that unsatisfiable is not measurably faster stands and is not superseded. That note is about Contains calls, which that entry makes few of. Its 1.47x here is a different mechanism — it parses version keys like everything else, and that is what this removes.

⚠️ The real cost is retained heap

Parsed versions of every package ever asked about now live for the life of the index, where they used to be transient garbage. Measured by a new TestIndexRetainedHeapAfterResolve (GPR_RETAIN=1) as the live heap one warmed RSFIndex keeps alive after a resolution completes — medians of five interleaved rounds, agreeing to the centibyte, and identical at both bases (it is a live-heap measurement, so it does not move with the base or with load):

entry	retained before	retained after
single-no-deps	0.01 MB	0.06 MB	6.0x
small-tree	0.07 MB	0.24 MB	3.4x
extras	0.09 MB	0.30 MB	3.3x
app-set	0.35 MB	0.99 MB	2.8x
wide-versions	1.31 MB	4.41 MB	3.4x
backtracking	0.32 MB	0.56 MB	1.8x
unsatisfiable	0.04 MB	0.12 MB	3.0x

Roughly triple, on closures of seven to eighteen packages. The memo is left unbounded, following the decoded blob cache's rationale ("a long-lived server process resolving arbitrary requests would want a bound; that is not this").

Saying the other half plainly, because that rationale has already been true of the CLI and quietly wrong for a server once before: this memo multiplies the per-package retention, and a bound over the index's caches is a prerequisite for embedding RSFIndex in a long-lived server process. "Bounded by the corpus" is a property of the key set, not of the memory, and the corpus is 932,861 packages. That is written beside the memo in rsfindex.go, along with the asymmetry a reviewer surfaced: versionPlanFor builds the same plan on the Metadata path, so a consumer that only calls Metadata pays the full retention increase and gets none of the speedup.

Equivalence

Re-run at this base. 4,007 resolutions against the production snapshot — the seven corpus entries plus 4,000 sampled package names, seed 1, 8-second per-case deadline — produce byte-identical transcripts: identical pins, identical decision ORDER, identical activated extras, identical failure report text.

• compared: 3,973 (2,365 produced pins, 1,608 produced a failure report)
• differing: 0
• excluded on the deadline: 34, and all 34 timed out on both sides — zero one-sided exclusions in either direction, so nothing was dropped from one build's column and not the other's.

Concurrency

index/shared_memo_test.go warms the memo and has eight goroutines share it, over four fixtures covering both of v0.6.0's independent safety arguments — the packed integer key, and the padParts fallback that a local label, a non-zero epoch or a long release segment forces. Which path each fixture takes is asserted by allocation count. Each fixture reports WARNING: DATA RACE under a v0.5.0 pin, 20 fresh processes each, 80 for 80.

It also drives version.ReleaseKey, which #46 made a second reader of a shared parsed version on the hot path (pep440set.verPos.init). That is safe — gpp clips the release slice it returns for exactly this hazard — but it was safe by argument only until this test drove it.

✅ The guard runs in CI. This was raised while #44 was still open: gpr CI ran plain go test ./..., under which these tests are a green no-op, so merging ahead of #44 would have shipped a guard that could not fire. #44 has now merged and this is rebased on top of it (61029b6), so the Race step covers this file. Measured, on a tree with the defensive copy deleted: the pre-existing concurrency test catches it 8 in 20 under -race and 0 in 20 without it — which is why the step, not the test, is the load-bearing part.

#44's new TestResolutionTranscriptMatchesGolden gate also passes unchanged on this branch (transcript[fast]: 125 cases … 45 discriminating), which is a second, CI-enforced equivalence check on top of the 4,007-case run below.

⚠️ Claims corrected after review — two unbriefed passes, both found real defects

First pass (three findings, all fixed):

1. "These are the only tests that object to a v0.5.0 pin" was FALSE. TestMemoIsSafeUnderConcurrentUse reports a race in 8 of 20 fresh processes. My original claim came from a single process — which is exactly the race-detector stack-deduplication trap I documented in the same file for other people. Reproduced independently and by me. Corrected in shared_memo_test.go, mock.go, the CHANGELOG and here, and reconciled with the "lottery" framing: 8-in-20 is not a second guard, it is a flake generator pointed at something real.
2. The v0.5.0 re-verification recipe no longer executes on this base. pep440set now calls version.ReleaseKey (gpp v0.7.0), so pinning back gives four compile errors, not race reports. The file now says where the verification was run (6c13230), and that repeating it also requires reverting pep440set — rather than leaving executable-looking instructions that cannot execute.
3. A stale profile citation. provider.go named releaseKey as "the next real win"; perf(pep440set): take the release group key from gpp, not from a render #46 deleted that symbol and shipped that win. Replaced with a re-profiled figure against the tree that actually exists (rankedVersions 27.5% of Resolve on warm wide-versions, candidate.Rank 17.4% inside it) and a warning to re-profile before acting on it.

Also corrected: the allocation discriminator's stated cause. A general-path pair whose release lengths already match still allocates 17 per comparison, so the allocations are key.compare boxing into interfaces, not padParts copying. The guard is sound; the explanation was not.

Second pass, after the rebase onto #44 (no correctness defect in the code path; everything below is prose or coverage, and all of it is fixed):

• The -benchmem figures in peak_heap_test.go's doc were an unlabelled survivor from the old base (24%/61% against the CHANGELOG's 55.4%/94.4%). Corrected, with the base named.
• The "80 for 80 under a v0.5.0 pin" claim over-read. That run predates the ReleaseKey assertions — which cannot be covered by it, since version.ReleaseKey does not exist before gpp v0.7.0. The scope is now stated exactly: the comparison half was demonstrated to detect; the ReleaseKey half rests on gpp's three-index slicing plus being driven under -race. The stated cost of repeating the run was also understated — this file's own ReleaseKey calls have to come out too, not just pep440set's.
• The "what else objects" list omitted index/shared_version_test.go — Make the correctness evidence reproducible in CI, and pin the shareability guarantee #44's own tests for the same hazard one level down. That is the same omission class this PR is correcting. Fixed in both places, along with a contradiction between mock.go ("pep440set passes") and shared_memo_test.go ("pep440set does not compile"): both are true, of different trees, and each now says which.
• "Caught by three tests" overstated their independence. Two catch it through a white-box pointer check, and in one of those it is a t.Fatalf precondition that aborts before the concurrent phase. Only TestVersionsMemoIsNotAliasedByTheCaller observes the defect through the exported API. Said so.
• The order/versions parallelism was enforced by a comment and by two appends sitting next to each other. Now enforced by TestVersionPlanHalvesStayParallel, element-wise across every fixture package. Mutation-tested: reversing plan.versions relative to plan.order (same length, wrong pairing — which a length check passes) is CAUGHT, and nilling it is CAUGHT.
• TestIndexRetainedHeapAfterResolve took excerpt from benchSnapshotT and ignored it, so running it without PYPIRSF_TEST_FILE silently measured the 139-package excerpt and produced numbers unlike the table. It now logs which corpus it measured, first line, with the package count.
• The overlap with Make the correctness evidence reproducible in CI, and pin the shareability guarantee #44's shared_version_test.go is real — same four fixture triples, same three shape assertions — and is now acknowledged in the file, along with what each covers that the other does not and the drift risk of keeping two copies in one package.

The returned slice is a copy

cmd/pyresolve's versions subcommand sorts the result of Versions in place, so handing back the memo's own slice would be a silent breaking change.

⚠️ Its cost is higher than this change was scoped with, and I am reporting that rather than the number I was given. Measured warm, medians of nine interleaved rounds against a variant that returns the memo's slice: −0.3% to +9.7%, worst on wide-versions (+9.7%) and backtracking (+8.1%). Against 6c13230 the same copy measured −0.1% to +4.0%, and the brief for this work said "under 2%, effectively free". The copy costs the same microseconds; warm resolution got faster, so those microseconds are now a larger fraction. That cost is inside every figure in the table above.

⚠️ It is also redundant on the resolution path: provider hands the result straight to candidate.Rank, which copies unconditionally, so a resolution copies twice. The copy is there for the exported contract.

⚠️ That does not make an internal no-copy accessor free, and an earlier revision of this PR said it did. A reviewer found that provider is not the only library-side caller — FilteredIndex.Versions and MultiIndex.Versions also call it, and FilteredIndex returns the inner index's slice by reference when its policy filters neither pre-releases nor files. Routing a no-copy accessor through that fast path would hand the memo's own slice to an arbitrary caller: exactly the silent breaking change the copy prevents. Nothing is broken today — RSFIndex.Versions returns a fresh copy that FilteredIndex then passes through — but the follow-up has a prerequisite, and the code now names it.

TestVersionsMemoIsNotAliasedByTheCaller was written for this day and could not fail until now; TestVersionsNeverReturnsTheMemosSlice is new and covers the first-call path, which that one cannot reach. ⚠️ All three tests that catch the deletion are in index/. Nothing outside notices — cmd/pyresolve's own tests included, and cmd/pyresolve is the caller the copy exists for.

Sweep

Issue #42's second half: the rationales asserting a parsed version.Version cannot be shared are corrected in index/rsfindex.go (the memo bullet, cloneMetadata, findEqualKey), index/mock.go, provider/provider.go and resolver/bench_test.go. Dated CHANGELOG sections are untouched — the diff on CHANGELOG.md is pure insertion.

Two findings from the sweep:

• cloneMetadata's Version substitution is not removable, which Take the parsed-version memo the v0.6.0 race fix unblocks, and sweep the rationales it invalidates #42 asked about. It had a second reason all along: buildMetadata never sets Version, so the memo holds a zero value and the caller's own version is the only thing on offer.
• index/mock.go credited resolver/concurrency_test.go with finding the original race. It found it in a variant where the mock stored parsed values; it does not guard the hazard on shipped code, and under a v0.5.0 pin it passes.

Follow-ups, deliberately not here

• Move Provider's ranked-list memo onto the index — the same v0.6.0 fix unblocks it. Ceiling is candidate.Rank, 17.4% of warm wide-versions' Resolve after this change, and only across resolutions. Kept out because a second effect would confound this one's measurement.
• An internal no-copy accessor for Versions, now that the copy measures up to 9.7% and the only library caller copies again immediately.
• findEqualKey could probe plan.versions instead of re-parsing plan.order. Only log2(n) probes, but it would also close the Metadata-only retention asymmetry above.

Measurement provenance

Every figure comes from a session run after two invalidations, and I am stating both because a number's provenance is part of the number:

1. An earlier dataset had three of five baseline rounds overwritten by another agent writing to the same scratch path. That whole comparison was discarded and re-run rather than patched — a median stitched from two sessions is not an interleaved measurement.
2. The base then moved from 6c13230 to 11da678. The full A/B was re-run again rather than carried forward, which is what surfaced the larger speedups and the higher copy cost.

The published session is nine interleaved rounds, base and candidate alternating within each round, with the 1-minute load average recorded next to every round. This host runs several agents; load ranged from 16 to 112 across the session, and an intermediate five-round attempt was discarded outright because per-round spreads reached 213% and a cold column cannot be read through that. Nine rounds and a median are what stand behind these figures, not a quiet machine — the per-round series is in the session log.

🤖 Generated with Claude Code