Phoenix500526 opened a new pull request, #10985:
URL: https://github.com/apache/arrow-rs/pull/10985
# Which issue does this PR close?
- Refs #10445.
# Rationale for this change
This draft explores whether allowing `StreamEncoder` to write to a
borrowed destination can reduce the overhead of returning a `Vec<Buffer>`
and then forwarding each buffer separately.
The motivation is primarily performance and memory behavior, but this is
an experiment rather than a claim that the API is ready or that it provides
a broad performance improvement. In particular, making the encoder accept
`Write` also makes it overlap more with `StreamWriter` and weakens the
sans-I/O boundary.
# What changes are included in this PR?
- Add `StreamEncoder::encode_to<W: Write + ?Sized>`.
- Add the corresponding `finish_to<W: Write + ?Sized>`.
- Keep ownership, flushing, and closing of the destination with the caller.
- Add byte-for-byte equivalence tests for regular, empty, dictionary, and
ZSTD-compressed streams.
- Add paired `encode + write` and `encode_to` Criterion benchmarks, with and
without ZSTD compression.
# Are these changes tested?
Yes. The following checks pass locally:
```shell
cargo test -p arrow-ipc --lib --features zstd
cargo test -p arrow-ipc --doc --features zstd
cargo clippy -p arrow-ipc --lib --tests --benches --features zstd -- -D
warnings
cargo fmt --all -- --check
```
A local Criterion run produced these 95% confidence intervals:
| Case | `encode + write` | `encode_to` |
|---|---:|---:|
| Uncompressed | 113.18–115.60 µs | 102.70–105.08 µs |
| ZSTD | 6.3468–6.4073 ms | 6.3396–6.4149 ms |
The uncompressed intervals did not overlap in this run. The ZSTD intervals
overlap and do not demonstrate an improvement. These are results from one
local machine, so I do not want to claim a stable percentage improvement.
Benchmark command:
```shell
cargo bench -p arrow-ipc --bench ipc_writer --features zstd -- \
'StreamEncoder/(encode_and_write_10|encode_to_10)'
```
Important limitations of this experiment:
- It benchmarks synchronous `Write` into `Vec<u8>`; it does not model an
async writer or backpressure.
- It measures elapsed time, not allocations, copied bytes, or peak memory.
- Encoding still collects a per-batch `Vec<EncodedBuffer>` internally, so
this does not implement the owned-buffer sink proposed in the issue.
- If writing fails partway through, the destination and encoder must be
discarded; this is documented but not represented by a poisoned state.
# Are there any user-facing changes?
This adds public methods without changing existing behavior. The API shape,
boundary, and naming are deliberately open for discussion while this is a
draft.
@alamb, CC'ing you because this follows your suggestion in #10277. I would
appreciate your feedback on whether this synchronous `Write` prototype is a
useful boundary, whether an owned-buffer sink would better preserve the
sans-I/O design, and whether the benchmark should include allocation or
memory measurements before proceeding.
--
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]
