viirya commented on code in PR #12024:
URL: https://github.com/apache/arrow-rs/pull/12024#discussion_r4460712772
##########
arrow-buffer/src/builder/boolean.rs:
##########
@@ -276,9 +276,51 @@ impl BooleanBufferBuilder {
self.advance(additional);
let offset = self.len() - additional;
- for (i, v) in slice.iter().enumerate() {
+
+ // Too short to fill a whole byte, so packing would only add overhead
+ if additional < 8 {
Review Comment:
Thanks for rerunning this on Milan. I can't reproduce the x86 numbers here,
but on Apple M4 Max `additional <= 8` is also better for every 8-value case,
and I don't see the single-element slowdown: 7168 single-value appends take
13.4–13.9 µs with either cutoff (30–31 µs on main). So in d71aaf956 the short
register path now takes up to 8 values. With the same bench, 7168 calls, main →
this PR:
- 8 values at offset 3, all false: 27.2 → 16.9 µs
- 8 values at offset 3, random: 167–169 → 17.1–17.2 µs
- 8 values at offset 0, all false: 27.1 → 17.4 µs (22 µs with `< 8`)
- 8 values at offset 0, random: 168 → 17.4 µs (22 µs with `< 8`)
- 1–7 values: unchanged at 18.6 µs (random) and 17.6 µs (all false)
`test_append_slice_all_byte_patterns` now also appends 16 values, so the
multiply path still sees all 256 patterns. Could you check whether the
single-element slowdown you saw on Milan still shows up with this version?
--
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]
