Repository navigation
Avoid a per-run zip_eq call in run-end bool decoding - #10962
Conversation
The prefill loops in the run-end bool decoder iterated with `itertools::zip_eq`. `ZipEq::next` is generic and has no `#[inline]`, so rustc emits it in a single codegen unit. When the decoder lands in a different unit, the loop calls it out of line once per run. A change elsewhere in the crate moved the decoder this way in the bench profile (16 codegen units, no LTO), and `decode_bool_nullable` slowed down by 17 to 25% in the CodSpeed simulation. The loops now use `zip`, like the sequential paths in the same file. `runend_decode_bools` checks once that the run ends and the values have the same length, which keeps the check that `zip_eq` made on each run. Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
| // NB: The decode loops pair run ends with values through `zip`, which stops at the shorter | ||
| // input, so this check must stay. Do not switch the loops to `itertools::zip_eq`: `ZipEq::next` | ||
| // has no `#[inline]`, so rustc can place it in another codegen unit, and the loop then calls it | ||
| // out of line once per run. | ||
| assert_eq!( | ||
| ends.len(), | ||
| values.len(), | ||
| "run ends and values must have the same length" | ||
| ); |
There was a problem hiding this comment.
maybe we should upstream this to itertools?
Merging this PR will degrade performance by 6.79%3 warnings
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing |
|
microbenchmarks are a lie I think, this is still a good change |
Summary
The prefill loops in the run-end bool decoder iterate with
itertools::zip_eq.ZipEq::nextis generic and has no#[inline], so rustc emits each instantiation in a single codegen unit, and a loop in a different unit has to call it out of line once per run. #10676 doesn't touch the decoder, but it moves the decoder to another codegen unit in the bench profile (16 codegen units, no LTO), anddecode_bool_nullableslows down by 17 to 25% in the CodSpeed simulation.Changes
The two prefill loops now use
zip, like the sequential paths in the same file.runend_decode_boolsasserts once that the run ends and the values have the same length. This keeps the panic thatzip_eqraised on a mismatch, and the few-runs paths now get it too. Every caller passes the children of aRunEndArray, which already checks the lengths at construction.The x86-64 assembly of
vortex-array, built with the CodSpeed simulation flags at the head of #10676 plus this change, has no call toZipEq::next, even though the decoder is still in the other codegen unit. Ondevelop, the decoder's CodSpeed numbers already match the inlined case, so this PR on its own may not show a change.