feat(bitpacking): Add batched index unpacking - #190
Conversation
Signed-off-by: Will Manning <will@willmanning.io>
Merging this PR will improve performance by 41.77%
Performance Changes
Tip Curious why performance improved? Comment Comparing Footnotes
|
Signed-off-by: Will Manning <will@willmanning.io>
Signed-off-by: Will Manning <will@willmanning.io>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28f12cd2d9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: Will Manning <will@willmanning.io>
|
Do we know how much this will increase compile time and code gen size? |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce09c988c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: Will Manning <will@willmanning.io>
added ~5% to rlib size, increased release build time by < 1 second |
Signed-off-by: Will Manning <will@willmanning.io>
Signed-off-by: Will Manning <will@willmanning.io>
|
I think you get 99% of this feature if you remove the private #[inline(always)] helper and add #[inline] on unpack_single and uncheck_unpack_single. This would lead to slightly larger rlib size but it's just 0.5% more |
|
also if we document to the callers that if they want to loop over indices they should use instead of then they get the benefits of this pr with minimal changes here |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
That is true when the packed width is a compile-time constant (i.e., Vortex's Vortex can avoid that cost only by duplicating the complete width match outside its loop... which is literally just implementing |
robert3005
left a comment
There was a problem hiding this comment.
ok, we want unpack_indices because we cannot guarantee that unpack_single will be inlined across crates
The benchmarks from #190 did not compare the three paths fairly. The compiler treated the bit width as a constant for some paths and removed work that real callers still do. This PR gives every path the same runtime bit width and covers all four integer types near the candidate cutoffs. It also restores extraneous code comment removals in #190 and makes the new tests fail safely if an unpack method skips an output.
## 🤖 New release * `fastlanes`: 0.7.0 -> 0.7.1 (✓ API compatible changes) <details><summary><i><b>Changelog</b></i></summary><p> <blockquote> ## [0.7.1](v0.7.0...v0.7.1) - 2026-09-01 ### Added - *(bitpacking)* Add batched index unpacking ([#190](#190)) ### Other - *(bitpacking)* more representative unpack_indices benchmarks ([#194](#194)) </blockquote> </p></details> --- This PR was generated with [release-plz](https://github.com/release-plz/release-plz/). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Add
unpack_indicesandunchecked_unpack_indicesto extract selected values without a complete 1,024-value unpack.Use
unpack_indiceswhen the bit width and packed length are compile-time constants. Useunchecked_unpack_indiceswhen the bit width is available only at runtime. It dispatches the runtime width once per batch.Both methods write into
MaybeUninitoutput. Useunpack_singlefor one selected value. Use full unpack above the recommended threshold.One private
#[inline(always)]helper implements single-value extraction. Publicunpack_singlekeeps an out-of-line boundary. The runtime dispatcher and batch path inline the helper after width dispatch. This keeps width-specific expansion inside library code.Performance
Native Apple M1 benchmarks use Rust 1.91.0 and
-C target-cpu=native.At 8 and 32 selected values, batch extraction is 2.0–3.4× faster than repeated
unpack_singlecalls.The table gives a conservative dispatch policy across the sampled packed widths.
u8n <= 16n > 16u16n <= 32n > 32u32n <= 64n > 64u64n <= 160n > 160The grid covers widths 1, 4, and 7 for
u8. It covers widths 1, 3, 8, and 15 foru16.It covers widths 1, 8, 16, 24, and 31 for
u32. It covers widths 1, 16, 32, 48, and 63 foru64.The limiting measured crossovers were 18–19, 32–34, 68–72, and 176–184. The rounded thresholds leave margin for other hardware.
Packed-width effects were non-monotonic, so the policy uses only the physical type and selected-value count.
cargo asmconfirms one runtime-width dispatch per batch. The generatedu16batch function contains no per-indexunpack_singlecalls.Code size
The release rlib grows from 3,750,536 bytes on
developto 3,942,416 bytes, a 5.1% increase. Object text grows from 419,814 bytes to 447,917 bytes, a 6.7% increase.Verification
Tests cover every integer type and runtime width. They also cover empty, duplicate, unordered, full-block, zero-width, and invalid inputs.
🤖 Generated with Codex