Skip to content

Avoid a per-run zip_eq call in run-end bool decoding - #10962

Merged
connortsui20 merged 1 commit into
developfrom
ct/runend-bool-decode-zip
Oct 29, 2026
Merged

connortsui20 merged 1 commit into
developfrom
ct/runend-bool-decode-zip

Conversation

@connortsui20

Copy link
Copy Markdown
Member

Summary

The prefill loops in the run-end bool decoder iterate with itertools::zip_eq. ZipEq::next is 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), and decode_bool_nullable slows 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_bools asserts once that the run ends and the values have the same length. This keeps the panic that zip_eq raised on a mismatch, and the few-runs paths now get it too. Every caller passes the children of a RunEndArray, 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 to ZipEq::next, even though the decoder is still in the other codegen unit. On develop, the decoder's CodSpeed numbers already match the inlined case, so this PR on its own may not show a change.

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>
@connortsui20 connortsui20 added the changelog/performance A performance improvement label Oct 29, 2026
Comment on lines +44 to +52
// 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"
);

@connortsui20 connortsui20 Oct 29, 2026 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we should upstream this to itertools?

@codspeed

codspeed Bot commented Oct 29, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 6.79%

3 warnings

⚠️ Walltime results may be unreliable

Some benchmarks used the Walltime instrument outside CodSpeed Macro Runners, which can make the results inconsistent.

Learn more about Macro Runners

See affected benchmarks

⚠️ 3 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

⚠️ Base and head ran on different runner types

Some benchmarks with significant performance changes were compared across different runner types, so the changes might not come from the code. Run base and head on the same runner type, or use CodSpeed Macro Runners for more stable results.

Learn more about Macro Runners

Open the report in CodSpeed to investigate

⚡ 5 improved benchmarks
❌ 7 regressed benchmarks
✅ 2292 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ Walltime mul_u64_nonnull_neon 20.2 µs 39.8 µs -49.26%
❌ CPU Simulation bench_compare_varbin[(50000, 2)] 133.8 µs 163 µs -17.87%
❌ CPU Simulation bench_compare_varbin[(50000, 64)] 135.6 µs 165 µs -17.83%
❌ CPU Simulation filter_powerlaw_by_random[10000] 670 ns 803.7 ns -16.64%
❌ CPU Simulation bench_compare_sliced_dict_varbinview[(7500, 10000)] 200.8 µs 227.9 µs -11.86%
❌ CPU Simulation bench_compare_sliced_dict_varbinview[(10000, 10000)] 207 µs 234.1 µs -11.56%
❌ CPU Simulation bench_compare_sliced_dict_varbinview[(1000, 10000)] 79.9 µs 90.2 µs -11.33%
⚡ CPU Simulation missing_stats[16] 86.3 µs 67.9 µs +27.04%
⚡ CPU Simulation random_i8[0.01] 123.3 µs 99.1 µs +24.44%
⚡ Walltime threshold_avx512[80] 2.8 µs 2.5 µs +11.8%
⚡ Walltime add_constant_shapes_neon[(32768, ConstantPerRow)] 15.8 µs 14.2 µs +11.44%
⚡ Walltime add_constant_shapes_neon[(32768, PerRowConstant)] 15.8 µs 14.2 µs +10.66%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing ct/runend-bool-decode-zip (da08a30) with develop (891c01a)

Open in CodSpeed

@connortsui20

Copy link
Copy Markdown
Member Author

microbenchmarks are a lie I think, this is still a good change

@connortsui20
connortsui20 marked this pull request as ready for review October 29, 2026 14:25
@connortsui20
connortsui20 enabled auto-merge (squash) October 29, 2026 14:25
@connortsui20
connortsui20 merged commit f04a28c into develop Oct 29, 2026
112 of 116 checks passed
@connortsui20
connortsui20 deleted the ct/runend-bool-decode-zip branch October 29, 2026 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/performance A performance improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants