Skip to content

perf(sequence): build single-match bitmap directly - #9262

Open
tomatotomata wants to merge 5 commits into
vortex-data:developfrom
tomatotomata:codex/sequence-compare-single-bit
Open

perf(sequence): build single-match bitmap directly#9262
tomatotomata wants to merge 5 commits into
vortex-data:developfrom
tomatotomata:codex/sequence-compare-single-bit

Conversation

@tomatotomata

@tomatotomata tomatotomata commented Aug 7, 2026

Copy link
Copy Markdown

Summary

The sequence equality kernel knows that a match bitmap contains exactly one set bit, but it currently evaluates a predicate for every row to build that bitmap. This change allocates an unset BitBufferMut, sets the known match index, and freezes it. The PR stays limited to the production comparison path after removing unrelated benchmark-harness edits.

Testing

  • rustfmt on encodings/sequence/src/compute/compare.rs
  • git diff --check
  • Focused cargo test -p vortex-sequence --lib --no-default-features compute::compare::tests was attempted twice but could not complete because the shallow checkout needs the uncached tpchgen Git dependency.

AI assistance

This PR was prepared with agentic AI assistance. I inspected the issue and surrounding implementation, kept the change scoped to the requested path, and recorded the local validation limitation above.

Fixes #9092

}

const NUM_VALUES: u64 = 100_000;
const SEQUENCE_COMPARE_LENGTHS: [usize; 3] = [1_000, 100_000, 1_000_000];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use NUM_VALUES

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Switched both comparison cases to the existing NUM_VALUES workload in b7f33d0.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please read the benchmarking docs

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Read docs/developer-guide/benchmarking.md and updated both cases to pass the length and match index through with_inputs, removing the captured values and std::hint::black_box. The focused run measured 19.99 microseconds median for from_iter and 87.3 nanoseconds for the single-set path, both below the 1 ms per-iteration limit.

Comment on lines +59 to +61
let mut buffer = BitBufferMut::new_unset(lhs.len());
buffer.set(set_idx);
let buffer = buffer.freeze();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NICE

@tomatotomata
tomatotomata force-pushed the codex/sequence-compare-single-bit branch from b7f33d0 to e1d3874 Compare August 8, 2026 00:03
@robert3005 robert3005 added the changelog/performance A performance improvement label Aug 8, 2026
@codspeed-hq

codspeed-hq Bot commented Aug 8, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 12.1%

❌ 1 regressed benchmark
✅ 1943 untouched benchmarks
🆕 2 new benchmarks
⏩ 85 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation decompress[u64, (10000, 4)] 316.9 µs 360.6 µs -12.1%
🆕 Simulation bench_sequence_compare_match_from_iter N/A 235.7 µs N/A
🆕 Simulation bench_sequence_compare_match_single_set N/A 22.1 µs N/A

Tip

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


Comparing tomatotomata:codex/sequence-compare-single-bit (e1d3874) with develop (5794e7d)

Open in CodSpeed

Footnotes

  1. 85 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.

@tomatotomata

tomatotomata commented Aug 9, 2026

Copy link
Copy Markdown
Author

I dug into the CodSpeed report and pushed the focused follow-up through f325e4d. The 12.1% regression was reported in the unrelated run-end decompression benchmark, while the production change is limited to the sequence comparison bitmap path. I removed the extra benchmark-harness edits, restored the benchmark file to its base contents, added the authorized DCO sign-off to the follow-up history, and updated the PR description. git diff --check passes. Let me know what you think.

Signed-off-by: ahmad <ahmadalgaidy@hotmail.com>
Signed-off-by: ahmad <ahmadalgaidy@hotmail.com>
This reverts commit e1d3874.

Signed-off-by: ahmad <ahmadalgaidy@hotmail.com>
Signed-off-by: ahmad <ahmadalgaidy@hotmail.com>
Signed-off-by: ahmad <ahmadalgaidy@hotmail.com>
@tomatotomata
tomatotomata force-pushed the codex/sequence-compare-single-bit branch from cdcd3bf to f325e4d Compare August 9, 2026 09:48
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.

Sequence compare bit buffer optimization (good first issue)

3 participants