Repository navigation
Fix frame-backed crashes, NumPy keys, update_special and BLOSC_NTHREADS (part 1/4 of #744) - #746
Open
FabianIsensee wants to merge 7 commits into
Open
FabianIsensee wants to merge 7 commits into
FabianIsensee wants to merge 7 commits into
Conversation
This was referenced Nov 3, 2026
Merged
NDArray.get_slice_numpy() decompressed with the GIL held, so threads reading separate arrays, or separate handles of one file, ran one at a time; on NFS this included the wait for the server. Release the GIL around b2nd_get_slice_cbuffer(). The GIL was also what kept two threads from using one super-chunk at the same time. Replace the per-NDArray read_lock with one reentrant lock per super-chunk, shared by the SChunk wrapper, the NDArray, its views and vlmeta, and hold it in every entry point into C. Keep the GIL when a Python postfilter, codec or filter could run during the read, and on wasm32. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01SwMZj7Wi7Sc8HtQpELyNXB
Hold the operands' handle locks while the miniexpr prefilter of a lazy expression reads their super-chunks from C. Count the reads that run without the GIL, so that exit and the registration of Python codecs and filters wait for them, and free handle locks that other threads held at a fork in the child. Lock the metalayer counters too. Skip the new tests on wasm32, shorten them, retry the GIL check on busy machines, and add tests for lazy expressions and fork. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01SwMZj7Wi7Sc8HtQpELyNXB
The miniexpr prefilter read the chunks of every in-memory operand from sc->data, and the fast matmul prefilter did so for every operand. Frames have no chunk table there: in-memory arrays created with contiguous=True, arrays from ndarray_from_cframe() (copy=False, the default) and, for matmul, any on-disk array. Any lazy expression or matmul with such an operand dereferenced a NULL table and crashed the interpreter. Read frame-backed in-memory operands through blosc2_schunk_get_chunk(), as on-disk ones already are, and keep frame-backed operands off the fast matmul path: in its Python checks, and in _set_pref_matmul() itself, which now raises ValueError for them before changing any state. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01SwMZj6jNaqsesMKnBde8Lr
The general indexing path (ndindex) raised, or returned a wrong shape, for keys that NumPy indexes fine: 554 of 18000 random keys in a comparison with NumPy. Fix the classes found: - slice bounds beyond the index range, and bool bounds or steps, next to ..., None or NumPy integers raised IndexError or TypeError; normalize them as NumPy reads them (clamped, bools as ints) before ndindex sees them; - 0-d integer arrays index as integers (AttributeError before); - scalar bools are 0-d boolean indices, as in NumPy: their dimension goes among adjacent advanced indices (integers included), else in front, and next to array indices they broadcast (True: no effect, False: an empty result or IndexError). The result shape comes from NumPy on a stride-0 stand-in for the array, so it costs no memory; - None next to (adjacent) array indices is supported; - 0-d arrays take NumPy scalar bools (AttributeError before) and raise IndexError for integer lists (ValueError before). Advanced indices separated by slices, ... or None still raise NotImplementedError (NumPy moves their dimensions to the front there). After this, none of the 18000 keys gives another result than NumPy; the remaining differences are that limitation and which error a key that is invalid for several reasons reports first. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01SwMZj6jNaqsesMKnBde8Lr
- update_special() built its special chunk in a temporary SChunk with default cparams, so on an NDArray's super-chunk the chunk header had another block size than the array (e.g. 20000 instead of 800 bytes) and every later read of the array failed: b2nd reads blocks selectively and checks their number per chunk. The new SChunk._special_chunk() makes zero, NaN, uninitialized and repeated-value chunks with the super-chunk's own cparams. - update_data() and insert_data() with copy=False did not free their compressed chunk when the C call failed (nor on a compression error), so it leaked wherever c-blosc2 did not free it itself. They now free it whenever the call fails. This needs the next c-blosc2 release (or the companion c-blosc2 PR) for its caller-keeps-the-chunk-on-failure rule; with an older c-blosc2, a failed frame update can free the chunk twice. - decompress_chunk() without dst fetched the whole compressed chunk only to read its size from the header, then read it again to decompress it; it now gets the lazy chunk (header and block offsets) for the size. astype(float64) of a 300x512x512 float32 array on disk: 385 -> 366 ms. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01SwMZj6jNaqsesMKnBde8Lr
LazyExpr._collect_flat_indices_from_bool_ndarray(), used for 1-D masks, took every chunk as chunk_len items, so the padding of the last chunk could yield positions past the end of the array: blosc2.full(1000, True, chunks=(300,)) gave 1200 positions. Clip each chunk to the items inside the array. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01SwMZj6jNaqsesMKnBde8Lr
C-Blosc2 uses BLOSC_NTHREADS, when set to a number it accepts, for every compression and decompression context, whatever nthreads the parameters ask for. blosc2.nthreads, and so the CParams/DParams defaults and numexpr's thread count, ignored it: with BLOSC_NTHREADS=4, Python reported 28 threads and gave numexpr 28 while C used 4. Start blosc2.nthreads from what C-Blosc2 gives a compression context (the new blosc2_ext.context_nthreads(), via blosc2_ctx_get_cparams()), and say in set_nthreads()'s docs that the variable wins at the C level. Importing blosc2 also overrode NUMEXPR_NUM_THREADS, which set_nthreads() recommends setting; it now leaves numexpr alone then, and otherwise gives it at most numexpr.MAX_THREADS (BLOSC_NTHREADS can go beyond that, which numexpr refuses with an error). Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01SwMZj6jNaqsesMKnBde8Lr
FabianIsensee
force-pushed
the
split-fixes
branch
from
November 4, 2026 13:30
cb8de96 to
8ed4a48
Compare
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.
Hi Francesc,
this is part 1 of 4 of the split of #744, following your proposal in #744 (comment): the bug fixes.
Dependencies:
update_data()/insert_data()withcopy=Falserely on its rule that a failed chunk write leaves the chunk with the caller. It is in c-blosc2 3.5.0, which main now bundles, so this branch no longer needs a TEMP commit.Commits (from #744):
8fee90b,0677313,4301191,6abf3f6and8ed4a48. The miniexpr guard and the lazy-expression speedups of #744 are not included. As you suggested, they wait for #741.Bug fixes (each with a test that fails without it)
contiguous=True, orndarray_from_cframe(cf)with the defaultcopy=False) dereferenced a NULL chunk table. So did the fastmatmulpath with on-disk or frame-backed operands.__getitem__'s general path raised, or returned wrong shapes, where NumPy returns a result: huge slice bounds, bool steps next to.../None/NumPy ints,(None, array), 0-d integer arrays, scalar bools next to arrays.update_specialon an NDArray's super-chunk wrote chunks with the wrong block size, so every later read failed.update_data/insert_data(copy=False)calls freed their chunk again after c-blosc2 had (double free), or leaked it, depending on where the C call failed. Fixed together with c-blosc2's ownership rule (Fix error-path leaks, dictionary chunks and offsets typesize (part 1/6 of #833) c-blosc2#835).BLOSC_NTHREADSdecides the real thread count in C, butblosc2.nthreadsignored it.NUMEXPR_NUM_THREADSwas overridden at import.Also:
decompress_chunk()reads only the chunk header for the size.Behaviour changes
blosc2.nthreadsreflectsBLOSC_NTHREADS.Found, not fixed (questions for you)
NDArray.schunk.lockingreports False even when the array was opened withlocking=True.pickle.dumps/copy.copy/copy.deepcopyraise TypeError for NDArray, SChunk and LazyExpr. Nothing inside python-blosc2 needs them, but user code with process pools has to pass paths orto_cframe()bytes.Tests
pytest tests: 9536 passed, 280 skipped, 2 failed with the bundled c-blosc2 3.5.0. The 2 failures are theb2viewhttp tests that need aiohttp, which also fail on main.AI disclosure: I used Claude Code (Anthropic) for the investigation, the code and the tests of #744, which I reviewed and tested. Claude Code also did this split and opened this PR on my behalf, as agreed in #744 (comment). The description is assembled from the one of #744.
Best,
Fabian
🤖 Generated with Claude Code