You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "pchintar (via GitHub)" <gi...@apache.org> on 2026/04/26 21:42:31 UTC
[PR] feat(ipc): Avoid repetitive heap allocations and buffer copies in IPC writer [arrow-rs]
pchintar opened a new pull request, #9836:
URL: https://github.com/apache/arrow-rs/pull/9836
# Which issue does this PR close?
- Closes #9835 .
# Rationale for this change
The current IPC writer path performs repeated heap allocations and buffer copies for every record batch, even when writing batches with identical structure. In particular, the use of `EncodedData` forces:
* allocation of new `Vec<u8>` buffers per batch
* copying of flatbuffer data via `to_vec()`
* destruction of all intermediate buffers after each write
This leads to unnecessary latency overhead in common scenarios such as streaming or iterative batch writes. The issue is purely performance-related and does not affect correctness.
# What changes are included in this PR?
This PR introduces a private, writer-specific fast path for non-dictionary batches in `StreamWriter` and `FileWriter`:
* Reuses a writer-owned scratch structure across batches:
* `FlatBufferBuilder`
* `arrow_data` buffer
* metadata vectors (`nodes`, `buffers`, `variadic_buffer_counts`)
* Avoids copying flatbuffer data by writing directly from `finished_data()`
* Bypasses the `EncodedData` path for non-dictionary batches while preserving it for dictionary cases
* Adds a helper to detect dictionary usage and safely fall back to the existing implementation when needed
# Are these changes tested?
Yes.
* Existing IPC writer tests (`cargo test -p arrow-ipc --lib writer`) pass without modification, confirming correctness and compatibility.
* Benchmark results (`ipc_writer.rs`) show significant improvements(30-45%) for non-compressed workloads
# Are there any user-facing changes?
No.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies for non-Dictionary types in IPC writer [arrow-rs]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#issuecomment-4398935121
Hi @alamb I just wanted to clarify: so this PR is indeed ready to be reviewed for non-dictionary types (The `ipc_writer` benchmark uses only non-Dictionary type). I've updated the PR title by explicitly mentioning "non-Dictionary" to make that clear & so the Dictionary work will be a separate PR after this. So, could you kindly review this one when you get a chance? Thanks.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#issuecomment-4323772156
run benchmark ipc_writer
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#issuecomment-4324417805
Also, it is clear that the compressed path is much slower than non-compressed overall, so I took a look into the zstd compressed path and I found out that in `arrow-ipc/src/compression.rs`, the current `compress_zstd` function was doing:
1. `compress()` → allocates a new `Vec`
2. `extend_from_slice()` → copies into output
That's one extra allocation + one extra copy per buffer. Zstd actually provides `compress_to_buffer()` which writes directly into an existing buffer. So, we can change current implementation from:
# Current `compress_zstd`(alloc -> compress -> copy)
```rust
#[cfg(feature = "zstd")]
fn compress_zstd(
input: &[u8],
output: &mut Vec<u8>,
context: &mut CompressionContext,
) -> Result<(), ArrowError> {
let result = context.zstd_compressor().compress(input)?;
output.extend_from_slice(&result);
Ok(())
}
```
---
# AFTER/New approach (compress -> direct write)
```rust
#[cfg(feature = "zstd")]
fn compress_zstd(
input: &[u8],
output: &mut Vec<u8>,
context: &mut CompressionContext,
) -> Result<(), ArrowError> {
use zstd_safe::compress_bound;
let compressor = context.zstd_compressor();
// Compute maximum compressed size
let bound = compress_bound(input.len());
// Reserve space and extend buffer to allow in-place write
let offset = output.len();
output.resize(offset + bound, 0);
// Compress directly into output buffer
let written = compressor
.compress_to_buffer(input, &mut output[offset..])
.map_err(|e| ArrowError::ExternalError(Box::new(e)))?;
// Truncate to actual compressed size
output.truncate(offset + written);
Ok(())
}
```
I'll make this modification later today and re-test
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies for non-Dictionary types in IPC writer [arrow-rs]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#issuecomment-4408255411
@alamb Thanks for the valuable feedback. I've re-worked this to:
* remove the separate `write_record_batch_fast` path
* share the reusable scratch-based encoding path instead
* added documentation & comments for the scratch + encoding helpers
* reuse the existing `write_message_parts` writer path
* replaced the fully qualified `crate::FieldNode` / `crate::Buffer` usage with direct imports
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#issuecomment-4372346054
Hi @alamb,
My sincere apologies for the v.late response — I missed this earlier because it's finals week for me rn (I'm a student).
Yes, I do think this's worth pursuing. So, in my Commit all existing supported non-dictionary types keep the same encoding logic, including:
```text
numeric types → fast path
Union → fast path
Utf8 / Binary → fast path
List / Struct / Map → fast path
Boolean → fast path
RunEndEncoded → fast path
```
Only this falls back/uses the current heap Re-allocation path:
```text
DataType::Dictionary(...)
or nested dictionary inside Struct/List/Map/Union/etc.
```
As you've suggested earlier, I'll work on the dictionary path too after this PR & setup a new PR for it after my finals are done within the next 3 days. In the meanwhile, would really appreciate a review if you're able to do so.
Thanks again!
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#issuecomment-4374242884
Thanks @alamb , I really appreciate that
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#issuecomment-4397312250
Marking as draft as I think this PR is no longer waiting on feedback and I am trying to make it easier to find PRs in need of review. Please mark it as ready for review when it is ready for another look
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#issuecomment-4323831587
Ok, so the results are positive (~15% improvement for non-compressed paths), but they're different from what I got, probably because of the different processors. I ran mine on my personal Mac, which has an Intel Core i9 x86_64 processor.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies for non-Dictionary types in IPC writer [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#issuecomment-4614005395
run benchmark ipc_writer
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies for non-Dictionary types in IPC writer [arrow-rs]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#issuecomment-4408532787
> Thansk for this @pchintar --
>
> I am a little worried that this introduces a second almost-but-not-quite-tthe-same path for writing batches to IPC. I think we could do better if we did a more holistic rearranging of the code -- let me know what you think
@alamb The dictionary path remains unchanged in this PR and should not require any additional copies, alignment requirements or regressions for that specific case.
I also looked into threading the writer further into `IpcDataGenerator`, but that would require restructuring the recursive dictionary emission path as well, so I kept this change focused on the shared reusable encoding path for this PR.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies for non-Dictionary types in IPC writer [arrow-rs]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on code in PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#discussion_r3210172297
##########
arrow-ipc/src/writer.rs:
##########
@@ -194,6 +194,26 @@ impl Default for IpcWriteOptions {
/// [Arrow IPC Format]: https://arrow.apache.org/docs/format/Columnar.html#serialization-and-interprocess-communication-ipc
pub struct IpcDataGenerator {}
+struct IpcWriterScratch {
Review Comment:
I've added comments as mentioned
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#issuecomment-4329647727
Or maaybe I misunderstood your intent -- do you want me to review this PR now? Or shall I wait for your next proposal?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies for non-Dictionary types in IPC writer [arrow-rs]
Posted by "adriangbot (via GitHub)" <gi...@apache.org>.
adriangbot commented on PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#issuecomment-4614034762
🤖 Arrow criterion benchmark running (GKE) | [trigger](https://github.com/apache/arrow-rs/pull/9836#issuecomment-4614005395)
**Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB) | `Linux bench-c4614005395-418-8bhzd 6.12.68+ #1 SMP Wed Apr 1 02:23:28 UTC 2026 aarch64 GNU/Linux`
<details><summary>CPU Details (lscpu)</summary>
```
Architecture: aarch64
CPU op-mode(s): 64-bit
Byte Order: Little Endian
CPU(s): 16
On-line CPU(s) list: 0-15
Vendor ID: ARM
Model name: Neoverse-V2
Model: 1
Thread(s) per core: 1
Core(s) per cluster: 16
Socket(s): -
Cluster(s): 1
Stepping: r0p1
BogoMIPS: 2000.00
Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache: 1 MiB (16 instances)
L1i cache: 1 MiB (16 instances)
L2 cache: 32 MiB (16 instances)
L3 cache: 80 MiB (1 instance)
NUMA node(s): 1
NUMA node0 CPU(s): 0-15
Vulnerability Gather data sampling: Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit: Not affected
Vulnerability L1tf: Not affected
Vulnerability Mds: Not affected
Vulnerability Meltdown: Not affected
Vulnerability Mmio stale data: Not affected
Vulnerability Reg file data sampling: Not affected
Vulnerability Retbleed: Not affected
Vulnerability Spec rstack overflow: Not affected
Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1: Mitigation; __user pointer sanitization
Vulnerability Spectre v2: Mitigation; CSV2, BHB
Vulnerability Srbds: Not affected
Vulnerability Tsa: Not affected
Vulnerability Tsx async abort: Not affected
Vulnerability Vmscape: Not affected
```
</details>
Comparing ipc-writer-avoid-repetitive-allocs (6ab2cbbc933a7573d0d227aa6b4b358febf78fba) to 4fa8d2f (merge-base) [diff](https://github.com/apache/arrow-rs/compare/4fa8d2ff5f18f2d773f9642631715509f844a062..6ab2cbbc933a7573d0d227aa6b4b358febf78fba)
BENCH_NAME=ipc_writer
BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench ipc_writer
BENCH_FILTER=
Results will be posted here when complete
---
[File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#issuecomment-4329626109
> I'll make this modification later today and re-test
Thanks @pchintar
I'll mark this PR as draft -- please let me know when it is ready for a look
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies for non-Dictionary types in IPC writer [arrow-rs]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on code in PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#discussion_r3210219705
##########
arrow-ipc/src/writer.rs:
##########
@@ -1550,6 +1596,163 @@ impl<W: Write> RecordBatchWriter for StreamWriter<W> {
}
}
+fn has_dictionary_batch(batch: &RecordBatch) -> bool {
+ batch
+ .schema()
+ .fields()
+ .iter()
+ .any(|field| has_dictionary_type(field.data_type()))
+}
+
+fn has_dictionary_type(data_type: &DataType) -> bool {
+ match data_type {
+ DataType::Dictionary(_, _) => true,
+ DataType::Struct(fields) => fields.iter().any(|f| has_dictionary_type(f.data_type())),
+ DataType::List(field)
+ | DataType::LargeList(field)
+ | DataType::ListView(field)
+ | DataType::LargeListView(field)
+ | DataType::FixedSizeList(field, _) => has_dictionary_type(field.data_type()),
+ DataType::Map(field, _) => has_dictionary_type(field.data_type()),
+ DataType::RunEndEncoded(_, field) => has_dictionary_type(field.data_type()),
+ DataType::Union(fields, _) => fields
+ .iter()
+ .any(|(_, f)| has_dictionary_type(f.data_type())),
+ _ => false,
+ }
+}
+
+fn write_record_batch_fast<W: Write>(
+ writer: &mut W,
+ batch: &RecordBatch,
+ write_options: &IpcWriteOptions,
+ compression_context: &mut CompressionContext,
+ scratch: &mut IpcWriterScratch,
+) -> Result<(usize, usize), ArrowError> {
+ scratch.fbb.reset();
+ scratch.nodes.clear();
+ scratch.buffers.clear();
+ scratch.arrow_data.clear();
+ scratch.variadic_buffer_counts.clear();
+
+ let batch_compression_type = write_options.batch_compression_type;
Review Comment:
Now with shared `encode_record_batch_into(...)` and the removal of `write_record_batch_fast(...)` there is:
* ONE serialization implementation
* reused by both paths & no duplicated logic
So this is resolved now I guess?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#issuecomment-4347466670
> Would this kind of bounded per-batch parallelism be acceptable in arrow-ipc, or would it introduce any new hidden costs? I'm really unsure of the impacts of parallelism here for the write process and so I'm really curious
I don't think adding parallelism (eg. threasd) to the underlying library calls acceptable as it comes with threading and memory overhead (more buffering) that are not appropriate for all usecases
What probably would be useful is if you could find some way (maybe an example) to show people who wanted to make this tradeoff how it could be done
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repetitive heap allocations and buffer copies in IPC writer [arrow-rs]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#issuecomment-4323150901
@alamb and @etseidl the benchmark results of `ipc_writer` show between 30-45% improvements for non-zstd where writer overhead is larger portion of runtime
<img width="1227" height="606" alt="Image 4-26-26 at 5 14 PM" src="https://github.com/user-attachments/assets/79a0b8a0-5819-4915-9938-0dd49e207896" />
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies for non-Dictionary types in IPC writer [arrow-rs]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on code in PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#discussion_r3210219705
##########
arrow-ipc/src/writer.rs:
##########
@@ -1550,6 +1596,163 @@ impl<W: Write> RecordBatchWriter for StreamWriter<W> {
}
}
+fn has_dictionary_batch(batch: &RecordBatch) -> bool {
+ batch
+ .schema()
+ .fields()
+ .iter()
+ .any(|field| has_dictionary_type(field.data_type()))
+}
+
+fn has_dictionary_type(data_type: &DataType) -> bool {
+ match data_type {
+ DataType::Dictionary(_, _) => true,
+ DataType::Struct(fields) => fields.iter().any(|f| has_dictionary_type(f.data_type())),
+ DataType::List(field)
+ | DataType::LargeList(field)
+ | DataType::ListView(field)
+ | DataType::LargeListView(field)
+ | DataType::FixedSizeList(field, _) => has_dictionary_type(field.data_type()),
+ DataType::Map(field, _) => has_dictionary_type(field.data_type()),
+ DataType::RunEndEncoded(_, field) => has_dictionary_type(field.data_type()),
+ DataType::Union(fields, _) => fields
+ .iter()
+ .any(|(_, f)| has_dictionary_type(f.data_type())),
+ _ => false,
+ }
+}
+
+fn write_record_batch_fast<W: Write>(
+ writer: &mut W,
+ batch: &RecordBatch,
+ write_options: &IpcWriteOptions,
+ compression_context: &mut CompressionContext,
+ scratch: &mut IpcWriterScratch,
+) -> Result<(usize, usize), ArrowError> {
+ scratch.fbb.reset();
+ scratch.nodes.clear();
+ scratch.buffers.clear();
+ scratch.arrow_data.clear();
+ scratch.variadic_buffer_counts.clear();
+
+ let batch_compression_type = write_options.batch_compression_type;
Review Comment:
Now with `shared encode_record_batch_into(...)` and the removal of `write_record_batch_fast(...)` there is:
* ONE serialization implementation
* reused by both paths & no duplicated logic
So this is resolved now I guess?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#issuecomment-4330216954
Hi @alamb,
So, I took a closer look at the `ipc_writer` benchmark & zstd path and the main cost seems to come from repeated calls to:
```rust
compress_to_vec(buffer, ...)
```
Right now the flow is strictly serial:
```text
write_array_data
→ write_buffer
→ compress_to_vec (zstd)
```
i.e.
```text
buffer1 → compress → write
buffer2 → compress → write
...
```
Since buffers are independent, I’m considering restructuring this to:
```text
collect buffers → compress in parallel → write in order
```
Conceptually:
```text
[buffer1, buffer2, buffer3]
↓
parallel compress
↓
append results (same order)
```
This would keep the same IPC format and compression behavior, while avoiding any output-size tradeoff.
Implementation-wise, something like:
```rust
let parallelism = thread::available_parallelism()
.map(|n| n.get())
.unwrap_or(1)
.min(4);
```
Then process bounded chunks:
```rust
for chunk in pending_buffers.chunks(parallelism) {
let compressed = compress_chunk_in_parallel(chunk)?;
append_in_original_order(compressed)?;
}
```
where each worker owns its compression context:
```rust
let mut ctx = CompressionContext::default();
codec.compress_to_vec(buffer.as_slice(), &mut out, &mut ctx)?;
```
Would this kind of bounded per-batch parallelism be acceptable in `arrow-ipc`, or would it introduce any new hidden costs?
Thanks!
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies for non-Dictionary types in IPC writer [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#issuecomment-5353081258
could we rebase this PR if we're still keen on it? apologies its been left unreviewed for a while, looks like we've had other performance improvement PRs in the meantime so just wanna check if this is still relevant
- https://github.com/apache/arrow-rs/pull/10044
- https://github.com/apache/arrow-rs/pull/10262
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#issuecomment-4374176277
> My sincere apologies for the v.late response — I missed this earlier because it's finals week for me rn (I'm a student).
No worries at all -- I have been tied up sorting out various release issues, and we are all just trying to do the best we can. Thank you for all your help
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repetitive heap allocations and buffer copies in IPC writer [arrow-rs]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#issuecomment-4323121319
<img width="1986" height="686" alt="Image 4-26-26 at 5 14 PM" src="https://github.com/user-attachments/assets/5422d0d4-3545-4af7-bd22-c0d3a96c5234" />
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
Posted by "adriangbot (via GitHub)" <gi...@apache.org>.
adriangbot commented on PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#issuecomment-4323781103
🤖 Arrow criterion benchmark running (GKE) | [trigger](https://github.com/apache/arrow-rs/pull/9836#issuecomment-4323772156)
**Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB) | `Linux bench-c4323772156-1847-d4rd4 6.12.55+ #1 SMP Sun Feb 1 08:59:41 UTC 2026 aarch64 GNU/Linux`
<details><summary>CPU Details (lscpu)</summary>
```
Architecture: aarch64
CPU op-mode(s): 64-bit
Byte Order: Little Endian
CPU(s): 16
On-line CPU(s) list: 0-15
Vendor ID: ARM
Model name: Neoverse-V2
Model: 1
Thread(s) per core: 1
Core(s) per cluster: 16
Socket(s): -
Cluster(s): 1
Stepping: r0p1
BogoMIPS: 2000.00
Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache: 1 MiB (16 instances)
L1i cache: 1 MiB (16 instances)
L2 cache: 32 MiB (16 instances)
L3 cache: 80 MiB (1 instance)
NUMA node(s): 1
NUMA node0 CPU(s): 0-15
Vulnerability Gather data sampling: Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit: Not affected
Vulnerability L1tf: Not affected
Vulnerability Mds: Not affected
Vulnerability Meltdown: Not affected
Vulnerability Mmio stale data: Not affected
Vulnerability Reg file data sampling: Not affected
Vulnerability Retbleed: Not affected
Vulnerability Spec rstack overflow: Not affected
Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1: Mitigation; __user pointer sanitization
Vulnerability Spectre v2: Mitigation; CSV2, BHB
Vulnerability Srbds: Not affected
Vulnerability Tsa: Not affected
Vulnerability Tsx async abort: Not affected
Vulnerability Vmscape: Not affected
```
</details>
Comparing ipc-writer-avoid-repetitive-allocs (8ce051e126cb09e4a688cb91d54bb3553780decc) to 4fa8d2f (merge-base) [diff](https://github.com/apache/arrow-rs/compare/4fa8d2ff5f18f2d773f9642631715509f844a062..8ce051e126cb09e4a688cb91d54bb3553780decc)
BENCH_NAME=ipc_writer
BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench ipc_writer
BENCH_FILTER=
Results will be posted here when complete
---
[File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies for non-Dictionary types in IPC writer [arrow-rs]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#issuecomment-6281435822
Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies for non-Dictionary types in IPC writer [arrow-rs]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #9836: feat(ipc): Avoid repeated heap allocations and buffer copies for non-Dictionary types in IPC writer
URL: https://github.com/apache/arrow-rs/pull/9836
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#issuecomment-4323313387
@adriangb could you pls run ipc_writer
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies for non-Dictionary types in IPC writer [arrow-rs]
Posted by "pchintar (via GitHub)" <gi...@apache.org>.
pchintar commented on code in PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#discussion_r3210172297
##########
arrow-ipc/src/writer.rs:
##########
@@ -194,6 +194,26 @@ impl Default for IpcWriteOptions {
/// [Arrow IPC Format]: https://arrow.apache.org/docs/format/Columnar.html#serialization-and-interprocess-communication-ipc
pub struct IpcDataGenerator {}
+struct IpcWriterScratch {
Review Comment:
I've added comments as mentioned
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#issuecomment-4347473397
It seems like hte approach of this PR (avoid allocations) still is a benefit -- do you think it is something worth pursuing (aka should I find time to review this PR?)
Thank you for your help (as always)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies for non-Dictionary types in IPC writer [arrow-rs]
Posted by "adriangbot (via GitHub)" <gi...@apache.org>.
adriangbot commented on PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#issuecomment-4614050347
🤖 Arrow criterion benchmark completed (GKE) | [trigger](https://github.com/apache/arrow-rs/pull/9836#issuecomment-4614005395)
**Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB)
<details><summary>CPU Details (lscpu)</summary>
```
Architecture: aarch64
CPU op-mode(s): 64-bit
Byte Order: Little Endian
CPU(s): 16
On-line CPU(s) list: 0-15
Vendor ID: ARM
Model name: Neoverse-V2
Model: 1
Thread(s) per core: 1
Core(s) per cluster: 16
Socket(s): -
Cluster(s): 1
Stepping: r0p1
BogoMIPS: 2000.00
Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache: 1 MiB (16 instances)
L1i cache: 1 MiB (16 instances)
L2 cache: 32 MiB (16 instances)
L3 cache: 80 MiB (1 instance)
NUMA node(s): 1
NUMA node0 CPU(s): 0-15
Vulnerability Gather data sampling: Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit: Not affected
Vulnerability L1tf: Not affected
Vulnerability Mds: Not affected
Vulnerability Meltdown: Not affected
Vulnerability Mmio stale data: Not affected
Vulnerability Reg file data sampling: Not affected
Vulnerability Retbleed: Not affected
Vulnerability Spec rstack overflow: Not affected
Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1: Mitigation; __user pointer sanitization
Vulnerability Spectre v2: Mitigation; CSV2, BHB
Vulnerability Srbds: Not affected
Vulnerability Tsa: Not affected
Vulnerability Tsx async abort: Not affected
Vulnerability Vmscape: Not affected
```
</details>
<details><summary>Details</summary>
<p>
```
group ipc-writer-avoid-repetitive-allocs main
----- ---------------------------------- ----
arrow_ipc_stream_writer/FileWriter/write_10 1.00 163.1±1.46µs ? ?/sec 1.13 184.7±1.68µs ? ?/sec
arrow_ipc_stream_writer/StreamWriter/write_10 1.00 159.2±1.92µs ? ?/sec 1.17 186.3±1.86µs ? ?/sec
arrow_ipc_stream_writer/StreamWriter/write_10/zstd 1.00 7.2±0.03ms ? ?/sec 1.01 7.3±0.11ms ? ?/sec
```
</p>
</details>
<details><summary>Resource Usage</summary>
**base (merge-base)**
| Metric | Value |
|--------|-------|
| Wall time | 30.0s |
| Peak memory | 2.7 GiB |
| Avg memory | 2.6 GiB |
| CPU user | 27.6s |
| CPU sys | 0.6s |
| Peak spill | 0 B |
**branch**
| Metric | Value |
|--------|-------|
| Wall time | 35.0s |
| Peak memory | 2.7 GiB |
| Avg memory | 2.6 GiB |
| CPU user | 31.5s |
| CPU sys | 0.1s |
| Peak spill | 0 B |
</details>
---
[File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
Posted by "adriangbot (via GitHub)" <gi...@apache.org>.
adriangbot commented on PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#issuecomment-4323785479
🤖 Arrow criterion benchmark completed (GKE) | [trigger](https://github.com/apache/arrow-rs/pull/9836#issuecomment-4323772156)
**Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB)
<details><summary>CPU Details (lscpu)</summary>
```
Architecture: aarch64
CPU op-mode(s): 64-bit
Byte Order: Little Endian
CPU(s): 16
On-line CPU(s) list: 0-15
Vendor ID: ARM
Model name: Neoverse-V2
Model: 1
Thread(s) per core: 1
Core(s) per cluster: 16
Socket(s): -
Cluster(s): 1
Stepping: r0p1
BogoMIPS: 2000.00
Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache: 1 MiB (16 instances)
L1i cache: 1 MiB (16 instances)
L2 cache: 32 MiB (16 instances)
L3 cache: 80 MiB (1 instance)
NUMA node(s): 1
NUMA node0 CPU(s): 0-15
Vulnerability Gather data sampling: Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit: Not affected
Vulnerability L1tf: Not affected
Vulnerability Mds: Not affected
Vulnerability Meltdown: Not affected
Vulnerability Mmio stale data: Not affected
Vulnerability Reg file data sampling: Not affected
Vulnerability Retbleed: Not affected
Vulnerability Spec rstack overflow: Not affected
Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1: Mitigation; __user pointer sanitization
Vulnerability Spectre v2: Mitigation; CSV2, BHB
Vulnerability Srbds: Not affected
Vulnerability Tsa: Not affected
Vulnerability Tsx async abort: Not affected
Vulnerability Vmscape: Not affected
```
</details>
<details><summary>Details</summary>
<p>
```
group ipc-writer-avoid-repetitive-allocs main
----- ---------------------------------- ----
arrow_ipc_stream_writer/FileWriter/write_10 1.00 159.7±1.40µs ? ?/sec 1.17 186.2±1.61µs ? ?/sec
arrow_ipc_stream_writer/StreamWriter/write_10 1.00 155.8±1.62µs ? ?/sec 1.18 183.7±1.77µs ? ?/sec
arrow_ipc_stream_writer/StreamWriter/write_10/zstd 1.00 7.2±0.07ms ? ?/sec 1.03 7.4±0.03ms ? ?/sec
```
</p>
</details>
<details><summary>Resource Usage</summary>
**base (merge-base)**
| Metric | Value |
|--------|-------|
| Wall time | 35.0s |
| Peak memory | 2.7 GiB |
| Avg memory | 2.6 GiB |
| CPU user | 30.6s |
| CPU sys | 0.7s |
| Peak spill | 0 B |
**branch**
| Metric | Value |
|--------|-------|
| Wall time | 35.0s |
| Peak memory | 2.6 GiB |
| Avg memory | 2.6 GiB |
| CPU user | 31.0s |
| CPU sys | 0.1s |
| Peak spill | 0 B |
</details>
---
[File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies for non-Dictionary types in IPC writer [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #9836:
URL: https://github.com/apache/arrow-rs/pull/9836#discussion_r3204026164
##########
arrow-ipc/src/writer.rs:
##########
@@ -194,6 +194,26 @@ impl Default for IpcWriteOptions {
/// [Arrow IPC Format]: https://arrow.apache.org/docs/format/Columnar.html#serialization-and-interprocess-communication-ipc
pub struct IpcDataGenerator {}
+struct IpcWriterScratch {
Review Comment:
I think some comments here explaining the rationale for this structure would be useful (avoid allocations during IPC read)
##########
arrow-ipc/src/writer.rs:
##########
@@ -194,6 +194,26 @@ impl Default for IpcWriteOptions {
/// [Arrow IPC Format]: https://arrow.apache.org/docs/format/Columnar.html#serialization-and-interprocess-communication-ipc
pub struct IpcDataGenerator {}
+struct IpcWriterScratch {
+ fbb: FlatBufferBuilder<'static>,
+ nodes: Vec<crate::FieldNode>,
Review Comment:
nit: can you please use `use crate::FieldNode` in the above and then use `FieldNode` directly (same for Buffer below) to avoid fully qulaified names?
##########
arrow-ipc/src/writer.rs:
##########
@@ -1550,6 +1596,163 @@ impl<W: Write> RecordBatchWriter for StreamWriter<W> {
}
}
+fn has_dictionary_batch(batch: &RecordBatch) -> bool {
+ batch
+ .schema()
+ .fields()
+ .iter()
+ .any(|field| has_dictionary_type(field.data_type()))
+}
+
+fn has_dictionary_type(data_type: &DataType) -> bool {
+ match data_type {
+ DataType::Dictionary(_, _) => true,
+ DataType::Struct(fields) => fields.iter().any(|f| has_dictionary_type(f.data_type())),
+ DataType::List(field)
+ | DataType::LargeList(field)
+ | DataType::ListView(field)
+ | DataType::LargeListView(field)
+ | DataType::FixedSizeList(field, _) => has_dictionary_type(field.data_type()),
+ DataType::Map(field, _) => has_dictionary_type(field.data_type()),
+ DataType::RunEndEncoded(_, field) => has_dictionary_type(field.data_type()),
+ DataType::Union(fields, _) => fields
+ .iter()
+ .any(|(_, f)| has_dictionary_type(f.data_type())),
+ _ => false,
+ }
+}
+
+fn write_record_batch_fast<W: Write>(
+ writer: &mut W,
+ batch: &RecordBatch,
+ write_options: &IpcWriteOptions,
+ compression_context: &mut CompressionContext,
+ scratch: &mut IpcWriterScratch,
+) -> Result<(usize, usize), ArrowError> {
+ scratch.fbb.reset();
+ scratch.nodes.clear();
+ scratch.buffers.clear();
+ scratch.arrow_data.clear();
+ scratch.variadic_buffer_counts.clear();
+
+ let batch_compression_type = write_options.batch_compression_type;
Review Comment:
this seems like a copy/paste/modify slight variation of https://github.com/apache/arrow-rs/blob/8ce051e126cb09e4a688cb91d54bb3553780decc/arrow-ipc/src/writer.rs#L1764-L1763
##########
arrow-ipc/src/writer.rs:
##########
@@ -1550,6 +1596,163 @@ impl<W: Write> RecordBatchWriter for StreamWriter<W> {
}
}
+fn has_dictionary_batch(batch: &RecordBatch) -> bool {
+ batch
+ .schema()
+ .fields()
+ .iter()
+ .any(|field| has_dictionary_type(field.data_type()))
+}
+
+fn has_dictionary_type(data_type: &DataType) -> bool {
+ match data_type {
+ DataType::Dictionary(_, _) => true,
+ DataType::Struct(fields) => fields.iter().any(|f| has_dictionary_type(f.data_type())),
+ DataType::List(field)
+ | DataType::LargeList(field)
+ | DataType::ListView(field)
+ | DataType::LargeListView(field)
+ | DataType::FixedSizeList(field, _) => has_dictionary_type(field.data_type()),
+ DataType::Map(field, _) => has_dictionary_type(field.data_type()),
+ DataType::RunEndEncoded(_, field) => has_dictionary_type(field.data_type()),
+ DataType::Union(fields, _) => fields
+ .iter()
+ .any(|(_, f)| has_dictionary_type(f.data_type())),
+ _ => false,
+ }
+}
+
+fn write_record_batch_fast<W: Write>(
Review Comment:
Could you please add some documentation to this function explaining how it is different than `write_record_batch` (I think the big change is that it doesn't handle dictionaries)?
##########
arrow-ipc/src/writer.rs:
##########
@@ -1165,23 +1188,33 @@ impl<W: Write> FileWriter<W> {
));
}
- let (encoded_dictionaries, encoded_message) = self.data_gen.encode(
- batch,
- &mut self.dictionary_tracker,
- &self.write_options,
- &mut self.compression_context,
- )?;
+ let (meta, data) = if has_dictionary_batch(batch) {
+ let (encoded_dictionaries, encoded_message) = self.data_gen.encode(
Review Comment:
If the goal of this PR is to avoid allocations in the IPC writer, I think we should take a more holistic approach
Rather than trying to fast path only the non dictionary path, what do you think about threading the writer somehow into `self.data_gen`?
It seems like the current code buffers the entire record batch's worth of messages into memory, and then writes them all out
It would be more memory efficient I think if the data was written to a (resuable) buffer and then immediately to the writer (or maybe even avoid the buffering and write to the wrtier directly)
You would likely have to do something about dictionary encoding -- but I think it is allowed to write dictionaris a column at a time (you don't have to write them all up front..)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org