Repository navigation
Refactor phase1 - #25
Merged
Merged
Conversation
The dense route of bslz4_csc_decode_multi did its mask+threshold collect as a scalar loop, while the sparse route and bslz4_decode_multi already call bslz4_collect_gt/_nz, which dispatch to the AVX-512/AVX2/SSE2/VSX/ NEON tiers. Call bslz4_collect_gt there too (main loop and tail): same semantics (mask > 0 and value > cut, raster order), so outputs are unchanged. Also drop BSLZ4_UNLIKELY from the dense accumulate's mask check. It is the common case, so the hint laid the hot path out as the cold one. Measured pinned on one Zen 4 core (EPYC 9454), u16, forced dense route, 1D pyFAI matrix (4.47M pixels, 1.7 entries/pixel), ms per frame: frames per call 1 25 before 18.5 16.1 after 12.8 10.9 Instructions per frame fell from 235M to about 182M. 403 tests pass. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Introduce bslz4_driver.c (owns the block/tail loop once), bslz4_registry.h (function-pointer stage/inner types), bslz4_common.h (C error codes, BE readers) and kernels_generic.cpp (the only dtype-generic work: sparse<T> / sparse_dot<T> for the 10 dtypes). Re-express bslz4_decode_multi and bslz4_csc_decode_multi as driver calls; generated layer and native API unchanged. Results are bit-identical to HEAD and within ~1% of HEAD perf.
Phase 2 (native cut-over): add the two dtype-agnostic entry points (bslz4_sparsify, bslz4_sparsify_and_dot) plus offsets_to_pointers, note_chunk, impl_available and per-implementation counters in bslz4_to_sparse.c with a hand-written c2py23 spec; add bslz4_registry.c (resolve, availability, counters) and the C bslz4_codec.h / bslz4_untranspose.h / bslz4_collect_caps.h headers. Regenerate the wrapper (17,659 -> 2,438 lines). Rewrite src/__init__.py over the new API with stage tables, pack_pipeline and adapters for _BSLZ4_MULTI/_BSLZ4_CSC_MULTI/ _BSLZ4_CSC_MULTI_BASE. Delete the generator, bslz4_core.hpp and the old generated bslz4_to_sparse.cpp; drop the padded-CSC family (test_padded.py fails by design). Update MANIFEST.in, check_sdist.py, build_extension.py. Phase 3 (matrix normaliser / selection hook): normalise_matrix() converts pyFAI/scipy/duck-typed CSC to f32/u32 once and validates contents; select_dot() returns the csc id. No new kernels. Non-padded suite: 403 passed, 2 skipped. Bit-identical sparse output and powder vs the git-head reference (parity digest matches). Perf within ~4% of git head on both routes.
Restrict the CSC matrix indices/indptr format check to 4-byte ints ('I'/'i')
so an int64 array is rejected at the boundary instead of being misread as
uint32 (plan.md section 8). Gitignore the local reference/phase build dirs.
Fortran-like aliasing/const-ness: restrict on all pointer arguments in the stage/inner fn-pointer typedefs (bslz4_registry.h), the driver (bslz4_driver.c), the codec/untranspose helpers and the generic kernels_generic.cpp templates + extern C wrappers; const on all read-only data. Behaviour is bit-identical (parity digest unchanged).
The hot per-block work no longer goes through an indirect function-pointer call. The dtype-agnostic C driver (bslz4_driver.c) now decides the dense/sparse route per block (it knows decoded_bytes/nbytes/dense_sparse_x) and calls a single C-linkage switch dispatch in the C++ TU (bslz4_sparse_dispatch / bslz4_sparse_dot_dispatch in kernels_generic.cpp); the per-dtype handlers are static inline templates in that same TU, so they inline into the switch bodies (no second call, jump table only). The CSC matrix + powder are bundled into a bslz4_csc struct (out/nout/data/ indices/indptr), and the dense path no longer takes tidx/tval. The struct fields are hoisted into local restrict pointers in the kernels so the compiler keeps them in registers. Bit-identical (parity digest unchanged); non-padded suite 403 passed, 2 skipped. Dense-vs-sparse route timing now matches the git-head reference (previous dense-data/sparse-route regression ~9-12% is gone).
…for review
The pipeline's collect tier id now actually drives the SIMD collect kernel
(bslz4_collect_gt/nz take the resolved collect id, picked with a predicted
if-chain rather than an indirect jump table so the hot path is unchanged),
and pack_pipeline / the optional pipeline= constructor arg are honoured
instead of being ignored. The module-level _make_* adapters resolve the
default pipeline at call time, so set_<tier>_collect toggles still work.
The instrumentation counters are now per-block: the driver bumps
DECOMPRESS/UNTRANSPOSE/COLLECT/DOT once per block (inline, no call in the
kernel), so a test can show the selected impl ran in every route and in the
tail and that the scalar collect tier was not used for a SIMD dtype.
Cleanup for review:
* Python/C stage-id agreement, pack_pipeline, resolution-error, int64-reject
and per-block counter tests (test/test_refactor.py);
* commit the bit-identical parity comparator (test/_parity.py);
* drop the dead _REBIND stub and the unused bslz4_active_collect_tier();
* fix the wrapper banner to the real c2py23 v0.5.8 commit and stop
regenerate_wrapper.py from walking up to an enclosing repo;
* update stale comments referencing bslz4_core.hpp / bslz4_to_sparse.cpp /
the old function-pointer dispatch.
Bit-identical (parity digest unchanged); non-padded suite 408 passed, 2
skipped. Dense-data/sparse-route timing is back at parity with the git-head
reference (the earlier ~8-14% regression was the collect switch, now an
if-chain).
Replace the single giant bslz4_sparse[_dot]_dispatch switch (which inlined
every dtype into one body and hurt register allocation on the hot path)
with a thin dtype switch that calls small, *noinline* per-dtype kernels.
Every kernel takes one pointer to a bslz4_work context (a single bundle of
the per-block state, replacing a dozen scalar ABI args), unpacks the fields
once into locals, and inlines its own template. Each kernel is thus
independently register-allocated; the switch stays small and scales to many
variants without bloating any one function.
The driver fills the bslz4_work per block and calls the dispatchers with a
single pointer; the bslz4_csc struct is folded into bslz4_work.
Variant mechanism: the dot id selects the dense-route strategy.
dot 0 = csc, dense route runs dot.dense then a separate >cut collect.
dot 1 = NEW dense_dot_fused variant that fuses the >cut sparsify into the
CSC loop (one pass over the block). It is bit-identical to dot 0
(same accumulation order, same ascending output) but scans the
block/mask once instead of twice.
The per-dtype kernel picks it as: w->dot_id == 1 ? dense_dot_fused<T> :
dense_dot<T>. Adding a further variant is just another leaf kernel + case,
not a re-spin of the whole dispatch. dot id 1 is registered as available in
bslz4_registry.c and selectable via chunk2sparseCSC*multi(dot=...) /
pack_pipeline(dot=1).
Pipeline repack (forced by c2py23): c2py23 accepts only signed-int scalar
inputs (parser.py _PYTYPE_MAP = {buffer,int,float}); a u64 pipeline param is
read as a 32-bit int, so the plan's 12-bit fields at bits 24..63 (dot at 36,
options at 48) are unreachable. Packed instead into the low 24 bits (4 bits
per stage id, 8 bits options) in BSLZ4_PIPE_MAKE and Python _pack.
Verify: bit-identical parity digest unchanged; non-padded suite 409 passed,
2 skipped; dense-data/sparse-route now faster than the git-head reference
(~3.92-4.00 ms vs ~4.06-4.16), i.e. the earlier ~4% codegen gap from the
mega-function is gone.
c2py23 accepts only int/float/buffer scalar inputs, so the packed u64
pipeline word was read as a signed 32-bit int; the dot (bit 36+) and options
(bit 48+) fields could never reach the C side. Replace the packed word with
a uint16_t stages[BSLZ4_STAGES_N] array -- one entry per stage/option
(decompress, untranspose, collect, dot, options bitmask) -- passed as a
c2py buffer (format 'H', n == 5). No packing/unpacking; the stage index
constants BSLZ4_STAGE_* and BSLZ4_STAGES_OPTIONS are the accessors.
* bslz4_common.h: drop BSLZ4_PIPE_* macros; add BSLZ4_STAGES_N /
BSLZ4_STAGES_OPTIONS / BSLZ4_OPT_DROP_NEGATIVES.
* bslz4_to_sparse.c: native entry points take const uint16_t *stages;
the C2PY_BEGIN spec declares stages as a buffer; wrapper regenerated.
* bslz4_registry.c: bslz4_resolve reads stages[BSLZ4_STAGE_*].
* __init__.py: pack_pipeline builds a np.uint16[5] array; _stages replaces
the old _pack bit-shift; adapters pass the array straight through.
Also fold the plan.md deviation notes on disk for the new structure:
stage-array pipeline (1 per option), per-dtype noinline kernels +
bslz4_work context instead of the fn-pointer table, the fused dot variant,
and the c2py 'int'/'float'/'buffer'-only input limitation.
Verify: bit-identical parity digest unchanged; non-padded suite 409 passed,
2 skipped; dense-route timing unchanged (unfused 3.087, fused 2.896 ms vs
git 3.107).
The per-tier collect functions (set_/get_/<tier>_collect_available) are gone from the public API; tier selection is now pack_pipeline(collect=...).
On Windows (LLP64) NumPy exposes a uint32 array with PEP 3118 format
char 'L' (unsigned long, 4 bytes) instead of 'I'. The sparsify_and_dot
indices/indptr checks only allowed 'I'/'i', so a Windows uint32 array was
rejected with 'format == "I" (got format="L")'. Broadening the checks
to accept 'I'/'i'/'L'/'l' and requiring itemsize == 4 mirrors the existing
output_adr ('I' or 'L') and npx_out ('i' or 'l') checks.
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
1 participant
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.
No description provided.