Skip to content

perf: cut the fill_value check on the chunk write path - #4382

Merged
d-v-b merged 4 commits into
zarr-developers:mainfrom
PursuitOfDataScience:perf/all-equal-write-path
Sep 23, 2026
Merged

d-v-b merged 4 commits into
zarr-developers:mainfrom
PursuitOfDataScience:perf/all-equal-write-path

Conversation

@PursuitOfDataScience

@PursuitOfDataScience PursuitOfDataScience commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Before zarr writes a chunk it asks one question: is this chunk entirely fill_value?
If it is, the chunk is skipped and no file is written.

That check is NDBuffer.all_equal. It runs on every chunk write, because
write_empty_chunks=False is the default. On an uncompressed float32 write it was
62.6% of the total time.

This PR makes the check cheap. Behaviour is unchanged.

Chunk write speedup by calling path: zarr whole-array write 2.16x, zarr per-chunk writes 1.74x, xarray.to_zarr 1.63x, xarray.to_zarr with dask 1.45x

Two things were slow

They are independent, and each one helps a different set of users.

1. The comparison used a dtype NumPy has no fast path for

When fill_value is zero the check compares raw bytes, not values, so that -0.0 is
not mistaken for 0.0 (#3144). It did that by viewing both sides as V{itemsize}, a
void dtype.

NumPy has vectorised comparison loops for integers. It has none for void, so the void
view quietly falls back to a generic element-by-element path.

An unsigned integer of the same width compares exactly the same bits and does have a
fast path. On a 16 MiB all-zero buffer that is 27x faster for float32 and 33x for
float64 on numpy 2.5.3, and 23x and 17x on numpy 2.2.6. Widths with no same-size
integer, longdouble and complex128, keep the void path.

This one helps zarr's own default, where fill_value is 0.0.

2. The scan never stopped early

If the first element already differs from fill_value, the answer is known. The old
code read the whole chunk anyway, because np.array_equal builds a full boolean array
before reducing it. On the write path "not uniform" is the common case, so that cost
was paid nearly every time.

It now walks slices of the leading axis, doubling in length, and returns on the first
slice that differs. A chunk that really is uniform costs the same as before.

Why the leading axis rather than a flattened view: a chunk is normally a strided
view into a larger array, so flattening it copies. An earlier version of this patch
required a contiguous buffer, which meant the early exit never actually ran and
measured exactly 1.00x end to end. Slicing axis 0 is a view whatever the layout.
test_all_equal_strided_chunk_view pins this.

This one helps xarray, which writes fill_value=NaN and so never reached the first
problem at all.

Results

Paired and interleaved with alternating arm order, bootstrap 90% CI on the median
ratio, Wilcoxon signed-rank on the paired times. AMD EPYC 7702, numpy 2.5.3, 256 MiB
float32, uncompressed. Every point below is significant at p < 1e-11.

Speedup across chunk size and data shape on two filesystems: from 1.56x for 1 MiB random chunks up to 13.1x for 16 MiB all-fill_value chunks

The worst single pair anywhere in the run was 1.05x, so no configuration regressed. The
win also survives the default zstd compressor at 2.0x to 3.2x, so it is not an artifact
of testing uncompressed.

Correctness

3,622 differential comparisons against the old implementation, 0 mismatches. They
cover 13 dtypes, C, F and strided layouts, -0.0, NaN, inf, mismatches at the first,
middle and last element, block boundaries, and the longdouble and complex128 fallbacks.

New tests cover agreement with a whole-buffer scan, -0.0 staying distinct (#3144),
NaN, non-contiguous buffers, strided chunk views, and shape[0] == 1, where there is
no leading axis to slice and the code must fall back.

For reviewers

This is purely a performance change; behaviour is meant to be identical. Two places I
would most value a second opinion:

  1. Is the unsigned integer view bit-identical to the void view for every dtype it
    accepts?
    This is the part that could change results rather than only timings. It
    is gated on itemsize (1, 2, 4, 8) with everything else left on the old path, and the
    differential test covers -0.0, NaN and inf, but a second read is worth it.
  2. Is slicing the leading axis the right shape for the early exit? Flattening would
    read more simply, but copies on the strided views this method actually receives.

I am already confident in the shape[0] == 1 and empty-array fallbacks, both covered by
tests.

Author attestation

  • I am a human, these are my changes, and I have reviewed and understood every change and can explain why each is correct.

TODO

  • Add unit tests and/or doctests in docstrings
  • Add docstrings and API docs for any new/modified user-facing classes and functions
  • New/modified features documented in docs/user-guide/*.md
  • Changes documented as a new file in changes/
  • GitHub Actions have all passed
  • Test coverage is 100% (Codecov passes)

`NDBuffer.all_equal` runs on every chunk write, because that is how
`write_empty_chunks=False` (the default) decides a chunk is uniformly
`fill_value` and can be skipped. Instrumented on an uncompressed float32
write it was 62.6% of total write time. Two independent causes.

The bit-pattern comparison for a zero `fill_value`, which exists so that
`-0.0` stays distinct from `0.0` (zarr-developers#3144), viewed both sides as `V{itemsize}`.
NumPy has vectorised integer comparison loops and no void one, so that falls
back to a generic elementwise path: 27x slower than the equivalent unsigned
integer view on float32 and 33x on float64, measured on numpy 2.5.3. Viewing
as an unsigned integer of the same width is equally bit-exact. Widths with no
same-size integer, longdouble and complex128, keep the void form.

The scan also had no early exit, so a buffer that is not uniformly
`fill_value` still had to be read in full before `np.array_equal` could
report the first mismatch, which on the write path is the common case. It now
walks slices of the leading axis, doubling in length, and returns on the first
slice that differs. The leading axis is used rather than a flattened view
because a chunk is normally a strided view into a larger array and cannot be
flattened without a copy; gating the early exit on contiguity makes it
silently never engage. A buffer that really is uniform is still covered in the
same number of element comparisons.

Both `fill_value` regimes benefit, for different reasons: zarr's own default
is `0.0` and takes the first change, while xarray writes `fill_value=NaN` and
takes only the second.

Behaviour is unchanged, including for `-0.0`, NaN, and non-contiguous buffers.

Assisted-by: ClaudeCode:claude-opus-5
Assisted-by: ClaudeCode:claude-opus-5
@codecov

codecov Bot commented Sep 20, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.29630% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 94.23%. Comparing base (c2451b3) to head (2806597).

Files with missing lines Patch % Lines
src/zarr/core/buffer/core.py 96.29% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4382   +/-   ##
=======================================
  Coverage   94.22%   94.23%           
=======================================
  Files          92       92           
  Lines       12942    12961   +19     
=======================================
+ Hits        12195    12214   +19     
  Misses        747      747           
Files with missing lines Coverage Δ
src/zarr/core/buffer/core.py 81.70% <96.29%> (+2.39%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Assisted-by: ClaudeCode:claude-opus-5
@d-v-b d-v-b added the benchmark Code will be benchmarked in a CI job. label Sep 23, 2026
# Handle None fill_value for Zarr V2
return False
@staticmethod
def _compare_all(data: NDArrayLike, other: Any, equal_nan: bool) -> bool:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of a static method we should have a free function that gets used in the body of a plain method

@d-v-b

d-v-b commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

this looks good, feel free to ignore my request about the static method thing because I can fix that in a follow-up

@codspeed

codspeed Bot commented Sep 23, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 15.33%

⚡ 12 improved benchmarks
✅ 101 untouched benchmarks
⏩ 37 skipped benchmarks1

Performance Changes

Benchmark BASE HEAD Efficiency
⚡ test_write_array[latency=0-batched-local-Layout(shape=(1000000000,), chunks=(100000,), shards=(10000000,))-None-semi_random] 6.7 s 5.5 s +21.38%
⚡ test_write_array[latency=0.03-batched-memory-Layout(shape=(1000000000,), chunks=(100000,), shards=(10000000,))-None-repeated] 7.5 s 6.2 s +20.75%
⚡ test_write_array[latency=0.03-batched-memory-Layout(shape=(1000000000,), chunks=(100000,), shards=(10000000,))-None-semi_random] 7 s 5.8 s +19.64%
⚡ test_write_array[latency=0.03-batched-memory-Layout(shape=(100000000,), chunks=(100000,), shards=(100000000,))-None-repeated] 873.3 ms 730.9 ms +19.49%
⚡ test_write_array[latency=0-batched-local-Layout(shape=(1000000000,), chunks=(100000,), shards=(10000000,))-None-repeated] 7.4 s 6.4 s +14.94%
⚡ test_write_array[latency=0.03-batched-memory-Layout(shape=(100000000,), chunks=(100000,), shards=(100000000,))-zstd-semi_random] 1.2 s 1.1 s +14.1%
⚡ test_write_array[latency=0-batched-local-Layout(shape=(100000000,), chunks=(100000,), shards=(100000000,))-None-repeated] 1,111.4 ms 975.9 ms +13.88%
⚡ test_write_array[latency=0-batched-local-Layout(shape=(100000000,), chunks=(100000,), shards=(100000000,))-None-semi_random] 952.9 ms 837.9 ms +13.73%
⚡ test_write_array[latency=0.03-batched-memory-Layout(shape=(1000000000,), chunks=(100000,), shards=(10000000,))-zstd-repeated] 10.1 s 9 s +12.99%
⚡ test_write_array[latency=0.03-batched-memory-Layout(shape=(100000000,), chunks=(100000,), shards=(100000000,))-None-semi_random] 806.6 ms 717.5 ms +12.43%
⚡ test_write_array[latency=0-batched-local-Layout(shape=(100000000,), chunks=(100000,), shards=(100000000,))-zstd-repeated] 1,066.7 ms 959.2 ms +11.21%
⚡ test_write_array[latency=0.03-batched-memory-Layout(shape=(1000000000,), chunks=(100000,), shards=(10000000,))-zstd-semi_random] 11.8 s 10.8 s +10.09%

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.


Comparing PursuitOfDataScience:perf/all-equal-write-path (ce1c66b) with main (7291469)2

Open in CodSpeed

Footnotes

  1. 37 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

  2. No successful run was found on main (c2451b3) during the generation of this report, so 7291469 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

@d-v-b
d-v-b merged commit f3c661c into zarr-developers:main Sep 23, 2026
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

benchmark Code will be benchmarked in a CI job.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants