-
Notifications
You must be signed in to change notification settings - Fork 5
perf(dotc1z): pool zstd encoders and decoders to reduce allocations #622
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
arreyder
wants to merge
8
commits into
main
Choose a base branch
from
crr/zstd-encoder-pool
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
7ab440b
perf(dotc1z): pool zstd encoders and decoders to reduce allocations
arreyder 164e413
fix: add period to comment for godot linter
arreyder d7c1e91
fix(dotc1z): ensure pool grows when initially empty
arreyder 08caa6f
fix(dotc1z): handle Close() return values for linter
arreyder 5a1503a
fix(dotc1z): check Close() errors in TestPooledRoundTrip
arreyder b93a6a8
fix(dotc1z): handle errors in createCompressedData test helper
arreyder cf1b39d
fix(pool_test): add proper error handling in all benchmarks
arreyder cef3191
fix(pool_test): use assert.* in concurrent test goroutines
arreyder File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| package dotc1z | ||
|
|
||
| import ( | ||
| "runtime" | ||
| "sync" | ||
|
|
||
| "github.com/klauspost/compress/zstd" | ||
| ) | ||
|
|
||
| // encoderPool manages reusable zstd.Encoder instances to reduce allocation overhead. | ||
| // All pooled encoders are configured with GOMAXPROCS concurrency. | ||
| var encoderPool sync.Pool | ||
|
|
||
| // pooledEncoderConcurrency is the concurrency level used for pooled encoders. | ||
| // Set at package init to GOMAXPROCS to match the default behavior. | ||
| var pooledEncoderConcurrency = runtime.GOMAXPROCS(0) | ||
|
|
||
| // getEncoder retrieves a zstd encoder from the pool or creates a new one. | ||
| // The returned encoder is NOT bound to any writer - call Reset(w) before use. | ||
| // Returns the encoder and a boolean indicating if it came from the pool. | ||
| func getEncoder() (*zstd.Encoder, bool) { | ||
| if enc, ok := encoderPool.Get().(*zstd.Encoder); ok && enc != nil { | ||
| return enc, true | ||
| } | ||
|
|
||
| // Create new encoder with default concurrency. | ||
| // This should not fail with valid options, but handle it gracefully. | ||
| enc, err := zstd.NewWriter(nil, | ||
| zstd.WithEncoderConcurrency(pooledEncoderConcurrency), | ||
| ) | ||
| if err != nil { | ||
| // Fallback: return nil and let caller create encoder with their options | ||
| return nil, false | ||
| } | ||
| return enc, false | ||
| } | ||
|
|
||
| // putEncoder returns a zstd encoder to the pool for reuse. | ||
| // The encoder is reset to release any reference to the previous writer. | ||
| // Encoders should be in a clean state (Close() called) before returning. | ||
| func putEncoder(enc *zstd.Encoder) { | ||
| if enc == nil { | ||
| return | ||
| } | ||
| // Reset to nil writer to release reference to previous output. | ||
| // This is safe even if the encoder was already closed. | ||
| enc.Reset(nil) | ||
| encoderPool.Put(enc) | ||
| } | ||
|
|
||
| // decoderPool manages reusable zstd.Decoder instances to reduce allocation overhead. | ||
| // All pooled decoders are configured with concurrency=1 (single-threaded) and low memory mode. | ||
| var decoderPool sync.Pool | ||
|
|
||
| // getDecoder retrieves a zstd decoder from the pool or creates a new one. | ||
| // The returned decoder is NOT bound to any reader - call Reset(r) before use. | ||
| // Returns the decoder and a boolean indicating if it came from the pool. | ||
| func getDecoder() (*zstd.Decoder, bool) { | ||
| if dec, ok := decoderPool.Get().(*zstd.Decoder); ok && dec != nil { | ||
| return dec, true | ||
| } | ||
|
|
||
| // Create new decoder with default options matching decoder.go defaults. | ||
| dec, err := zstd.NewReader(nil, | ||
| zstd.WithDecoderConcurrency(1), | ||
| zstd.WithDecoderLowmem(true), | ||
| zstd.WithDecoderMaxMemory(defaultDecoderMaxMemory), | ||
| ) | ||
| if err != nil { | ||
| // Fallback: return nil and let caller create decoder with their options | ||
| return nil, false | ||
| } | ||
| return dec, false | ||
| } | ||
|
|
||
| // putDecoder returns a zstd decoder to the pool for reuse. | ||
| // The decoder is reset to release any reference to the previous reader. | ||
| func putDecoder(dec *zstd.Decoder) { | ||
| if dec == nil { | ||
| return | ||
| } | ||
| // Reset to nil reader to release reference to previous input. | ||
| // If Reset fails (bad state), don't return to pool. | ||
| if err := dec.Reset(nil); err != nil { | ||
| return | ||
| } | ||
| decoderPool.Put(dec) | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be across all connectors in C1 ... which I guess is the point.