Skip to content

Experiment with StreamEncoder::encode_to - #10985

Draft
Phoenix500526 wants to merge 1 commit into
apache:mainfrom
Phoenix500526:codex/ipc-stream-encoder-encode-to
Draft

Experiment with StreamEncoder::encode_to#10985
Phoenix500526 wants to merge 1 commit into
apache:mainfrom
Phoenix500526:codex/ipc-stream-encoder-encode-to

Conversation

@Phoenix500526

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

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:

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:

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.

A direct Write path lets us measure the cost of returning buffers and
forwarding them separately before settling the public sink design.

Refs apache#10445
@github-actions github-actions Bot added arrow Changes to the arrow crate arrow-ipc labels Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arrow Changes to the arrow crate arrow-ipc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant