Repository navigation
Make the matmul, miniexpr and Menudet prefilters unable to need the GIL - #774
Open
FabianIsensee wants to merge 1 commit into
Open
FabianIsensee wants to merge 1 commit into
FabianIsensee wants to merge 1 commit into
Conversation
C-Blosc2 runs prefilters on its worker threads when nthreads > 1 (and on the calling thread without the GIL when compressions release it), but the error paths of the miniexpr and matmul prefilters raised Python exceptions: Cython took the GIL for the raise and the wrapper then called PyErr_Occurred() without it. With the caller holding the GIL, any error (e.g. an operand chunk that fails to decompress) deadlocked matmul and legacy-evaluator expressions and reductions with nthreads > 1; without it, it segfaulted. Declare miniexpr_prefilter, matmul_prefilter and their helpers `noexcept nogil` and remove every `raise` (Cython would still wrap one in an implicit `with gil`); use cdivision there and in the Menudet prefilter (the zero-division checks took the GIL as well). aux_miniexpr() records its error in its worker slot and returns -1; _store_data() and _prefilter_data() raise it (C-Blosc2 ignores the return code of a prefilter with a disposable output, i.e. of reductions). aux_matmul() frees what it allocated and returns -1. These prefilters, and the workers of parallel native graph maps, decode their inputs themselves, so operands with a codec or filter registered from Python ran that callback without the GIL (segfault; the Menudet reduction even with one thread). The routing gates (matmul fast path, _set_pref_expr(), _set_pref_matmul(), Menudet input eligibility, native graph layout) now decline such operands, so they take another path. matmul_prefilter raised "Unsupported dtype" for dtypes other than float and int; blosc2.matmul() never sent those, but it sent float16 and longdouble (kind "f"), which the kernels computed as float64: wrong results, and for float16 reads and writes past the block buffers. The fast path now takes float32/float64 only, and _set_pref_matmul() checks the dtypes before the prefilter is set. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DiPFztdpnKMJkzrjT36BuP
This was referenced Nov 6, 2026
This branch has not been deployed
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
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.
C-Blosc2 calls prefilters from its worker threads when
nthreads > 1, and from the calling thread without the GIL when a compression releases it (set_releasegil(True),_can_compress_without_gil())._check_cparams()allows that for the miniexpr, matmul and Menudet prefilters because they are C-only, but two of them were not: the error paths ofminiexpr_prefilter/aux_miniexpr()andmatmul_prefilter/aux_matmul()raised Python exceptions. Cython wraps araisein anogilfunction in an implicitwith gil, and the (notnogil) wrappers then calledPyErr_Occurred()without the GIL. In addition, all of these prefilters decompress their operands themselves, so a codec or filter registered from Python ran on threads without the GIL.What still happens on main (5ff9823, after #741), with an operand chunk whose header parses but whose blocks cannot be read (block offsets past the chunk):
blosc2.matmul(x1, x2, ...)through the fast path: deadlock with 4 threads (the caller holds the GIL inside the compression and the worker that wants it for theraisewaits forever), segfault underset_releasegil(True)with 1 and 4 threads.BLOSC2_EXPRESSION_EVALUATOR=legacy(the miniexpr prefilter): the same deadlocks and segfaults (sum()included, whose prefilter compression now also releases the GIL), andSystemError: <class 'ValueError'> returned a result with an exception setforsum()with one thread.MenudetError), so those cases are not part of this PR any more.Operands compressed with a codec or filter registered from Python, all segfaults on main: matmul fast path, legacy expressions and reductions, and the typed evaluator's Menudet prefilter (1-D operands; the reduction even with one thread, since its driver runs without the GIL), all with 4 threads; and
(x1 + x2)[:]on >= 2M items through the parallel native graph map, whose workers also decode the inputs themselves. With one thread the matmul and legacy paths fail with an error, because the callbacks need an SChunk that the prefilter's decompression context does not have.dtypes: the
raise ValueError("Unsupported dtype")inmatmul_prefilteris not reachable throughblosc2.matmul(), whose fast path wantsdtype.kind == "f". But float16 and longdouble are kind "f" too, andaux_matmul()computed them with the float64 kernel: NaN/garbage results, and for float16 reads of 4x the operand block buffers and writes of 4x the output block. Calling_set_pref_matmul()directly also accepted int8/int16 (int64 kernel, same overrun), complex, bool, unsigned (the raise) and mixed or structured dtypes.What this PR does:
miniexpr_prefilter,matmul_prefilter,aux_miniexpr(),aux_matmul()andmatmul_block_kernel()arenoexcept nogiland contain noraiseand no Python object. These two and the Menudet prefilter usecdivision, because Cython's zero-division checks take the GIL too (all divisors there are shapes and strides > 0). In the generated C, none of the three prefilters and their helpers calls the Python C API any more.aux_miniexpr()keeps its first error in its worker slot (me_worker_s.error, like the Menudet workers' status) and returns -1;_store_data()(writes) andSChunk._prefilter_data()(reductions, where C-Blosc2 ignores the return code because the output is disposable) raise it asRuntimeError("miniexpr: ...").aux_matmul()frees what it allocated (its 2-pointer tables are on the stack now) and returns -1;blosc2.matmul()then falls back to its chunked path as for other fast-path failures._set_pref_matmul()raisesTypeErrorbefore setting the prefilter unless both operands and the output have one dtype out of float32, float64, int32, int64 (native byte order);matmul_prefilterreturns -1 for anything else._matmul_can_use_fast_path()takes float32/float64 only._check_cparams()does) are declined by the routing gates: the matmul fast-path predicate,_set_pref_expr(),_set_pref_matmul(),menudet_prefilter_inputs_eligible()andnative_graph._compressed_execution_layout(). Those operations take their other paths, which decompress with the GIL.Not in this PR: a chunk written with other cparams than the array's (e.g. through
update_chunk()) is not caught by the gate. A guard inside the Python codec/filter trampolines would catch every GIL-free call, butPyGILState_Check()is not in the Limited API that the abi3 wheels use.Tests:
tests/ndarray/test_prefilter_errors.py: each case runs in a child interpreter (python test_prefilter_errors.py <case> <args>, astests/test_python_codec_threads.pydoes), whose faulthandler ends it after 25 s with the stacks of its threads (plus a 30 ssubprocesstimeout), so a regression fails instead of hanging CI or killing the worker. On WASM (no subprocesses, no threads) the single-thread cases with the GIL run in-process and the others are skipped.test_prefilter_error_raises(matmul, legacycompute(), legacysum(); 1 and 4 threads; with and withoutset_releasegil(True)): aRuntimeError, for the miniexpr cases with the prefilter's message. On main: 3 deadlocks (timeout), 6 segfaults, 1SystemError, 1ValueError(one thread with the GIL, where main raises the prefilter's error directly), and matmul with one thread and the GIL passes. The legacy cases are skipped on NumPy < 2 and WASM, where the legacy evaluator does not use the miniexpr prefilter.test_prefilter_skips_python_callback_operands(Python filter on every route, Python codec on two; 1 and 4 threads): exact results. On main: 14 of the 16 cases fail (10 segfaults, 4 errors); only the single-thread typedcompute()and[:]cases pass.tests/ndarray/test_linalg.py:test_matmul_fast_path_skips_float_dtypes_without_kernel(float16, and longdouble where it is wider than float64: chunked path, exact results; fails on main),test_set_pref_matmul_rejects_dtypes_without_kernel(complex, bool, int16, float16, structured, mixed, non-native byte order:TypeErrorand no prefilter left set; fails on main),test_matmul_prefilter_supported_dtypes(float32, float64, int32, int64 through the prefilter, 1 and 4 threads, exact results; passes on main too).-Wall -Wextra).Performance: no measurable change.
blosc2.matmul()through the fast path (float32/float64, 512² to 2048²),(x * y + x).compute()and.sum()with the typed evaluator (1-D, 1e7 and 4e7 items: Menudet prefilter and native graph routes) and with the legacy one (2000² and 4000², miniexpr prefilter), 1 and 8 threads, A/B against main and main built a second time, alternating, 6 rounds on a 2x EPYC 9554 node pinned to 16 cores, wheels built with GCC 15: fix/main geomean 0.996 (main vs main 0.997), every cell within the main-vs-main spread except the 4e7-item typedsum()with 8 threads at +1.8%. That cell runs code this PR does not change: it is +1.8% also with the Python routing gate removed, and 0.994 with GCC 13 builds of the same code, so it is code placement.AI disclosure: I used Claude Code (Anthropic) to find the problems, write the fix and the tests, run the benchmarks and draft this description. I reviewed and tested everything.
🤖 Generated with Claude Code
https://claude.ai/code/session_01DiPFztdpnKMJkzrjT36BuP