Optimize bit iteration and validity checking with SIMD popcount

[robert3005]

recreation of #8261 which had real performance benefits for iterating valid indices and had guidelines for future agents performance work

[codspeed-hq]

Merging this PR will improve performance by ×2.4

⚡ 14 improved benchmarks
✅ 1588 untouched benchmarks
🆕 5 new benchmarks
⏩ 42 skipped benchmarks¹

Performance Changes

	Mode	Benchmark	BASE	HEAD	Efficiency
⚡	Simulation	slice_bounds[(16384, 0.1)]	38.7 µs	3 µs	×13
⚡	Simulation	slice_bounds[(16384, 0.9)]	38.7 µs	3 µs	×13
⚡	Simulation	slice_bounds[(4096, 0.1)]	11.3 µs	2.2 µs	×5
⚡	Simulation	slice_bounds[(4096, 0.9)]	11.3 µs	2.2 µs	×5
⚡	Simulation	many_indices[(16384, 0.1)]	51.2 µs	16.3 µs	×3.1
⚡	Simulation	many_indices[(16384, 0.9)]	51.2 µs	16.3 µs	×3.1
⚡	Simulation	many_indices[(4096, 0.1)]	19.9 µs	14.8 µs	+34.44%
⚡	Simulation	many_indices[(4096, 0.9)]	19.9 µs	14.8 µs	+34.44%
⚡	Simulation	filter_run_end[len=16384_run=16_density=0.9]	87.8 µs	69.1 µs	+27.18%
⚡	Simulation	filter_run_end[len=16384_run=16_density=0.5]	88.2 µs	69.5 µs	+27.02%
⚡	Simulation	filter_run_end[len=16384_run=16_density=0.1]	89 µs	70.2 µs	+26.73%
⚡	Simulation	filter_run_end[len=4096_run=16_density=0.5]	27.9 µs	23 µs	+21.31%
⚡	Simulation	filter_run_end[len=4096_run=16_density=0.9]	27.9 µs	23 µs	+21.31%
⚡	Simulation	bitwise_not_vortex_buffer_mut[128]	273.6 ns	244.4 ns	+11.93%
🆕	Simulation	for_each_set_index[(16384, 0.01)]	N/A	8.4 µs	N/A
🆕	Simulation	for_each_set_index[(16384, 0.1)]	N/A	17.9 µs	N/A
🆕	Simulation	for_each_set_index[(16384, 0.5)]	N/A	56.3 µs	N/A
🆕	Simulation	for_each_set_index[(16384, 0.9)]	N/A	85.3 µs	N/A
🆕	Simulation	for_each_set_index[(16384, 0.99)]	N/A	89.8 µs	N/A

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.

---

_{Comparing claude/bit-iterator-perf-0IMQ5 (e53eab2) with develop (b0d7e8c)²}

Footnotes

1. 42 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

2. No successful run was found on develop (25b6b55) during the generation of this report, so b0d7e8c was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

[0ax1]

@claude review in depth

[github-actions]

Claude finished @0ax1's task in 3m 36s —— View job

---

In-depth review

I reviewed the full diff (git diff HEAD^...HEAD) across vortex-buffer, vortex-mask, encodings/runend, and vortex-duckdb, plus the new benchmarks and AGENTS.md guidance. Overall this is a clean, correct, well-tested performance change. The core primitives (count_range, for_each_set_index) are sound and the call-site rewrites preserve semantics. Details below.

• Gather context (diff, changed files)
• Review vortex-buffer bit buffer changes
• Review vortex-mask changes
• Review runend filter & duckdb bool exporter changes
• Review benchmarks & AGENTS.md
• Investigate reported regression / correctness
• Post review

---

Correctness (verified)

BitBuffer::count_range (vortex-buffer/src/bit/buf.rs:390) — Counts count_ones(buffer, offset + start, end - start), correctly accounts for the buffer's bit offset, and asserts start <= end and end <= len (panics instead of UB on misuse). Good. The offset-aware test (test_count_range_with_offset) covers the tricky case.

