Skip to content

perf/chunkrequest#3730

Draft
d-v-b wants to merge 14 commits intozarr-developers:mainfrom
d-v-b:perf/chunkrequest
Draft

perf/chunkrequest#3730
d-v-b wants to merge 14 commits intozarr-developers:mainfrom
d-v-b:perf/chunkrequest

Conversation

@d-v-b
Copy link
Contributor

@d-v-b d-v-b commented Feb 27, 2026

  • simplifies logic related to is_complete_chunk
  • introduce ChunkRequest object for compacting chunk-aligned iteration used in chunk decoding / encoding

part of #3720

d-v-b and others added 5 commits February 25, 2026 18:33
Introduces CodecChain, a frozen dataclass that chains array-array,
array-bytes, and bytes-bytes codecs with synchronous encode/decode
methods. Pure compute only -- no IO, no threading, no batching.

Also adds sync roundtrip tests for individual codecs (blosc, gzip,
zstd, crc32c, bytes, transpose, vlen) and CodecChain integration tests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions github-actions bot added the needs release notes Automatically applied to PRs which haven't added release notes label Feb 27, 2026
d-v-b and others added 9 commits February 27, 2026 23:35
…ort strategy (zarr-developers#3718)

* tests: Add non-power-of-2 shard shapes to benchmarks

Add (30,30,30) to large_morton_shards and (10,10,10), (20,20,20),
(30,30,30) to morton_iter_shapes to benchmark the scalar fallback path
for non-power-of-2 shapes, which are not fully covered by the vectorized
hypercube path.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* tests: Add near-miss power-of-2 shape (33,33,33) to benchmarks

Documents the performance penalty when a shard shape is just above a
power-of-2 boundary, causing n_z to jump from 32,768 to 262,144.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* style: Apply ruff format to benchmark file

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* changes: Add changelog entry for PR zarr-developers#3717

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* perf: Fix near-miss penalty in _morton_order with hybrid ceiling+argsort strategy

For shapes just above a power-of-2 (e.g. (33,33,33)), the ceiling-only
approach generates n_z=262,144 Morton codes for only 35,937 valid
coordinates (7.3× overgeneration). The floor+scalar approach is even
worse since the scalar loop iterates n_z-n_floor times (229,376 for
(33,33,33)), not n_total-n_floor.

The fix: when n_z > 4*n_total, use an argsort strategy that enumerates
all n_total valid coordinates via meshgrid, encodes each to a Morton code
using vectorized bit manipulation, then sorts by Morton code. This avoids
the large overgeneration while remaining fully vectorized.

Result for test_morton_order_iter:
  (30,30,30): 24ms  (ceiling, ratio=1.21)
  (32,32,32): 28ms  (ceiling, ratio=1.00)
  (33,33,33): 32ms  (argsort, ratio=7.3 → fixed from ~820ms with scalar)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: Address pre-commit CI failures in _morton_order

- Replace Unicode multiplication sign × with ASCII x in comment (RUF003)
- Add explicit type annotation for np.argsort result to satisfy mypy

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: Cast argsort result via np.asarray to resolve mypy no-any-return

np.stack returns Any in mypy's view, so indexing into it also returns
Any. Using np.asarray(..., dtype=np.intp) makes the type explicit and
avoids the no-any-return error at the return site.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: Pre-declare order type to resolve mypy no-any-return in _morton_order

np.asarray and np.stack return Any with numpy 2.1 type stubs, causing
mypy to infer the return type as Any. Pre-declaring order as
npt.NDArray[np.intp] before the if/else makes the intended type explicit.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Davis Bennett <davis.v.bennett@gmail.com>
@d-v-b d-v-b force-pushed the perf/chunkrequest branch from 6518fd1 to 9473b2a Compare February 28, 2026 04:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs release notes Automatically applied to PRs which haven't added release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants