pchintar commented on code in PR #9778:
URL: https://github.com/apache/arrow-rs/pull/9778#discussion_r3133816560
##########
arrow-ipc/src/reader.rs:
##########
@@ -868,16 +868,47 @@ fn get_dictionary_values(
Ok(dictionary_values)
}
-/// Read the data for a given block
+/// Reads the full data block (metadata + body) from the underlying reader.
+///
+/// Uses a zero-initialized buffer for small blocks. For larger blocks, reads
+/// into a temporary `Vec<u8>` and reuses the allocation when it is 64-byte
+/// aligned, matching Arrow's `ALIGNMENT` for `MutableBuffer`. Otherwise, it
+/// falls back to copying into an Arrow-aligned buffer.
+///
+/// This reduces redundant zero-initialization on large reads while preserving
+/// the alignment expected by Arrow buffers.
fn read_block<R: Read + Seek>(mut reader: R, block: &Block) -> Result<Buffer,
ArrowError> {
reader.seek(SeekFrom::Start(block.offset() as u64))?;
let body_len = block.bodyLength().to_usize().unwrap();
let metadata_len = block.metaDataLength().to_usize().unwrap();
let total_len = body_len.checked_add(metadata_len).unwrap();
- let mut buf = MutableBuffer::from_len_zeroed(total_len);
- reader.read_exact(&mut buf)?;
- Ok(buf.into())
+ if total_len < 8 * 1024 {
+ let mut buf = MutableBuffer::from_len_zeroed(total_len);
+ reader.read_exact(&mut buf)?;
+ return Ok(buf.into());
+ }
+
+ let mut vec = Vec::with_capacity(total_len);
+ reader
+ .by_ref()
+ .take(total_len as u64)
+ .read_to_end(&mut vec)?;
+
+ if vec.len() != total_len {
+ return Err(ArrowError::IpcError(format!(
+ "Expected IPC block of length {total_len}, got {}",
+ vec.len()
+ )));
+ }
+
+ if ((vec.as_ptr() as usize) & 63) == 0 {
+ Ok(Buffer::from_vec(vec))
+ } else {
+ let mut buf = MutableBuffer::from_len_zeroed(total_len);
Review Comment:
Thanks @alamb — this is very helpful.
My local results were positive, but the CI benchmark clearly shows the
current hybrid approach regresses on this machine, so I simplified this and
used the always-`Vec' approach now. i got these results below:
arrow_ipc_reader/StreamReader/read_10
time: [766.81 µs 769.58 µs 772.72 µs]
change: [−9.3763% −6.4994% −3.1025%] (p = 0.00 <
0.05)
Performance has improved.
Found 10 outliers among 100 measurements (10.00%)
5 (5.00%) high mild
5 (5.00%) high severe
arrow_ipc_reader/StreamReader/no_validation/read_10
time: [296.39 µs 297.77 µs 299.29 µs]
change: [−36.261% −32.040% −27.379%] (p = 0.00 <
0.05)
Performance has improved.
Found 15 outliers among 100 measurements (15.00%)
5 (5.00%) high mild
10 (10.00%) high severe
arrow_ipc_reader/StreamReader/read_10/zstd
time: [4.4544 ms 4.4599 ms 4.4659 ms]
change: [−0.8033% −0.6462% −0.4666%] (p = 0.00 <
0.05)
Change within noise threshold.
Found 3 outliers among 100 measurements (3.00%)
2 (2.00%) high mild
1 (1.00%) high severe
arrow_ipc_reader/StreamReader/no_validation/read_10/zstd
time: [3.9956 ms 3.9998 ms 4.0050 ms]
change: [−3.1747% −1.6976% −0.8610%] (p = 0.01 <
0.05)
Change within noise threshold.
Found 6 outliers among 100 measurements (6.00%)
5 (5.00%) high mild
1 (1.00%) high severe
arrow_ipc_reader/FileReader/read_10
time: [763.61 µs 766.17 µs 769.31 µs]
change: [−16.186% −12.265% −8.2022%] (p = 0.00 <
0.05)
Performance has improved.
Found 13 outliers among 100 measurements (13.00%)
5 (5.00%) high mild
8 (8.00%) high severe
arrow_ipc_reader/FileReader/no_validation/read_10
time: [298.19 µs 299.92 µs 301.84 µs]
change: [−33.724% −29.865% −25.465%] (p = 0.00 <
0.05)
Performance has improved.
Found 10 outliers among 100 measurements (10.00%)
5 (5.00%) high mild
5 (5.00%) high severe
Benchmarking arrow_ipc_reader/FileReader/read_10/mmap: Warming up for 3.0000
s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase
target time to 7.8s, enable flat sampling, or reduce sample count to 50.
arrow_ipc_reader/FileReader/read_10/mmap
time: [1.5154 ms 1.5176 ms 1.5202 ms]
change: [−0.8244% +0.0946% +0.6836%] (p = 0.85 >
0.05)
No change in performance detected.
Found 3 outliers among 100 measurements (3.00%)
1 (1.00%) high mild
2 (2.00%) high severe
arrow_ipc_reader/FileReader/no_validation/read_10/mmap
time: [119.04 µs 119.33 µs 119.65 µs]
change: [−0.7147% −0.3515% −0.0017%] (p = 0.05 <
0.05)
Change within noise threshold.
Found 2 outliers among 100 measurements (2.00%)
2 (2.00%) high mild
--
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]
