Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,28 @@ keeps the generic solver free of Python knowledge, and it is required from day
one: without it, any user of `[extras]` syntax silently gets an incomplete
closure.

**PPM-backed index implementations do NOT live here.** `RSFIndex`,
`OfflineIndex`, and `DBIndex` need PPM's deps-blob decoder, store types, or
database — all in a private repo that imports this module, so putting them here
would invert the dependency. They implement `index.MetadataIndex` from the PPM
side. `DBIndex` is the clearest case: a public module cannot reach PPM's
`pypi_projects` table. What belongs here is anything generic: the interface,
`MockIndex`, `CachedJSONIndex`, and eventually `FilteredIndex`/`MultiIndex`.

**A cached value handed to more than one caller must be copied on every path.**
`boundedCache.get` coalesces concurrent misses through singleflight, which hands
the *same* value to every waiter — so copying only on a cache hit is not enough,
and the leftover sharing shows up as a data race under load rather than as a
test failure. `CachedJSONIndex.Files` copies on the way out for this reason, and
there is a `-race` test that fails if the copy is removed. PPM hit this exact
bug in its own snapshot cache (#19291).

**Cache keys must name immutable content.** A key that can describe two
payloads over time serves a stale one until eviction, and no TTL fixes that — it
only shortens the window. `(package, snapshot)` qualifies, with one documented
exception: `yanked` is mutable within a published snapshot (RFD §5.1), tracked
as #18650.

## Build & test

```bash
Expand Down
5 changes: 4 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ module github.com/posit-dev/go-pyresolver

go 1.25.0

require github.com/posit-dev/go-python-packaging v0.2.0
require (
github.com/posit-dev/go-python-packaging v0.2.0
golang.org/x/sync v0.22.0
)

require github.com/rstudio/go-version v0.0.2 // indirect
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,7 @@ github.com/rstudio/go-version v0.0.2 h1:ihU0xaF+Yuya0p2J6C8dfyB4gu/YehM1d6cTpabf
github.com/rstudio/go-version v0.0.2/go.mod h1:Xfuma+m4R9L0P+Hof8iDDiL4/TJ8VjpdDXWq8qGXHL4=
github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U=
github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U=
golang.org/x/sync v0.22.0 h1:SZjpbeLmrCk4xhRSZFNZW5gFUeCeFgjekvI/+gfScek=
golang.org/x/sync v0.22.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
174 changes: 174 additions & 0 deletions index/cache.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
// SPDX-License-Identifier: Apache-2.0 OR MIT

package index

import (
"container/list"
"sync"

"golang.org/x/sync/singleflight"
)

// boundedCache is an LRU cache with an explicit byte budget and singleflight
// coalescing of concurrent misses.
//
// # Why byte accounting is caller-supplied rather than delegated to ristretto
//
// A general-purpose cache cannot size an arbitrary Go value. PPM learned this
// the hard way: its shared ristretto cache cannot size a non-[]byte value, so
// it admits Go objects at cost 0 and they escape the byte budget entirely
// (rstudio/package-manager#19374). PPM's own cachehelpers.BoundedCache exists
// for exactly that reason and takes a caller-supplied sizer. This does the
// same, which also spares a public library a cache dependency.
//
// # Safety contract
//
// An entry is only sound to cache if its key names IMMUTABLE content. A key
// that can describe two different payloads over time will serve a stale one
// until eviction, and no TTL makes that correct -- it only makes the window
// shorter. See CachedJSONIndex for how its key satisfies this, and for the one
// field that does not.
//
// # Copy contract
//
// This cache shares values by reference with every reader, and Get hands the
// SAME value to every goroutine coalesced into one singleflight flight.
// Callers that hand values onward to code which may mutate them must copy on
// BOTH paths -- see CachedJSONIndex.Files.
type boundedCache[V any] struct {
maxEntries int
maxBytes int64
sizeOf func(V) int64

sf singleflight.Group

mu sync.Mutex
entries map[string]*list.Element
lru *list.List // front = most recently used
totalBytes int64
}

type cacheEntry[V any] struct {
key string
value V
bytes int64
}

func newBoundedCache[V any](maxEntries int, maxBytes int64, sizeOf func(V) int64) *boundedCache[V] {
return &boundedCache[V]{
maxEntries: maxEntries,
maxBytes: maxBytes,
sizeOf: sizeOf,
entries: make(map[string]*list.Element),
lru: list.New(),
}
}

// lookup returns the cached value for key, promoting it to most-recently-used.
func (c *boundedCache[V]) lookup(key string) (V, bool) {
c.mu.Lock()
defer c.mu.Unlock()

el, ok := c.entries[key]
if !ok {
var zero V
return zero, false
}
c.lru.MoveToFront(el)

return el.Value.(*cacheEntry[V]).value, true
}

// put stores value under key, evicting least-recently-used entries until both
// budgets are satisfied.
func (c *boundedCache[V]) put(key string, value V) {
size := c.sizeOf(value)

c.mu.Lock()
defer c.mu.Unlock()

if el, ok := c.entries[key]; ok {
existing := el.Value.(*cacheEntry[V])
c.totalBytes -= existing.bytes
existing.value = value
existing.bytes = size
c.totalBytes += size
c.lru.MoveToFront(el)
c.evictLocked()
return
}

// An entry larger than the whole budget is not cached at all. Storing it
// would evict everything else and then still not fit.
if c.maxBytes > 0 && size > c.maxBytes {
return
}

c.entries[key] = c.lru.PushFront(&cacheEntry[V]{key: key, value: value, bytes: size})
c.totalBytes += size
c.evictLocked()
}

// evictLocked drops least-recently-used entries until both budgets hold.
// Callers must hold c.mu.
func (c *boundedCache[V]) evictLocked() {
for c.lru.Len() > 0 {
overEntries := c.maxEntries > 0 && c.lru.Len() > c.maxEntries
overBytes := c.maxBytes > 0 && c.totalBytes > c.maxBytes
if !overEntries && !overBytes {
return
}

oldest := c.lru.Back()
if oldest == nil {
return
}
entry := oldest.Value.(*cacheEntry[V])
c.lru.Remove(oldest)
delete(c.entries, entry.key)
c.totalBytes -= entry.bytes
}
}

// get returns the cached value for key, building it with build on a miss.
// Concurrent misses for one key are coalesced into a single build.
//
// The returned value is NOT a copy -- see the copy contract on boundedCache.
func (c *boundedCache[V]) get(key string, build func() (V, error)) (V, error) {
if v, ok := c.lookup(key); ok {
return v, nil
}

res, err, _ := c.sf.Do(key, func() (any, error) {
// Re-check under the flight: a concurrent flight for this key may have
// completed and populated the cache between our miss and here.
if v, ok := c.lookup(key); ok {
return v, nil
}

built, err := build()
if err != nil {
return nil, err
}
c.put(key, built)
return built, nil
})
if err != nil {
var zero V
return zero, err
}

v, ok := res.(V)
if !ok {
var zero V
return zero, nil
}
return v, nil
}

// stats reports current occupancy, for tests and for a future metrics surface.
func (c *boundedCache[V]) stats() (entries int, bytes int64) {
c.mu.Lock()
defer c.mu.Unlock()
return c.lru.Len(), c.totalBytes
}
Loading