Skip to content

Performance audit: 10 findings in the copy engine, content stores, and registry client #1395

Description

@TerryHowe

Summary

I ran a performance audit over the hot paths in main — the copy engine, the local content stores, and the registry client. Ten findings below. The top five are measured on this checkout with throwaway benchmarks (written, run, then deleted — no working-tree changes); findings 6–10 are by inspection, reasoning stated but unmeasured, and labeled as such.

The library is structurally sound. The DAG traversal, the dedup tracker, and the metadata caching proxy are all the right shapes. What it loses is concentrated in four places: a buffer optimization that never takes effect, a concurrency default tuned for a different workload, an allocation pattern that triples memory on every manifest read, and an OCI index that gets rewritten on every push.

Environment: main @ 9f50ec9, go1.26.2 darwin/arm64, Apple M1, APFS. Numbers are single-machine and single-run — treat ratios as sound and absolute times as indicative.

Happy to split any of these into separate issues and send PRs. Findings 1–4 are all small, contained changes.


1. The pooled 1 MiB copy buffer is never used — every copy runs in 32 KiB chunks

High · measured
content/oci/storage.go:35,155 · content/file/file.go:42,472,514,561 · content/file/utils.go:107,365 · internal/ioutil/io.go:38

Both content stores keep a sync.Pool of 1 MiB buffers, with a comment explaining the choice:

the buffer size should be larger than or equal to 128 KiB for performance considerations. we choose 1 MiB here so there will be less disk I/O.

Every call site then hands that buffer to io.CopyBuffer with an *os.File as the destination. io.CopyBuffer discards the caller's buffer whenever dst implements io.ReaderFrom or src implements io.WriterTo — and *os.File implements both. Control lands in genericReadFrom, which allocates its own 32 KiB buffer per call.

All four call sites in the library hit this: OCI blob ingest, file-store save, tar creation, and tar extraction. The pool is allocated, populated, and returned to — pure overhead, zero effect.

Direct probe, 4 MiB copy with a 1 MiB buffer supplied:

dst = *os.File             (library today)     reads=129   maxChunk=  32 KiB
dst = struct{io.Writer}{f} (ReaderFrom hidden) reads=  5   maxChunk=1024 KiB

On APFS the wall-clock cost is small — a 16 MiB verified ingest measured 768 MB/s against 793 MB/s with the buffer actually applied, about 3%. The unambiguous win is garbage: 33,727 B/op → 945 B/op (−97%), because the 32 KiB buffer is freshly allocated on every push. Tar extraction is where this compounds: writeFile is called once per archive entry, so a layer with 10,000 files allocates 10,000 buffers and issues 32× the intended number of write syscalls. On network-backed or FUSE filesystems, where syscall count dominates rather than being absorbed by the page cache, the throughput gap should be considerably wider than 3%.

Fix: one line per call site — wrap the destination so the fast path can't be taken:

io.CopyBuffer(struct{ io.Writer }{dst}, src, buf)

the same trick net/http uses. The zero-copy paths ReadFrom is reaching for can't apply here anyway, since content has to pass through the digest verifier. The alternative is to delete the pools and accept the 32 KiB default — but keeping a pool that does nothing is the worst of both.


2. defaultConcurrency = 3 costs ~5× on latency-bound copies

High · measured — copy.go:39

The default is annotated "This value is consistent with dockerd and containerd." Those tune for bandwidth-bound pulls of a few large layers, where three streams saturate a link. oras-go's characteristic workload is different: many small blobs, artifact manifests, referrer graphs, signature payloads — where each node costs round trips rather than bytes. The limiter is also per-node, not per-byte, so a 2 KiB signature blob occupies the same slot as a 2 GB layer.

Copying a 40-layer artifact to a destination with 5 ms of simulated per-request latency (125 requests in every run):

Concurrency Elapsed
3 (default) 259 ms
8 120 ms
16 70 ms
32 47 ms

Same request count, 5.5× the wall clock. Real registries have 30–100 ms RTT, not 5, which widens the gap. Callers who care about copy speed are already setting this field by hand; the default quietly penalizes everyone who doesn't know to.

Fix: raise the default to 8–16. A weight-aware limiter — charging manifests and small blobs less than large layers — would be better still, and semaphore.Weighted is already the type in use, so Acquire could take a size-derived weight instead of the hardcoded 1 in syncutil.LimitedRegion. That's a larger change; bumping the constant is not.


