Cache.metaCache is dead: every cache hit does open+stat+ReadFile+JSON parse instead of the in-memory lookup README promises #70

New Issue

2026-08-09T03:45:28+02:00

clawbot commented

2026-08-09 03:45:28 +02:00

Verified against main at 61f42e6.

Cache.metaCache map[VariantKey]variantMeta is declared at internal/imgcache/cache.go:47 and initialised at :73. It is never read and never written — grep returns exactly those two lines. The variantMeta type (cache.go:32-36) is dead with it.

Consequently every cache hit goes to the filesystem: os.Open + Stat + os.ReadFile of the .meta sidecar + json.Unmarshal (internal/imgcache/storage.go:442-474).

README.md:49 claims "In-process caching of request-to-output mappings targets 1-5k r/s". That mechanism is this field, and it does not exist. Four syscalls and a JSON parse per hit is the difference between hitting that target and not.

Definition of done

Either implement or delete — do not leave a third initialised-but-unused field.

If implementing (recommended):

The in-memory map is consulted on hit before touching the filesystem, and populated on store and on first read-through.
It is bounded — an unbounded map keyed by variant cache key is a memory leak with the same shape as the disk-fill issue #51 just fixed. Use an LRU with a configurable entry cap (prefer a well-maintained library; check ~/.claude/GO_PACKAGE_DEFAULTS.md and record the decision).
Concurrency-safe: guarded by a mutex or sync.Map. The suite runs with -race as of #55 and must stay clean.
Invalidation is correct on eviction and purge — a stale in-memory entry pointing at a file #51's evictor deleted must not produce a false hit. This is the part most likely to go wrong; cover it with a test that evicts and then reads.
Failing tests first: a second hit for the same key performs no filesystem read (inject or count via the storage layer); an evicted key is not served from the in-memory map.

If deleting: remove the field, the variantMeta type, and correct README.md:49 so it no longer claims a mechanism that does not exist.

Coordination

internal/imgcache/cache.go is heavily changed by PR #55. Do this after #55 merges, and treat point 4 as blocking — the eviction path and this cache must agree.

Verified against `main` at `61f42e6`. `Cache.metaCache map[VariantKey]variantMeta` is declared at `internal/imgcache/cache.go:47` and initialised at `:73`. It is **never read and never written** — grep returns exactly those two lines. The `variantMeta` type (`cache.go:32-36`) is dead with it. Consequently every cache hit goes to the filesystem: `os.Open` + `Stat` + `os.ReadFile` of the `.meta` sidecar + `json.Unmarshal` (`internal/imgcache/storage.go:442-474`). `README.md:49` claims "In-process caching of request-to-output mappings targets 1-5k r/s". That mechanism is this field, and it does not exist. Four syscalls and a JSON parse per hit is the difference between hitting that target and not. ## Definition of done Either implement or delete — do not leave a third initialised-but-unused field. **If implementing (recommended):** 1. The in-memory map is consulted on hit before touching the filesystem, and populated on store and on first read-through. 2. It is **bounded** — an unbounded map keyed by variant cache key is a memory leak with the same shape as the disk-fill issue #51 just fixed. Use an LRU with a configurable entry cap (prefer a well-maintained library; check `~/.claude/GO_PACKAGE_DEFAULTS.md` and record the decision). 3. Concurrency-safe: guarded by a mutex or `sync.Map`. The suite runs with `-race` as of #55 and must stay clean. 4. Invalidation is correct on eviction and purge — a stale in-memory entry pointing at a file #51's evictor deleted must not produce a false hit. This is the part most likely to go wrong; cover it with a test that evicts and then reads. 5. Failing tests first: a second hit for the same key performs no filesystem read (inject or count via the storage layer); an evicted key is not served from the in-memory map. **If deleting:** remove the field, the `variantMeta` type, and correct `README.md:49` so it no longer claims a mechanism that does not exist. ## Coordination `internal/imgcache/cache.go` is heavily changed by PR #55. Do this **after** #55 merges, and treat point 4 as blocking — the eviction path and this cache must agree.

clawbot added this to the 1.0.0 milestone 2026-08-09 03:45:28 +02:00

clawbot referenced this issue

2026-08-09 16:10:39 +02:00

Tracking: 1.0.0 sequencing and critical path #103

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/pixa#70