-
-
Notifications
You must be signed in to change notification settings - Fork 420
perf: use sync methods for chunk encoding / decoding #3885
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
d-v-b
wants to merge
99
commits into
zarr-developers:main
Choose a base branch
from
d-v-b:perf/prepared-write-v2
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+5,345
−453
Open
Changes from all commits
Commits
Show all changes
99 commits
Select commit
Hold shift + click to select a range
a75604a
feat: define `PreparedWrite` and `SupportsChunkPacking` data structures
d-v-b a072c31
Merge branch 'main' of https://github.com/zarr-developers/zarr-python…
d-v-b 47a407f
feat: new codec pipeline that uses sync path
d-v-b 3c27e49
feat: complete second codecpipeline
d-v-b 9b834a4
Merge branch 'main' of https://github.com/zarr-developers/zarr-python…
d-v-b c731cf2
fix: handle rectilinear chunks
d-v-b 9e25150
Merge branch 'main' of https://github.com/zarr-developers/zarr-python…
d-v-b ae0580c
fixup
d-v-b 0effe4d
feat: SupportsSetRange protocol + sync byte-range writes
d-v-b 7f45aba
feat: add sync codec methods to V2 and numcodecs codecs
d-v-b 9b26f90
feat: SyncCodecPipeline — synchronous codec pipeline with per-chunk p…
d-v-b a48f4f7
feat: partial-shard write support in ShardingCodec
d-v-b fba975e
test: codec invariants + pipeline parity matrix
d-v-b 1be5563
chore: gitignore local agent/planning notes
d-v-b 985716b
Merge branch 'main' into perf/prepared-write-v2
d-v-b 65d98a1
Merge branch 'main' into perf/prepared-write-v2
d-v-b 3f3e7ea
Merge branch 'perf/prepared-write-v2' of https://github.com/d-v-b/zar…
d-v-b 8b23d22
chore: remove unused PreparedWrite and SupportsChunkCodec
d-v-b cd3c14b
chore: remove stale phased-pipeline test files
d-v-b 88eac8f
refactor: rename SyncCodecPipeline to FusedCodecPipeline
d-v-b c3d11d0
refactor: lift _merge_chunk_array to module level
d-v-b 71f0d32
refactor: extract _async_read_fallback to module level
d-v-b f463035
refactor: extract _async_write_fallback to module level
d-v-b 621361a
test(bench): parametrize test_e2e over both codec pipelines
d-v-b fb812c4
test(bench): parametrize test_e2e over a synthetic latency dimension
d-v-b d2e08de
Merge branch 'main' of https://github.com/zarr-developers/zarr-python…
d-v-b 241282a
chore: restore deleted comments in V2Codec._decode_sync
d-v-b 0324a85
docs: explain non-obvious behaviors in sharding sync methods
d-v-b 2f97362
docs: convert RST inline literals to Markdown-style backticks
d-v-b c9c8c26
perf: memoize encoded inner chunk for scalar complete-shard writes (#…
d-v-b 5c536bb
merge: integrate upstream/main (subchunk write orders #3826, partial-…
d-v-b 3b4707a
perf+fix: bulk whole-shard read + repair _ShardIndex construction pos…
d-v-b 5ae0d88
fix: FusedCodecPipeline honors subchunk_write_order + coalesced parti…
d-v-b fed58c0
fix: address roborev review (job 222) — Fused sharding correctness
d-v-b 11bba96
Merge branch 'main' into perf/prepared-write-v2
d-v-b 7656383
docs: correct FusedCodecPipeline framing — sync scheduling, not IO/co…
d-v-b b7b9570
Merge remote-tracking branch 'origin/perf/prepared-write-v2' into per…
d-v-b 16c932d
refactor: stop hard-coding assumptions about the 'unordered' write order
d-v-b efb4b36
feat: make FusedCodecPipeline the default codec pipeline
d-v-b 0814ffd
fix: ShardingCodec inner pipeline follows the configured default, not…
d-v-b 071f87b
fix: Fused async decode/encode must evolve codec specs (HIGH-2) + sha…
d-v-b 6f12b42
test: dedupe codec-pipeline tests against the shared CodecPipelineTes…
d-v-b 7d606f2
test: unify the create/write/read suite tests into one Scenario-param…
d-v-b 4cc328a
test: prune test_fused_pipeline.py to its irreducible Fused-specific …
d-v-b 9acdb3a
test: dissolve test_codec_invariants.py, redistributing by subject
d-v-b 35255fa
test: drop redundant read-parity matrix, move partial-read coverage t…
d-v-b 9855060
test: rename test_sync_codec_pipeline -> test_chunk_transform; drop c…
d-v-b cccab40
feat: remove byte-range-write support pending store-interface decision
d-v-b 79e0896
refactor: remove dead _get_default_chunk_spec helper
d-v-b 100a69a
docs: correct ShardingCodec._encode_sync docstring re: write order
d-v-b 89a92c7
refactor: remove unused ShardingCodec._load_shard_index wrapper
d-v-b bba8382
docs: drop stale set_range_sync mention from FusedCodecPipeline docst…
d-v-b b8e8950
docs: use plain single backticks in docstrings, not RST double-backticks
d-v-b e5482f9
feat: default codec_pipeline.max_workers to 1 (sequential), threading…
d-v-b 558365f
Merge branch 'main' into perf/prepared-write-v2
d-v-b 298295f
perf: vectorize shard_dict build in _encode_partial_sync (fix write r…
d-v-b 98027cf
fix: thread spec forward in FusedCodecPipeline.evolve_from_array_spec
d-v-b e3d260e
refactor: extract shared pipeline logic into freestanding functions
d-v-b 9ca6932
Merge branch 'main' into perf/prepared-write-v2
d-v-b 3d2174d
test: cover the max_workers>1 thread-pool path and concurrent decode
d-v-b 7e9bbbb
test: add zarr v2 scenarios to the shared codec-pipeline suite
d-v-b 7578455
perf: encode the shard index once, via a shared layout helper
d-v-b a839a89
test: pin read_missing_chunks=False semantics on sharded arrays
d-v-b 62d0634
Ig/fused additions (#180)
d-v-b 3842691
perf: sync IO for sharding byte getters; evolve the nested inner pipe…
d-v-b 9348149
fix: root-cause the cross-file test flake (pytest-asyncio loop leak);…
d-v-b bda7c9f
refactor: dedupe sharding sync/async mirrors; fix un-threaded inner-c…
d-v-b ae5bca6
refactor: canonical chunk write-state functions; fix complete-chunk m…
d-v-b 48ab6be
Merge branch 'main' into perf/prepared-write-v2
d-v-b d609896
refactor: canonical chunk read functions (decode_and_scatter / scatte…
d-v-b 05eaf45
test: property tests asserting fast paths equal general paths
d-v-b 47ea356
fix: address open review findings across sharding, pipeline, and tests
d-v-b 2bf7679
Merge remote-tracking branch 'upstream/main' into perf/prepared-write-v2
d-v-b 72040f1
doc: add changelog entries for PR #3885
d-v-b 748f8dc
Merge branch 'main' into perf/prepared-write-v2
d-v-b e80d2e5
test: cover async fallback paths orphaned by the Fused default
d-v-b 0879b40
Merge branch 'main' into perf/prepared-write-v2
d-v-b fe70ba1
refactor: move reused functions around chunks into standalone file
ilan-gold c631d37
refactor: use more helpful name and remove comment
ilan-gold 310994b
refactor: move more
ilan-gold 7d9e3d7
refactor: move more
ilan-gold 83d2d85
Ig/as completed read (#194)
d-v-b 659502e
fix: bulk shard decode must not serve reordering reads in natural order
d-v-b a4d99a4
fix: make ChunkTransform spec cache torn-read-safe under threading
d-v-b 3f72c8d
fix: default Codec.is_fixed_size so sharded vlen/numcodecs reads don'…
d-v-b 4650564
refactor: drop unreachable None branch in _async_write_fallback
d-v-b 1ae5949
feat: keep BatchedCodecPipeline as the default; FusedCodecPipeline is…
d-v-b 38ee940
Merge remote-tracking branch 'upstream/main' into perf/prepared-write-v2
d-v-b 4e83284
test: remove commented-out byte-range-write fast-path tests
d-v-b 962b7c9
apply changes from code review
d-v-b e177f2a
refactor: drop dead sel_shape reshape in _decode_partial_sync bulk path
d-v-b 5665d38
docs: document the experimental FusedCodecPipeline
d-v-b 70b1bb5
Update docs/user-guide/experimental.md
d-v-b 6d84651
Update docs/user-guide/experimental.md
d-v-b cb32b0b
Update src/zarr/codecs/sharding.py
d-v-b 5c718f8
Update src/zarr/codecs/sharding.py
d-v-b ab3e17d
Update src/zarr/codecs/sharding.py
d-v-b fcd97d0
Merge branch 'main' into perf/prepared-write-v2
d-v-b 4c5549d
fix: syntax, missing import
d-v-b File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Added `SyncByteGetter` and `SyncByteSetter` runtime-checkable protocols and a `get_ranges_sync` method on the `Store` ABC. These let custom byte getters/setters opt into the synchronous codec pipeline's fast path for in-memory IO, which the sharding codec uses for its inner chunks. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Added `FusedCodecPipeline`, an opt-in codec pipeline that runs codec compute synchronously and in bulk (avoiding the per-chunk async scheduling overhead of the default `BatchedCodecPipeline`), giving large speedups for sharded arrays (up to ~24x writes / ~14x reads on many-chunks-per-shard layouts, more with compression) and no regressions on compute-bound workloads. The default pipeline is unchanged; enable the new one with `zarr.config.set({"codec_pipeline.path": "zarr.core.codec_pipeline.FusedCodecPipeline"})`. |
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
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could do some work on this. I'll have to check the benchmarks to understand more. But any guidance here would be appreciated