3. -DONE- content.ReadAll allocates 3× the content size on every call

High · measured — content/reader.go:148

ReadAll sizes its buffer to desc.Size (capped at 32 MiB — correctly, since the declared size is attacker-controlled) and fills it with bytes.Buffer.ReadFrom. But ReadFrom insists on 512 bytes of headroom before each read; with capacity exactly desc.Size, the final iteration finds none, grows the buffer to 2n+512, and copies the whole thing across — only to read 0, io.EOF into it.

This is the hottest path in the library: every manifest parse, every config read, every Successors call, and every push into the in-memory CAS goes through it.

size current with headroom
4 KiB 9,261 ns · 12,929 B/op 9,135 ns · 5,505 B/op
1 MiB 1,120,862 ns · 3,147,959 B/op 708,962 ns · 1,058,750 B/op
8 MiB 4,686,412 ns · 25,167,854 B/op 3,897,379 ns · 8,398,670 B/op

1 MiB: 936 → 1,479 MB/s (+58% throughput, −66% allocated)
8 MiB: 1,790 → 2,152 MB/s (+20% throughput, −67% allocated)

Fix: make([]byte, 0, initialCap + bytes.MinRead). One term. The 32 MiB cap and the VerifyReader size/digest enforcement are untouched, so the security property the cap exists for is preserved — a forged desc.Size still can't drive the allocation.


4. Pushing a manifest to an OCI store rewrites all of index.json — cost is quadratic in store size

High · measured — content/oci/oci.go:145 → :250 → :427

Store.Push tags every manifest by digest; tag() calls saveIndex() when AutoSaveIndex is set — and it is, by default. saveIndex clones the entire tag map, walks it twice, allocates a fresh annotations map per tagged descriptor, marshals the complete index, and writes the whole file. Copying an image index with M manifests into a store holding T tags does M full rewrites at O(T) each, plus M whole-file writes.

Pushing n small manifests into a fresh store:

n elapsed allocated
10 2.9 ms 0.6 MB
100 33.9 ms 11.5 MB
500 195 ms 176.6 MB

50× the manifests, 282× the allocation. 176 MB of garbage to write 500 manifests that together total well under a megabyte. This is the shape users hit pulling a large multi-arch index or a repository's referrer set into a local OCI layout.

Fix: mark the index dirty on push and flush once — at an explicit SaveIndex(), on close, or debounced. That API already exists and is already documented as the manual alternative; the default just doesn't use it. Failing that, saveIndex can at minimum stop cloning the tag map and rebuilding every annotations map on each call.


5. Three HTTP round trips per blob, where the spec allows one

Medium · measured — copy.go:245 · registry/remote/repository.go:1253

Counting requests for a cold copy to an empty registry:

50 layers + config + manifest = 52 nodes → 155 requests (3.0 per node)

  HEAD  blob            51    copyGraph's dst.Exists() pre-check
  POST  blob-upload     51    begin upload session
  PUT   blob-upload     51    upload + commit
  HEAD  manifest         1
  PUT   manifest         1

Two separate things here. The HEAD is copyGraph's existence check, which is exactly right when copying into a warm registry — it's what makes re-pushes nearly free. On a cold push it's 51 wasted round trips, a third of all traffic. The POST+PUT pair is unconditional: blobStore.Push always opens an upload session and then commits to the returned Location, even though the distribution spec supports a single-request monolithic upload (POST /v2/<name>/blobs/uploads/?digest=<digest> carrying the body).

At 50 ms RTT, 155 requests is roughly 7.8 s of pure latency for a 52-node artifact before a byte of payload is accounted for.

Fix: use the single-request POST for blobs under a size threshold, falling back to POST+PUT on 4xx from registries that don't implement it — roughly a 33% cut in requests. The HEAD is worth keeping, though skipping it for small blobs (where the check costs about as much as the upload) is a defensible option to expose. Both belong behind a flag rather than as unconditional changes.


6. -DONE- Opening an OCI store fans out an unbounded number of goroutines

Medium · by inspection — internal/graph/memory.go:94,98 · content/oci/readonlyoci.go:182

oci.New loads index.json and calls graph.Memory.IndexAll for each entry. IndexAll recurses with syncutil.Go(ctx, nil, fn, successors...) — a nil limiter, which makes LimitRegion return nil and region.Start() a no-op. Every node in every manifest gets its own goroutine, each opening a file, reading it, and parsing JSON, with nothing bounding the fan-out. On a store with a few hundred manifests that's thousands of concurrent file opens competing for the same disk.

