Resolve bitpacking functions once for bitwidth - #9721
Open
robert3005 wants to merge 6 commits into
Open
Conversation
Merging this PR will degrade performance by 12.36%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | take[duplicates/repeated/primitive/nonnull/chunks=16/indices=1000] |
198.7 µs | 233 µs | -14.74% |
| ❌ | WallTime | words_gather_scalar_avx2[65536] |
8.3 µs | 9.4 µs | -11.64% |
| ❌ | Simulation | compress_fsst[(500, 64, 8)] |
483.1 µs | 540.6 µs | -10.64% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing claude/bitpacked-fastlanes-function-pointers-ckpnee (0c12a8f) with develop (f80b83c)
Footnotes
-
206 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. ↩
Comment on lines
+211
to
+233
| #[inline(always)] | ||
| fn as_block<P, const N: usize>(slice: &[P]) -> &[P; N] { | ||
| match slice.try_into() { | ||
| Ok(block) => block, | ||
| Err(_) => block_len_mismatch(N, slice.len()), | ||
| } | ||
| } | ||
|
|
||
| #[inline(always)] | ||
| fn as_block_mut<P, const N: usize>(slice: &mut [P]) -> &mut [P; N] { | ||
| let len = slice.len(); | ||
| match slice.try_into() { | ||
| Ok(block) => block, | ||
| Err(_) => block_len_mismatch(N, len), | ||
| } | ||
| } | ||
|
|
||
| /// Kept out of line so the kernel wrappers stay frameless trampolines: the panic formatting | ||
| /// would otherwise reserve stack on every call. | ||
| #[cold] | ||
| #[inline(never)] | ||
| fn block_len_mismatch(expected: usize, actual: usize) -> ! { | ||
| vortex_panic!("Expected a FastLanes block of {expected} elements, got {actual}") |
Contributor
There was a problem hiding this comment.
hmm..
No inline(always) please.
Are you sure the try_into() doesn't copy?
BitPacked arrays are type erased, so decoding went through the fastlanes `unchecked_*` entry points, which re-dispatch on the runtime bit width with a 65-arm match for every 1024-value block (and for every `scalar_at`). Add `BitPackedKernels`, a set of function pointers to the const-width kernel instantiations (`unpack`, `unpack_single`, `unfor_pack`), lazily resolved once per array through a `OnceLock` on `BitPackedData` and shared by every decode path: canonicalization, mapped cast, take, filter, scalar_at, is_constant, between, and the fused FoR decompress. The fused compare kernel resolves its `unpack_cmp` instantiation once per call, since it is generic over the comparison closure. The resolved kernels take slices and check block lengths, so the unsafe runtime-width calls are gone from the decoding logic. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R9mScTPzB4Vw4PCdSyc862 Signed-off-by: Claude <noreply@anthropic.com>
`bitpack_primitive` still dispatched on the runtime bit width through `unchecked_pack` for every 1024-value block. Resolve the const-width pack kernel once per call via `BitPackedPhysical::resolve_pack` instead, so no path outside kernel resolution matches on the width. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R9mScTPzB4Vw4PCdSyc862 Signed-off-by: Claude <noreply@anthropic.com>
Inspecting the release assembly showed two misses. `BitPackedData::kernels` was not inlined, so `scalar_at` paid an out-of-line call before the indirect kernel call; mark it `#[inline]`. Every kernel wrapper also carried the `vortex_panic!` formatting for a block-length mismatch inline, which reserved a stack frame on the hot path; move it into a `#[cold]` out-of-line function so the wrappers reduce to a length compare and a tail jump into the fastlanes kernel. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R9mScTPzB4Vw4PCdSyc862 Signed-off-by: Claude <noreply@anthropic.com>
…ters Resolve the FastLanes kernels when a `BitPackedData` is constructed rather than on first use. `BitPackedData::try_new` now takes the array's `PType`, rejects a bit width wider than the type, and stores the kernels directly, so the `OnceLock` is gone. Replace the per-type `ResolvedKernels` enum with a type-erased `BitPackedKernels<()>`: the function pointers are transmuted to a placeholder element type for storage and transmuted back by `typed::<P>()`, which checks the recorded physical `PType` first. This removes the enum, the `kernels_from` trait method, and the per-call variant match; the accessor is now a ptype compare and a struct copy. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R9mScTPzB4Vw4PCdSyc862 Signed-off-by: Claude <noreply@anthropic.com>
Make the resolved kernel pointers `unsafe fn` with the same length contract the fastlanes `unchecked_*` entry points had. The wrappers now reinterpret the slices with a `debug_assert` and a pointer cast, as the original code did, instead of a checked `try_into` and an out-of-line panic. In release the block wrappers reduce to a bare tail jump into the fastlanes kernel. Callers carry the SAFETY reasoning the checks used to enforce. `BitPackedData` no longer duplicates `bit_width`: the resolved kernels already record it, and `bit_width()` reads it from there. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R9mScTPzB4Vw4PCdSyc862 Signed-off-by: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R9mScTPzB4Vw4PCdSyc862 Signed-off-by: Claude <noreply@anthropic.com>
robert3005
force-pushed
the
claude/bitpacked-fastlanes-function-pointers-ckpnee
branch
from
September 2, 2026 17:59
7f83e7e to
0c12a8f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Instead of using unchecked_* variants of bitpacking functions resolve the functions once. Avoids finding the right generic function for given bitwidth on every call