vdpoora opened a new pull request, #1429:
URL: https://github.com/apache/arrow-go/pull/1429
### Rationale for this change
`getCompressor` builds a new `zstd.Encoder` for every writer. A new encoder
allocates its window and match tables, about 2.3 MB at the default level,
before it compresses anything. A service that writes each table or batch with
its own writer pays that on every call. In our case, one writer per series in
each Grafana query, this was about 63% of all allocations on that path, and the
GC load that followed was a third of the CPU.
#1331 pooled the zstd decoders for the same reason, and `parquet/compress`
already pools its zstd encoders (#717). This does the same for the IPC writer.
### What changes are included in this PR?
- zstd encoders come from a `sync.Pool`, one per encoder level: the library
default, plus the four `zstd.EncoderLevel` presets that `WithZstdLevel` maps
to. An encoder is only reused for the level it was built with, and is built
with the same options as before.
- `Writer.Close`, `FileWriter.Close` and `GetRecordBatchPayload` return the
encoders. A writer keeps its encoder for its whole lifetime, as before, so a
long-lived stream sees no change. An earlier version returned the encoder after
each record batch; a long-lived writer then sometimes rebuilt it after the pool
was cleared by a GC, so I dropped that.
- Before going back to the pool, the encoder is `Reset(nil)`. That waits for
any in-flight block goroutines (also after a failed `Write`) and drops the
reference to the last output buffer.
- lz4 is unchanged.
### Are these changes tested?
New `TestZstdEncoderPool`:
- `Close` returns every encoder, for `Writer` and `FileWriter` with
`WithCompressConcurrency(4)`. This fails without the change.
- The output of a reused encoder is byte-identical to its first use, after a
larger stream went through it in between.
- 16 concurrent writers with mixed sizes and compression concurrency
round-trip correctly under `-race`.
`go test -race ./arrow/ipc/... ./arrow/flight/...` passes, and
`golangci-lint run ./arrow/ipc/` is clean. Run on macOS only.
New `BenchmarkWriterZstd`. Old and new test binaries were run alternately, 8
runs each, on an Apple M2 Pro:
```
sec/op B/op
allocs/op
writer_per_record/256rows 246.4µ → 4.5µ (-98%) 2279Ki → 7.1Ki
(-99.7%) 72 → 44
writer_per_record/4096rows 280.7µ → 21.8µ (-92%) 2296Ki → 24.7Ki
(-98.9%) 72 → 44
one_writer/256rows 2.98µ → 2.98µ (~) 2.755Ki → 2.755Ki (~)
23 → 23
one_writer/4096rows 19.4µ → 19.6µ (~) 19.54Ki → 19.54Ki (~)
23 → 23
```
### Are there any user-facing changes?
No API change. Writers that use zstd allocate less. Idle encoders can stay
in the pool until GC clears it, at most one per level and per P.
--
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]