BitBuffer::for_each_set_index (vortex-buffer/src/bit/buf.rs:434) — The all-ones fast path (word == u64::MAX → emit base..base+64) is only safe because iter_padded() zero-fills the trailing bits of the final partial word, so a fully-set u64 word can only be a genuine full chunk. I confirmed this against arrow_buffer::BitChunks semantics, and it's directly exercised by test_for_each_set_index_all_set (len=130, not a multiple of 64) and the sparse tests at len 65/200/1000. Correct.

Mask::valid_counts_for_indices (vortex-mask/src/lib.rs:634) — The gap-accumulation rewrite (count_range(prev, next_idx) summed over monotonically increasing gaps) is equivalent to the old bit-by-bit walk, giving count[0, next_idx). The boundary behavior is preserved: the old code panicked once the iterator was exhausted (next_idx > len), and the new assert!(next_idx <= buffer.len()) matches exactly (next_idx == len remains valid, consistent with the [0, n_rows] doc). Non-increasing/duplicate indices also behave identically.

filter_run_end_primitive (encodings/runend/src/compute/filter.rs:78) — mask.count_range(start, end) replaces the per-bit value_unchecked loop; count += run_trues and keep = run_trues > 0 are equivalent to the old count += pred / keep |= pred. start = end is preserved. The old unchecked read relied on end <= mask.len(); count_range now asserts this, so it's strictly safer. The NumCast::from(run_trues) conversion is bounded by the run length (≤ a run-end value, which fits in R by construction), so the vortex_expect won't fire in practice.

BoolExporter::export (vortex-duckdb/src/exporter/bool.rs:49) — Zipping the sliced bit iterator into as_slice_mut(len) drops the throwaway Vec<bool> + extra copy. dst and the iterator are both length len, so every destination slot is written. Correct and a nice allocation saving.

Minor / non-blocking

• vortex-mask/src/lib.rs:645 — Switching from vortex_panic! to a bare assert! loses the project's VortexError-style panic formatting. The check itself is preserved (assert!, not debug_assert!, so it stays in release builds). Consider assert!(next_idx <= buffer.len(), "Row indices exceed array length") → keep as-is, or restore vortex_panic! for consistency with surrounding code. Trivial.
• for_each_set_index all-ones branch (buf.rs:439) — for k in 0..64 { f(base + k); } is fine; clippy may prefer (base..base + 64).for_each(&mut f). Cosmetic.
• The public count_range / for_each_set_index doc comments are good (they explain why, not just what) — matches the repo's API-doc requirement.

On the CodSpeed report

The only regression flagged is slice_empty_vortex (-14.66%), which exercises slicing an empty buffer and touches none of the code paths changed here. Combined with the explicit "Unknown Walltime execution environment / inconsistent data on Hosted Runners" warning in the same comment, this reads as measurement noise, not a real regression from this PR. The 48 new benchmarks are the ones this PR adds, and they demonstrate the intended wins (e.g. is_valid_per_element 186µs vs execute_mask_then_value 8.9µs at 1024). Worth a quick re-run / acknowledgement on CodSpeed rather than a code change.

Tests & benches

Test coverage is solid: parameterized rstest cases for count_range (with offset) and for_each_set_index (empty, sub-word, multi-word, all-set, offset). The new benches (validity_is_valid, mask_iteration, valid_counts, run_end_filter, bool_export) are registered with harness = false and directly back the guidance added to AGENTS.md.

Suggested checks before merge (I did not run these in-sandbox):

cargo nextest run -p vortex-buffer -p vortex-mask -p vortex-runend -p vortex-duckdb
cargo clippy --all-targets --all-features

Verdict: LGTM. Correct, semantics-preserving, well-tested. Only nits are cosmetic; the flagged regression appears to be runner noise on an unrelated empty-slice benchmark.
· claude/bit-iterator-perf-0IMQ5

[0ax1]

should we land the benchmarks upfront. then the codspeed reports could confirm the speedup. i assume this is thoroughly unit tested so this is my main nit.

[robert3005]

Made #8656. I will rebase this once that merges

[0ax1]

Disabled auto-merge. Feel free to land if the benchmarks look good.

[0ax1]

@robert3005 benchmarks looking good!