Repository navigation
Keep codecs and filters registered from Python on one thread - #785
Open
FabianIsensee wants to merge 1 commit into
Open
FabianIsensee wants to merge 1 commit into
FabianIsensee wants to merge 1 commit into
Conversation
Codecs and filters registered from Python run Python code, which needs the
GIL, so C-Blosc2 must never run them on its worker threads. The SChunk
and NDArray constructors refuse nthreads > 1 for them, but C-Blosc2 picks
the thread count itself in two cases, and the interpreter crashes:
- Containers made by schunk_from_cframe()/ndarray_from_cframe(), or opened
with SChunk(urlpath=..., mode="r"), get C-Blosc2's global thread count.
After set_nthreads(4) reading one segfaults. blosc2.open() gives the
container blosc2.nthreads (ncores by default) and then refused it:
"Cannot use multi-threading with user defined Python codecs".
- BLOSC_NTHREADS replaces the nthreads of every context C-Blosc2 creates,
one included, so with BLOSC_NTHREADS=4 even SChunk(data=..., cparams=
{"codec": <python codec>, "nthreads": 1}) segfaults while compressing.
Give such containers one-thread contexts where they are made from frames
or files, and let open() use one thread for them. While BLOSC_NTHREADS is
above one, raise a ValueError instead: when a codec or filter with Python
functions is registered, and where contexts for data that uses one are set
up (_check_cparams(), _check_dparams() and the new one-thread helper), since
C-Blosc2 reads the variable again for every context.
The tests run every case that could crash in a child interpreter.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01DiPFztdpnKMJkzrjT36BuP
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.
Codecs and filters registered with Python functions (
register_codec(),register_filter()) need the GIL, so C-Blosc2 must not run them on its worker threads. The SChunk/NDArray constructors refusenthreads > 1for them, but C-Blosc2 picks the thread count itself in two cases, and the interpreter crashes:schunk_from_cframe()/ndarray_from_cframe(), or opened withSChunk(urlpath=..., mode="r"), get C-Blosc2's global thread count. Afterblosc2.set_nthreads(4), reading one segfaults.blosc2.open()gives the containerblosc2.nthreads(the number of cores by default) and then refuses it withValueError: Cannot use multi-threading with user defined Python codecs, unless the caller passes cparams and dparams withnthreads=1.BLOSC_NTHREADSreplaces the nthreads of every context that C-Blosc2 creates, an explicit 1 included. WithBLOSC_NTHREADS=4,blosc2.SChunk(data=..., cparams={"codec": <python codec>, "nthreads": 1, ...})segfaults while compressing.This PR changes only python-blosc2:
One thread for existing containers:
_one_thread_for_python_callbacks()gives a super-chunk whose cparams name a codec or filter registered from Python one-thread contexts (the constructor's rule). It is called afterschunk_from_cframe(),ndarray_from_cframe(),SChunk(mode="r")andopen().open()also usesnthreads=1in the cparams/dparams it applies to such containers, so it opens them instead of raising; explicitnthreads > 1passed toopen()for such a container is reduced to 1 (the Python-levelopen()fills inblosc2.nthreadsas the default, so the two cannot be told apart). Other containers are not affected. These are the only places where a container gets contexts that python-blosc2 did not ask for. The stores, proxies andfrom_cframe()go through them;copy(), slices and the setters build their contexts from parameters that_check_cparams()/_check_dparams()check.BLOSC_NTHREADS: while it is above 1, the code raisesValueError: BLOSC_NTHREADS=4 is not supported with codecs or filters registered from Python: C-Blosc2 would run them on 4 threads, but they need one (unset BLOSC_NTHREADS or set it to 1)instead of crashing. The effective count comes fromcontext_nthreads(1), which creates a context asking for one thread and reads back what C-Blosc2 gave it. It is checked at three sites:register_codec()/register_filter()with Python functions. This catches the usual case, the variable set before the process starts, for every path. A process that setsBLOSC_NTHREADSabove 1 and registers a Python codec now fails at registration instead of crashing at first use._check_cparams()/_check_dparams()when the cparams name such a codec or filter, which covers the constructors and the cparams/dparams setters._one_thread_for_python_callbacks(), which covers the containers above,open()included.The last two are needed because C-Blosc2 reads the variable again for every context, so it can also be set after registering.
Not covered when
BLOSC_NTHREADSis set only after registering:compress2()/decompress2()/decompress()/vldecompress(); Python codecs need an SChunk, so these calls fail anyway, and Decide threads and the GIL per chunk for Python codecs and filters #769 gives them one thread);Python prefilters/postfilters with
BLOSC_NTHREADSare not covered at all (same cause, but they are per-context callbacks, not registered ids).Cost: a few byte lookups per container made from a frame or file;
context_nthreads(1)(one context created and freed) only for data that uses a Python codec or filter.Tests (
tests/test_python_callbacks_nthreads.py, all in child interpreters, skipped on WASM):set_nthreads(4): an SChunk fromschunk_from_cframe(),open()andSChunk(mode="r"), and an NDArray fromndarray_from_cframe()andopen(), with a Python codec, plus (frame, open) with a Python filter. Each case reads and, where possible, writes.BLOSC_NTHREADS=4at start-up: registering a Python codec or filter must raise.BLOSC_NTHREADS=4set after registering (codec and filter): a new SChunk, a frame copy,open()and the dparams setter must raise. The check is skipped if C-Blosc2 does not see a variable set throughos.environ, since it cannot use it either then.open()ValueError) and pass here.Perf (GCC 14 wheels, main built twice, EPYC node, 5 interleaved rounds):
schunk_from_cframe()(copy and view),ndarray_from_cframe(),SChunk(mode="r")andopen()of an SChunk and an NDArray (8-72 us per call), each with and without a Python codec registered, are within -1.2%..+1.5% of main (main vs main: -0.6%..+1.8%), so there is no measurable change.Relation to open PRs (
git merge-tree: clean with all of them, #769-#775 and #777-#783):decompress2()/decompress()/vldecompress()use one thread for chunks of Python codecs;BLOSC_NTHREADSdefeats that. Here the variable is refused at registration (and for containers), not inside those calls._reads_need_python(sc), the same check as_runs_python_callbacks(sc.storage.cparams)here; the names differ, so both build together, and once both are in, one can go.Question for Francesc: would you want a C-level fix as well? C-Blosc2 could itself run chunks of codecs and filters that need the calling thread one block after another, whatever
nthreadsorBLOSC_NTHREADSsay. This could be done with additive registration functions that take a flags argument (blosc2_register_codec_flags()/blosc2_register_filter_flags(), no change to the structs or the ABI). That would make the one-thread workarounds here, in #769 and in #775 unnecessary, andBLOSC_NTHREADSwould keep working with Python codecs. I have a prototype with tests and can send it as a PR if you want it.I used Claude Code (Anthropic) to trace the crashes, write the change and the tests, and run the test suites and benchmarks; I reviewed the result.
🤖 Generated with Claude Code
https://claude.ai/code/session_01DiPFztdpnKMJkzrjT36BuP