Repository navigation
Resolve bitpacking functions once for bitwidth - #9721
robert3005 wants to merge 6 commits into
Conversation
Merging this PR will improve performance by 17.63%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | WallTime | words_gather_dispatch_avx512[65536] |
1,021 ns | 868 ns | +17.63% |
| 🆕 | WallTime | bitpack_blocked_compress_avx2 |
N/A | 7.5 µs | N/A |
| 🆕 | WallTime | bitpack_blocked_compress_avx512 |
N/A | 5.6 µs | N/A |
| 🆕 | WallTime | bitpack_blocked_compress_neon |
N/A | 11.5 µs | N/A |
Tip
Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.
Comparing claude/bitpacked-fastlanes-function-pointers-ckpnee (2d8c403) with develop (dc0e885)2
Footnotes
-
518 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. ↩
-
No successful run was found on
develop(cc018ee) during the generation of this report, so dc0e885 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
7f83e7e to
0c12a8f
Compare
|
I might be lacking context, but what's the expected benefit here? |
|
The unchecked_ functions need to perform a switch to pick the right width, that switch might not be properly lifted and eliminated. By using the checked functions we pick the right function ONCE and then dispatch to it. It removes a bunch of logic from the generated code and is more inline friendly since we remove those massive switch statements from hot path |
|
This PR has been marked as stale because it has been open for 14 days with no activity. Please comment or remove the stale label if you wish to keep it active, otherwise it will be closed in 7 days |
|
Shall we try and merge thsi |
|
@joseph-isaacs I think this just needs a tick |
416e15d to
0537277
Compare
d9a1ec6 to
1937091
Compare
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>
1937091 to
2d8c403
Compare
Pull request was closed
4 participants
Instead of using unchecked_* variants of bitpacking functions resolve the functions once. Avoids finding the right generic function for given bitwidth on every call