Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
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,
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,
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
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) CPU Details (lscpu) ``` 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 ``` group ipc-writer-avoid-repetitive-allocs main - -- arrow_ipc_stream_writer/FileWriter/write_10 1.00159.7±1.40µs ? ?/sec1.17186.2±1.61µs? ?/sec arrow_ipc_stream_writer/StreamWriter/write_10 1.00155.8±1.62µs ? ?/sec1.18183.7±1.77µs? ?/sec arrow_ipc_stream_writer/StreamWriter/write_10/zstd1.00 7.2±0.07ms ? ?/sec1.03 7.4±0.03ms? ?/sec ``` Resource Usage **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 | --- [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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
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` CPU Details (lscpu) ``` 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 ``` 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] feat(ipc): Avoid repeated heap allocations and buffer copies in IPC writer [arrow-rs]
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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
