Repository navigation
Conversation
The encoder() singleton used the default zstd window, so it kept 64 MiB of match history for the life of the process. After the bounded-memory pack writer, that history was most of the heap a push leaves live. Every input of the singleton is an object of at most inMemoryCap, a probeWindow head, or a block of a .cue or packed-refs. A window of inMemoryCap finds almost all of the same matches. Objects larger than inMemoryCap use streamEncPool, which this does not change. Measured with a real pack through ReadPack over a fake S3: - Live heap after a push: 72 MiB to 16 MiB, for Xe/x and golang/go. - Xe/x peak RSS: 204 MiB to 123 MiB. - golang/go peak RSS: about 476 MiB to 395 MiB. - .bin bytes: identical for both repositories. - .cue bytes: +0.09% for Xe/x, +0.22% for golang/go. Wall time does not change by a measurable amount. The plan records the full table. Assisted-by: Claude Opus 5.5 via Claude Code Signed-off-by: Xe Iaso <xe@tigrisdata.com>
There was a problem hiding this comment.
🟢 Approval recommended
The focused change is consistent with encoder input bounds and preserves decoder compatibility.
0 open findings
What changed in this PR
Caps the shared zstd encoder window at 1 MiB to reduce retained memory during pushes.
Changes:
- Configures the singleton encoder with a 1 MiB window.
- Documents memory and compression measurements and updates the optimization plan.
| File | Description |
|---|---|
internal/storage/tigris/compress.go |
Caps the shared encoder window. |
docs/plans/push-memory.md |
Records results and marks the work complete. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
encoder()zstd singleton toinMemoryCap(1 MiB). It used the default window, which kept 64 MiB of match history for the life of the process.docs/plans/push-memory.mdand remove the item from the phase 3 list.Details
After #25, the zstd history was most of the heap that a push leaves live. Every input of the singleton is at most
inMemoryCap: an object, aprobeWindowhead, or a block of a.cueor packed-refs. A 1 MiB window finds almost all of the same matches. Objects larger thaninMemoryCapusestreamEncPool, and this PR does not change them.Measured by feeding a real pack through
ReadPackover a fake S3 that throws away.binbodies. Xe/x is the median of 3 runs. golang/go is 2 runs..binbytes (both repos).cuebytesWall time does not change by a measurable amount. Existing data is unaffected: a smaller encoder window only changes new writes, and the decoder accepts both.
Test plan
go build ./...go test ./...passes1b1ad2fas the baseline