zeroshade commented on code in PR #1254:
URL: https://github.com/apache/arrow-go/pull/1254#discussion_r3897322549
##########
parquet/compress/brotli.go:
##########
@@ -41,15 +44,44 @@ func (b brotliCodec) EncodeLevel(dst, src []byte, level
int) []byte {
dst = make([]byte, 0, maxlen)
}
buf := bytes.NewBuffer(dst[:0])
- w := brotli.NewWriterLevel(buf, level)
+ pool := brotliWriterPool(level)
+ var w *brotli.Writer
+ if pool != nil {
+ if cached := pool.Get(); cached != nil {
+ w = cached.(*brotli.Writer)
+ w.Reset(buf)
+ }
+ }
+ if w == nil {
+ w = brotli.NewWriterLevel(buf, level)
+ }
_, err := w.Write(src)
if err != nil {
+ releaseBrotliWriter(pool, w)
panic(err)
}
if err := w.Close(); err != nil {
+ releaseBrotliWriter(pool, w)
panic(err)
}
- return buf.Bytes()
+ compressed := buf.Bytes()
+ releaseBrotliWriter(pool, w)
+ return compressed
+}
+
+func brotliWriterPool(level int) *sync.Pool {
+ if level < brotli.BestSpeed || level > brotli.BestCompression {
+ return nil
+ }
+ return &brotliWriterPools[level]
+}
+
+func releaseBrotliWriter(pool *sync.Pool, w *brotli.Writer) {
+ if pool == nil {
+ return
+ }
+ w.Reset(nil)
+ pool.Put(w)
Review Comment:
Returning every Brotli writer to a process-global pool retains its large
encoder workspace. In a fresh-process probe, one 256 KiB level-11 encode left
roughly 44 MiB live after one GC (level 9 was about 43 MiB and the default
level about 11 MiB), while the non-pooled writer retained none. Sustained or
concurrent compression can retain multiple writers per level and processor.
Please introduce a bounded-memory policy—for example, avoid pooling
high-workspace levels or use a bounded cache—and add memory-retention coverage.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
