sunchao commented on code in PR #12046:
URL: https://github.com/apache/arrow-rs/pull/12046#discussion_r4520111554
##########
parquet/src/arrow/buffer/offset_buffer.rs:
##########
@@ -144,6 +178,52 @@ impl<I: OffsetSizeTrait> OffsetBuffer<I> {
}
}
+/// Number of bytes [`copy_value`] copies at once
+const COPY_CHUNK: usize = 16;
+
+/// Copies `src[start..start + len]` to `dst`
+///
+/// Values of up to `4 * COPY_CHUNK` bytes are copied in whole `COPY_CHUNK`
+/// byte chunks when both `src` past `start` and `dst` have room for them, as
+/// this is faster than `memcpy` for short values. This may then write up to
+/// `COPY_CHUNK` bytes past `dst + len`, but never past `dst + dst_len`.
+///
+/// # Safety
+///
+/// - `start + len <= src.len()`
+/// - `len <= dst_len`, and `dst` must be valid for writes of `dst_len` bytes
+/// - `dst..dst + dst_len` must not overlap `src`
+#[inline(always)]
+unsafe fn copy_value(src: &[u8], start: usize, len: usize, dst: *mut u8,
dst_len: usize) {
+ // SAFETY: `start <= src.len()`
+ let src_ptr = unsafe { src.as_ptr().add(start) };
+ // SAFETY: only called below when `i + COPY_CHUNK <= available`, so the
+ // chunk is within both `src[start..]` and `dst..dst + dst_len`
+ let copy_chunk =
+ |i: usize| unsafe { std::ptr::copy_nonoverlapping(src_ptr.add(i),
dst.add(i), COPY_CHUNK) };
+ let available = (src.len() - start).min(dst_len);
+ if len <= COPY_CHUNK && available >= COPY_CHUNK {
+ copy_chunk(0);
+ } else if len > COPY_CHUNK
+ && len <= 4 * COPY_CHUNK
+ && available >= len.next_multiple_of(COPY_CHUNK)
+ {
+ // Explicitly unrolled, as LLVM turns the equivalent loop into `memcpy`
+ copy_chunk(0);
+ copy_chunk(COPY_CHUNK);
+ if len > 2 * COPY_CHUNK {
+ copy_chunk(2 * COPY_CHUNK);
+ if len > 3 * COPY_CHUNK {
+ copy_chunk(3 * COPY_CHUNK);
+ }
+ }
+ } else {
+ // SAFETY: `src[start..start + len]` is valid, and `dst` is valid for
+ // `len` bytes
+ unsafe { std::ptr::copy_nonoverlapping(src_ptr, dst, len) };
Review Comment:
Can we skip the byte copy for empty dictionary values while still appending
their offsets? I read the same in-memory Parquet file containing 65,536
dictionary-encoded empty strings through `ParquetRecordBatchReader` in default
1,024-row batches. The median scan time went from **202 µs** on `e2c48ef` to
**412 µs** on this head (seven alternating release-mode rounds on EPYC Milan,
Rust 1.97.1, identical decoded output).
The base `Vec::extend_from_slice` bypasses the zero-length copy. This path
calls `memcpy(0)` for every key in the optimized Linux build. A scratch build
with only a zero-length early return in `copy_value` brought the scan back to
**201 µs** and kept most of the measured gains for 8- and 19-byte strings.
--
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]
