Repository navigation
Skip the index store in mark_indexes_stale for unindexed arrays - #742
Merged
FrancescAlted merged 2 commits intoNov 5, 2026
Merged
Conversation
Contributor
Author
|
Heads-up: I just opened #744 (python-blosc2 performance and bug fixes, companion of Blosc/c-blosc2#833). It contains this PR's commits unchanged, since the work there was done on top of it. You can merge this one first (then #744's diff gets smaller) or close it when merging #744. Whatever is easier for you. |
Member
|
Please, rebase. |
NDArray.__setitem__ calls mark_indexes_stale() after every write. Loading the store resolves the urlpath and stats every cached persistent path, and on NFS each stat() of a file with buffered writes first pushes them to the server. Return early when no cached store has indexes and the array's vlmeta has no index store. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Move _may_have_indexes next to the store helpers, iterate over a snapshot of the persistent store cache, and use plain helpers in the two tests. Co-Authored-By: Claude Opus 5.5 <[email protected]>
FabianIsensee
force-pushed
the
setitem-skip-unindexed-store
branch
from
November 3, 2026 09:07
03db12a to
028e5ac
Compare
Contributor
Author
|
done :-) |
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,
this is the python-blosc2 companion to Blosc/c-blosc2#821. As described there, we use blosc2 in nnU-Net and ran into very slow writes on our network drives. I investigated this together with Claude Opus 5.5. With the c-blosc2 fix in place, most of the remaining cost of
arr[...] = xon NFS comes from python-blosc2's index bookkeeping, which this PR addresses.Problem
NDArray.__setitem__callsindexing.mark_indexes_stale()after every write, also for arrays that have no index. For persistent arrays this loads the index store, which does aPath.resolve()plus astat()for every cached persistent path. On NFS astat()of a file with buffered writes first pushes those writes to the server. Right now nobody notices because c-blosc2 flushes on every chunk update anyway, but with Blosc/c-blosc2#821 these stats become the main cost of a setitem.Fix
mark_indexes_stale()returns early if the array is persistent, no store cached in the process has indexes and the array's vlmeta has noblosc2_indexesentry. In all other cases the existing code runs unchanged.resize()benefits too since it calls the same function.Benchmarks
setitem, one 786 KB chunk per call, clevel 0, on NFS with the c-blosc2 fix: ~8 ms -> 1.0 ms per chunk. Without the c-blosc2 fix there is no measurable difference.
This one is probably mostly NFS-specific (it's the flush-before-stat that hurts), although on other network filesystems a
stat()can still cost a round trip to the server. Not tested.Gotchas
_purge_stale_persistent_caches()still stats every cached path on each write, so the cost grows with the number of arrays a process has touched. Might be worth a separate lookTests
test_setitem_without_indexes_skips_index_store: setitem on an unindexed persistent array must not load the index store (fails on main)test_setitem_marks_on_disk_index_stale_with_cold_caches: with all in-process caches cleared, setitem still marks an index that is only known from the file's vlmeta as staleb2viewHTTP tests that also fail on main (missing optional module)AI disclosure: I used Claude Code (Anthropic) for the investigation, the code, the tests and a first draft of this description.
Best,
Fabian