Skip to content

Make CPU input buffer size a setting, default 256 chunks - #168

Merged
nclack merged 5 commits into
fix/cpu-read-schedulingfrom
fix/cpu-read-buffer-capacity
Sep 24, 2026
Merged

nclack merged 5 commits into
fix/cpu-read-schedulingfrom
fix/cpu-read-buffer-capacity

Conversation

@nclack

@nclack nclack commented Sep 24, 2026 •

Copy link
Copy Markdown
Owner

CPU input buffers previously held one chunk per decode worker, limiting simultaneous file reads when each merged request contains several chunks. A new CPU executor setting, chunks_per_input_buffer (CpuLimits.chunks_per_input_buffer in Python, default 256), sets how many chunks each of the two input buffers holds. It must be at least decode_workers, so every worker can get a chunk, and at most 16384, the most chunks a batch can have. Values outside that range are rejected, with a message, when the executor is created. The worker pool and per-worker decoder workspaces stay the same.

This restores simultaneous reads. Each merged read still holds at most min(decode_workers, chunks_per_input_buffer) chunks, the rule used in the measurements below, so a buffer holds several reads at once. With 32 workers, reads are at most 32 chunks, so a full 256-chunk buffer splits into at least 8 reads. With one decode worker, reads do not merge. A separate read-size setting is left for a later PR.

This breaks existing budgets. The input reserve is 2 * chunks_per_input_buffer * max_encoded_chunk_bytes, plus per-chunk bookkeeping. With the defaults, 256 chunks and a 4 MiB encoded bound, that is 2 GiB before decoder workspaces and output buffers, so a 1 GiB CpuLimits budget that worked before now fails with BUDGET when the pipeline starts. Set max_encoded_chunk_bytes to the largest encoded chunk expected, lower chunks_per_input_buffer, or raise max_memory_bytes. When the budget is too small, starting now logs the bytes it tried to reserve for input buffers, decoder workspaces, output buffers, and the rest; a read plan that cannot fit logs its size too. Documentation shows the cost and raises the example budget to 3 GiB. The native pipeline fixture now uses a 1 KiB encoded bound appropriate for its small test chunks, retaining its 32 MiB budget. The throughput-cpu bench scenario no longer fit its 4 GiB budget with the 4 MiB bound; it now sets chunks_per_input_buffer to 256 and a 5 GiB budget. The bench reads the setting from a scenario's pipeline.chunks_per_input_buffer, default 256.

The motivating Turin experiment used 32 decode workers, 64 KiB chunks, 16 KiB Blosc blocks, and sixteen raw 1 GiB shards. In an isolated benchmark patch with persistent shard handles and default readahead, moving from 32 to 256 chunks improved cold translated 256×256 crop output by 47–55% and full-scan output by 71–97% across Blosc-LZ4 and Blosc-Zstd, adding 28.04 MiB of accounted memory at those 64 KiB chunk bounds, not the 2 GiB reserved with the default 4 MiB bound. These are benchmark-patch measurements; this PR does not include the persistent-handle or LZ4 additions and its final binary has not been benchmarked.

Regression coverage checks 513 chunks through three input groups of exactly 256, 256, and 1 chunks at 1, 2, and 32 workers; merged reads of at most decode_workers chunks, with exact read ranges, shard order, and reads per buffer at 1, 2, and 32 workers; complete output; exact memory-budget admission; oversized-allocation rejection; and rejection of chunks_per_input_buffer below decode_workers or above 16384, in C and in Python. Private read-plan tests check complete coverage at 256, 257, and 1024 workers without starting hundreds of threads. Formatting and whitespace checks pass, and the strict documentation build passes locally. At 18cee45, the CPU workflow built without CUDA and passed 24/24 CTest entries on Ubuntu and 25/25 on macOS (the native test executables plus the Python pytest suite), followed by the native dependency audit; the macOS job passed on a rerun after the known test_scheduler timing failure. The GPU-enabled regression workflow passed 36/36 CTest entries, the TSan-labeled check passed 1/1, and the build passed.

Stacked on #164; the base is fix/cpu-read-scheduling. #161 stacks on this branch.

@codecov

codecov Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.77778% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.82%. Comparing base (0dafafd) to head (07d503d).

Files with missing lines Patch % Lines
bench/main.c 0.00% 6 Missing ⚠️
src/executor/cpu_executor.c 88.57% 1 Missing and 3 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@                     Coverage Diff                     @@
##           fix/cpu-read-scheduling     #168      +/-   ##
===========================================================
+ Coverage                    59.71%   59.82%   +0.11%     
===========================================================
  Files                           75       75              
  Lines                        11474    11493      +19     
  Branches                      1984     1987       +3     
===========================================================
+ Hits                          6852     6876      +24     
- Misses                        3806     3807       +1     
+ Partials                       816      810       -6     
Flag Coverage Δ
unittests 59.82% <77.77%> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
python/damacy/__init__.py 96.04% <100.00%> (+0.02%) ⬆️
python/damacy/_components.c 0.00% <ø> (ø)
src/executor/cpu_executor.c 86.24% <88.57%> (+0.28%) ⬆️
bench/main.c 0.00% <0.00%> (ø)

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nclack
nclack force-pushed the fix/cpu-read-buffer-capacity branch from 8d0931e to 18cee45 Compare September 24, 2026 22:38
@nclack nclack changed the title Use 256 chunks per CPU input buffer Make CPU input buffer size a setting, default 256 chunks Sep 24, 2026
@nclack
nclack added this pull request to stack #170 September 24, 2026 23:07
@nclack
nclack force-pushed the fix/cpu-read-buffer-capacity branch from 18cee45 to 07d503d Compare September 24, 2026 23:13
@nclack
nclack merged commit 7ba9200 into main Sep 24, 2026
12 checks passed
@nclack
nclack deleted the fix/cpu-read-buffer-capacity branch September 24, 2026 23:34
nclack added a commit that referenced this pull request Sep 24, 2026
Translated crops in the CPU backend issue one read per chunk in query
order, bypassing the read merging and shard ordering used by CUDA. The
CPU executor now uses the same merger to combine adjacent or overlapping
ranges and distribute reads across shard files, while decoding each
distinct chunk once and assembling every requested crop.

The merger's read step is now a separate function. The CPU executor
calls it directly, so its temporary storage does not include CUDA's
per-chunk dispatch records. CUDA still goes through the existing
wrapper, and its behavior is unchanged.

Chunk offsets map into merged input ranges without changing the
decode-worker setting or the two bounded input buffers. Read-plan
storage and temporary sorting allocations respect the executor memory
limit; temporary pressure from an active batch is retried. Equal shard
paths can be separate strings, as they are in CPU prepared plans. The
shutdown fix, which drains active reads before releasing plans and their
shard paths, is part of #160, so this branch no longer carries its own
copy. #160 also adds `tests/test_cpu_executor.c`; this PR extends it.

On its own, this PR lowers the number of simultaneous reads when a shard
has at least `decode_workers` contiguous chunks. Each merged read then
fills an input buffer, so with the defaults 2 reads are in flight
instead of 16. #168 restores it: input buffers hold 256 chunks by
default, while merged reads still hold at most `decode_workers` chunks.
With 32 workers, reads are at most 32 chunks, so a full buffer splits
into at least 8 reads. Merge the two together.

The regression tests use a reader that records submissions to check
merged ranges and shard order, exact output and repeated-chunk reuse,
reader saturation, fill-only batches, fills sharing an input buffer with
merged reads, FIFO results, retained output, memory admission, and read
failure/shutdown. A new coalescer test, `test_equal_path_copies`, covers
equal paths with different pointers.

Validation at `9385079`:

- [CPU CI](https://github.com/nclack/damacy/actions/runs/36065679537):
24/24 CTest entries pass on Ubuntu and 25/25 on macOS, including the new
executor regressions; the extension has no CUDA dependencies.
- [CUDA-enabled
CI](https://github.com/nclack/damacy/actions/runs/36065682194): 36/36
CTest targets pass, plus the Python import check and coverage upload.
- [Build](https://github.com/nclack/damacy/actions/runs/36065677255) and
the [existing TSan
check](https://github.com/nclack/damacy/actions/runs/36065684441) pass.
The strict documentation build passes locally. Changed C files pass
formatting checks; `git diff --check` passes.

No new Turin throughput or storage measurements have been made. This
change addresses the scheduling behavior identified in #163; the
performance effect and suspected kernel readahead still need
measurement.

Stacked on #167; the base is `platform/macos`. #168 stacks on this
branch.

Closes #163.

---------

Co-authored-by: Nathan Clack <nclack@biohub.org>
nclack added a commit that referenced this pull request Sep 24, 2026
Add `IndexQuery` to CPU and CUDA pipelines, combining ordered index
arrays with contiguous slices. Axes form a Cartesian product, preserving
order, duplicates, and singleton dimensions. Rectangular and indexed
requests can share a batch with the same output shape.

Stacked on #168; the base branch is `fix/cpu-read-buffer-capacity`. On
this base the CPU executor keeps the merged reads from #164 and the
input buffers from #168; index gathering happens during assembly, after
decoding, so read planning is unchanged. #162 stacks on this branch.

```python
query = damacy.IndexQuery(
    "image.zarr/0",
    selection=([7, 2, 7], slice(16, 80), [100, 3, 40, 3]),
)
assert query.shape == (3, 64, 4)
```

Preparation copies index vectors, enumerates only touched shards and
chunks, and publishes owned gather plans. Index storage grows with
vector lengths, without a planning record for every output voxel. CPU
assembly and CUDA dispatch consume those same plans. CUDA keeps each
chunk's selected range in a separate per-batch array that only indexed
samples use, so rectangular chunk records keep their size.
`PlanLimits.max_plan_bytes` includes owned index data.

`CudaLimits.max_index_bytes` sets device index storage within the total
GPU budget. It must be zero or hold every index a batch can contain, 8
bytes × samples × the sum of the output shape, so an accepted query
never runs out of room. A smaller nonzero value raises `BudgetExceeded`
when the pipeline is created. With zero, `push` raises `BudgetExceeded`
for an `IndexQuery`; the query is not consumed and the pipeline keeps
running. Out-of-bounds coordinates are found when metadata arrives and
raise `InvalidArgument` from `pop`, which stops the pipeline.

The C API uses `damacy_sample.rank` and tagged `axes`. Each active axis
explicitly selects `DAMACY_AXIS_INTERVAL` with `.interval` or
`DAMACY_AXIS_INDICES` with `.indices`. Zero and unknown tags, invalid
intervals, and null or empty index arrays fail validation. Preparation
derives the bounding AABB. `damacy_tuning` gains `max_index_bytes`;
`damacy_tuning_defaults` sets it to 64 MiB, and a zeroed tuning rejects
indexed samples. C callers must rebuild and migrate their query
initializers and tuning setup. Negative indices are rejected at push and
out-of-bounds coordinates at pop; interpolation and NGFF level selection
remain separate future work.

CUDA decode-gap timing skips the first wave until a previous decode
event has been recorded, avoiding an invalid event-handle call.

Validation:

- CI at `61ce41c`: the [CPU
workflow](https://github.com/nclack/damacy/actions/runs/36066071759)
passes 25/25 CTest entries on Ubuntu and 26/26 on macOS; the macOS job
passed on a rerun after the known `test_scheduler` timing failure.
[CUDA](https://github.com/nclack/damacy/actions/runs/36066074277) passes
37/37 with the coverage upload,
[TSan](https://github.com/nclack/damacy/actions/runs/36066076482)
passes, and the
[build](https://github.com/nclack/damacy/actions/runs/36066069451)
passes.
- Ruff, Pyright, and the strict documentation build pass.
- Before the review fixes, CPU-only: 24/24 CTest targets, including 70
Python tests.
- Before the review fixes, AddressSanitizer/UndefinedBehaviorSanitizer:
5/5 targeted CTest targets covering selections, owned plans, lookahead,
prefetch, and CPU execution.
- Before the review fixes, CUDA on L40: all 36 CTest targets passed,
including 197 Python tests.
- Before the review fixes, CUDA compute-sanitizer: 35 query and
malformed-input tests, zero memory or API errors.

closes #138

---------

Co-authored-by: Nathan Clack <nclack@biohub.org>
github-actions Bot added a commit that referenced this pull request Sep 24, 2026
CPU input buffers previously held one chunk per decode worker, limiting
simultaneous file reads when each merged request contains several
chunks. A new CPU executor setting, `chunks_per_input_buffer`
(`CpuLimits.chunks_per_input_buffer` in Python, default 256), sets how
many chunks each of the two input buffers holds. It must be at least
`decode_workers`, so every worker can get a chunk, and at most 16384,
the most chunks a batch can have. Values outside that range are
rejected, with a message, when the executor is created. The worker pool
and per-worker decoder workspaces stay the same.

This restores simultaneous reads. Each merged read still holds at most
`min(decode_workers, chunks_per_input_buffer)` chunks, the rule used in
the measurements below, so a buffer holds several reads at once. With 32
workers, reads are at most 32 chunks, so a full 256-chunk buffer splits
into at least 8 reads. With one decode worker, reads do not merge. A
separate read-size setting is left for a later PR.

This breaks existing budgets. The input reserve is `2 *
chunks_per_input_buffer * max_encoded_chunk_bytes`, plus per-chunk
bookkeeping. With the defaults, 256 chunks and a 4 MiB encoded bound,
that is 2 GiB before decoder workspaces and output buffers, so a 1 GiB
`CpuLimits` budget that worked before now fails with `BUDGET` when the
pipeline starts. Set `max_encoded_chunk_bytes` to the largest encoded
chunk expected, lower `chunks_per_input_buffer`, or raise
`max_memory_bytes`. When the budget is too small, starting now logs the
bytes it tried to reserve for input buffers, decoder workspaces, output
buffers, and the rest; a read plan that cannot fit logs its size too.
Documentation shows the cost and raises the example budget to 3 GiB. The
native pipeline fixture now uses a 1 KiB encoded bound appropriate for
its small test chunks, retaining its 32 MiB budget. The `throughput-cpu`
bench scenario no longer fit its 4 GiB budget with the 4 MiB bound; it
now sets `chunks_per_input_buffer` to 256 and a 5 GiB budget. The bench
reads the setting from a scenario's `pipeline.chunks_per_input_buffer`,
default 256.

The motivating Turin experiment used 32 decode workers, 64 KiB chunks,
16 KiB Blosc blocks, and sixteen raw 1 GiB shards. In an isolated
benchmark patch with persistent shard handles and default readahead,
moving from 32 to 256 chunks improved cold translated 256×256 crop
output by 47–55% and full-scan output by 71–97% across Blosc-LZ4 and
Blosc-Zstd, adding 28.04 MiB of accounted memory at those 64 KiB chunk
bounds, not the 2 GiB reserved with the default 4 MiB bound. These are
benchmark-patch measurements; this PR does not include the
persistent-handle or LZ4 additions and its final binary has not been
benchmarked.

Regression coverage checks 513 chunks through three input groups of
exactly 256, 256, and 1 chunks at 1, 2, and 32 workers; merged reads of
at most `decode_workers` chunks, with exact read ranges, shard order,
and reads per buffer at 1, 2, and 32 workers; complete output; exact
memory-budget admission; oversized-allocation rejection; and rejection
of `chunks_per_input_buffer` below `decode_workers` or above 16384, in C
and in Python. Private read-plan tests check complete coverage at 256,
257, and 1024 workers without starting hundreds of threads. Formatting
and whitespace checks pass, and the strict documentation build passes
locally. At `18cee45`, the [CPU
workflow](https://github.com/nclack/damacy/actions/runs/36065751602)
built without CUDA and passed 24/24 CTest entries on Ubuntu and 25/25 on
macOS (the native test executables plus the Python pytest suite),
followed by the native dependency audit; the macOS job passed on a rerun
after the known `test_scheduler` timing failure. The [GPU-enabled
regression
workflow](https://github.com/nclack/damacy/actions/runs/36065753652)
passed 36/36 CTest entries, the [TSan-labeled
check](https://github.com/nclack/damacy/actions/runs/36065755729) passed
1/1, and the
[build](https://github.com/nclack/damacy/actions/runs/36065749180)
passed.

Stacked on #164; the base is `fix/cpu-read-scheduling`. #161 stacks on
this branch.

---------

Co-authored-by: Nathan Clack <nclack@biohub.org> 7ba9200
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant