Add shared zstd encoder pools. #267
Conversation
On some sample testing this compression performs ~4x faster than "wasting" the encoders.
go/pkg/reader/reader.go
Outdated
| var encoderInit sync.Once | ||
| var encoders sync.Pool | ||
|
|
||
| func newEncoder() interface{} { |
There was a problem hiding this comment.
I think this is leaky: zstd.NewWriter spawns goroutines that refer to some internal data, then when the zstd.Encoder is purged from the sync.Pool that internal data + those goroutines leak.
Here is an example workaround: https://github.com/mostynb/zstdpool-syncpool
And some background: klauspost/compress#264
There was a problem hiding this comment.
I missed not only the goroutines would leak but also the encoder itself since GC wouldn't ever catch it.
I imported your lib in - thanks!
There was a problem hiding this comment.
No problem- feedback welcome. It hasn't been thoroughly tested yet, but I figure it makes sense to share the effort in figuring out how to use this zstd api safely/efficiently (hence the tiny lib).
There was a problem hiding this comment.
Will certainly send PRs if I find any issues :)
go/pkg/reader/reader.go
Outdated
| var encoders sync.Pool | ||
|
|
||
| func newEncoder() interface{} { | ||
| e, _ := zstd.NewWriter(nil) |
There was a problem hiding this comment.
You might want to use zstd.WithEncoderConcurrency(1), to avoid creating more goroutines than are necessary.
On some sample testing this compression performs ~4x faster
than "wasting" the encoders.