Skip to content

Land #907 - #910

Closed
mostynb wants to merge 1 commit into
buchgr:masterfrom
mostynb:reuse-compressed-chunk-buffer
Closed

Land #907#910
mostynb wants to merge 1 commit into
buchgr:masterfrom
mostynb:reuse-compressed-chunk-buffer

Conversation

@mostynb

@mostynb mostynb commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

This is a squashed and rebased version of #907, so I can trigger CI on it in that state before landing.

Under a burst of concurrent uploads, bazel-remote could be OOM-killed by a
transient Go heap overshoot even though idle/steady-state memory is small.
Heap profiling (alloc_space) pointed at zstd.(*Encoder).encodeAll: the write
path compresses each blob in 1 MiB chunks and called EncodeAll(in, nil) per
chunk, so a fresh output slice was allocated for every chunk of every upload.

The chunk size is exactly 1<<20, and klauspost's EncodeAll only pre-allocates
an output buffer when len(src) < 1<<20 (strictly less-than). A full chunk
therefore starts from a nil dst and grows it by repeated append-doubling, so
each 1 MiB chunk churned several MiB of transient garbage. With no memory
backpressure on concurrent Puts, the allocation rate outran the GC and the
process was killed.

Thread a reusable dst through zstdimpl.EncodeAll (both the pure-Go and cgo
backends already accept one) and have casblob.WriteAndClose allocate a single
output buffer, sized to compressBound(chunkSize), reused across all chunks of
the blob. Sizing to the ZSTD_compressBound worst case ensures EncodeAll never
has to grow (and reallocate) the buffer, even for incompressible chunks whose
output is slightly larger than the input.

A microbenchmark of the write path (16 MiB blob, incompressible data) shows
per-upload allocations drop from ~79 MB/op to ~1.1 MB/op.
@mostynb

mostynb commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator Author

This has landed.

@mostynb mostynb closed this Jul 22, 2026
@mostynb
mostynb deleted the reuse-compressed-chunk-buffer branch July 22, 2026 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants