Punisheroot opened a new pull request, #24071:
URL: https://github.com/apache/datafusion/pull/24071
## Which issue does this PR close?
- Closes #13867.
## Rationale for this change
`ArrowBytesMap` only requires growable byte storage while values are being
inserted. `BufferBuilder<u8>` wraps `MutableBuffer`, whose allocations use
64-byte alignment. This alignment is unnecessary for byte storage and makes
buffer growth more expensive.
A `Vec<u8>` provides the required append, lookup, length, and capacity
operations. When the map is materialized, `Buffer::from_vec` transfers the
allocation into an Arrow `Buffer` without copying it.
## What changes are included in this PR?
- Replace the internal `BufferBuilder<u8>` in `ArrowBytesMap` with `Vec<u8>`.
- Use `extend_from_slice` when storing new values.
- Convert the completed `Vec<u8>` into an Arrow `Buffer` without copying.
- Add a focused Criterion benchmark covering:
- unique 4-byte values;
- unique 32-byte values;
- 32-byte values with low cardinality.
This PR intentionally changes only `ArrowBytesMap`. The other structures
mentioned in #13867 are left for separate follow-up PRs.
## Are these changes tested?
Yes.
Validation performed on Ubuntu 24.04 under WSL2 with Rust 1.97.0:
- `cargo fmt --all --check`
- `cargo clippy --all-targets --all-features -- -D warnings`
- `cargo test -p datafusion-physical-expr-common --all-features`
- 80 unit tests passed
- 8 doctests passed
- `cargo test -p datafusion-physical-plan group_values`
- 65 tests passed
### Benchmark results
Criterion comparison against the `BufferBuilder<u8>` implementation at
commit `f9dde71ec`, using 100 samples, a 3-second warm-up, and a 5-second
measurement period:
| Benchmark | BufferBuilder | Vec | Criterion result |
|---|---:|---:|---|
| `short_unique` | 306.39 us | 302.80 us | Within noise threshold |
| `long_unique` | 410.23 us | 159.08 us | 61.24% lower time |
| `long_low_cardinality` | 49.90 us | 48.97 us | No change detected |
For `long_unique`, throughput increased by approximately 158%. Repeating the
comparison with the execution order reversed produced approximately 158.57 us
for `Vec` and 440.05 us for `BufferBuilder`.
No stable performance regression was observed in the short-value or
low-cardinality cases.
## Are there any user-facing changes?
No. This is an internal implementation and performance change with no public
API or behavior changes.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
