Make CPU input buffer size a setting, default 256 chunks - #168
Merged
Merged
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ 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
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
nclack
force-pushed
the
fix/cpu-read-buffer-capacity
branch
from
September 24, 2026 22:38
8d0931e to
18cee45
Compare
This was referenced Sep 24, 2026
Merged
nclack
added this pull request to stack #170
September 24, 2026 23:07
nclack
force-pushed
the
fix/cpu-read-buffer-capacity
branch
from
September 24, 2026 23:13
18cee45 to
07d503d
Compare
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
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.
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_bufferin Python, default 256), sets how many chunks each of the two input buffers holds. It must be at leastdecode_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 GiBCpuLimitsbudget that worked before now fails withBUDGETwhen the pipeline starts. Setmax_encoded_chunk_bytesto the largest encoded chunk expected, lowerchunks_per_input_buffer, or raisemax_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. Thethroughput-cpubench scenario no longer fit its 4 GiB budget with the 4 MiB bound; it now setschunks_per_input_bufferto 256 and a 5 GiB budget. The bench reads the setting from a scenario'spipeline.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_workerschunks, 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 ofchunks_per_input_bufferbelowdecode_workersor 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. At18cee45, 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 knowntest_schedulertiming 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.