Meanwhile the top level is fully serial — loadIndex iterates index.Manifests one at a time. So concurrency is applied at exactly the level where it hurts and withheld where it would help.

Fix: thread a real semaphore.Weighted through IndexAll instead of nil, and parallelize the outer loadIndex loop under the same limiter.


7. ExtendedCopy serializes its entire discovery phase

Medium · by inspection — extendedcopy.go:150

findRoots walks predecessors with a single-threaded stack loop, one FindPredecessors call at a time. Against a remote source each is a Referrers API call — a network round trip — and none overlap. The Concurrency option doesn't apply until findRoots has already returned every root it's going to find.

For the workload ExtendedCopy exists to serve — an artifact with a wide referrer set: signatures, SBOMs, attestations — discovery latency is serialized and can easily dominate the copy it precedes.

Fix: replace the stack walk with a bounded concurrent BFS under the same limiter ExtendedCopyGraph already builds, sharing the visited set behind a mutex.


8. Store.delete clones and scans the full tag map once per node

Medium · by inspection — content/oci/oci.go:203

Every call does s.tagResolver.Map() — a full maps.Clone of the reference index — then walks all of it comparing descriptors, to find the references pointing at one target. Store.Delete drives that in a loop over the dangling-node queue, so deleting a manifest with N successors from a store with T tags is O(N×T) with N whole-map copies, and each untag that lands triggers a full saveIndex on top.

Fix: resolver.Memory already maintains a reverse digest→tags index and exposes it as TagSet. Use it here instead of scanning.


9 & 10. Two per-operation allocations worth cleaning up

Low · by inspection

Site What happens Fix
internal/status/tracker.go:41 TryCommit evaluates make(chan struct{}) before LoadOrStore, so a channel is allocated on every call including the hits that throw it away. copyGraph calls it at least twice per node — once in the worker, once in the parent's wait loop. Check Load first, allocate only on the miss.
registry/remote/auth/client.go:133 redirectSafeClient copies the whole http.Client struct and builds a fresh CheckRedirect closure on every send() — up to three per Do. The wrapper is stateless with respect to the request. Build the wrapped client once per auth.Client and cache it.

What's already right

Worth stating, since an audit that only lists problems misrepresents the codebase:

  • The DAG traversal and dedup tracker. copyGraph correctly releases its concurrency slot before waiting on successors (copy.go:274), which is what prevents the classic deadlock where parents hold all the slots. The status.Tracker handoff via closed channels is a clean, allocation-light way to make concurrent workers converge on one copy per node.
  • The metadata caching proxy. cas.Proxy tees non-leaf fetches into memory as they stream, so a manifest is read once from the network and served from RAM for the successor walk and the push — and NewProxyWithLimit bounds it so large layers never land in the cache. Right design, correctly implemented.
  • Token caching and request coalescing. concurrentCache plus syncutil.Once collapses concurrent token fetches for the same scope into a single request, and the scope-hint mechanism (AppendRepositoryScope) lets a copy acquire one pull+push token instead of re-challenging on every verb. The per-request slices.Clone and strings.Join in that path are trivial next to the round trip they avoid.
  • The self-heal retry in copyGraph. The MANIFEST_BLOB_UNKNOWN recovery path (copy.go:305) deliberately skips re-pushing non-manifest successors, on the stated grounds that blob existence is reported reliably. That's the correct cost trade — the naive version would re-upload every layer on a recoverable error.

The structural gap: no benchmarks

There are zero benchmarks in the repository — not one func Benchmark across 20,676 lines of non-test library code.

Four of the five measured findings above are the kind of regression a benchmark in CI would have caught the day it landed. Finding 1 especially: a deliberate optimization was written, commented, and has quite possibly never once taken effect.

A small suite would cover most of the surface: copyGraph against an in-memory target at several graph shapes, oci.Store push and open at several store sizes, content.ReadAll across blob sizes, and CleanScopes. Run with -benchmem and compared via benchstat, allocation counts alone would have flagged findings 1, 3, and 4.

I'd suggest this is worth doing before the perf fixes, so each one lands with a number attached.


Measurements taken with temporary benchmark files in the repo root, run with go test -bench, then deleted. HTTP request counts come from a stub registry over httptest, so they reflect what the client emits, not any real registry's behavior.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions