Skip to content

Add per-reader readahead control - #172

Merged
nclack merged 4 commits into
mainfrom
feat/file-reader-readahead
Oct 6, 2026
Merged

nclack merged 4 commits into
mainfrom
feat/file-reader-readahead

Conversation

@nclack

@nclack nclack commented Sep 28, 2026 •

Copy link
Copy Markdown
Owner

Scattered crops can benefit from disabling OS file readahead, but scans can slow down substantially. Add FileReader(readahead=False) so applications can choose per reader. The default remains True, preserving existing behavior.

The option applies to buffered bulk reads with both CPU and CUDA executors. Linux uses POSIX_FADV_RANDOM; macOS uses F_RDAHEAD=0. Each newly opened chunk or shard gets the setting, including after cache eviction. A rejected OS request fails the read with an I/O error. Metadata readers remain independent; this option does not clear or bypass the page cache.

C callers get an additive damacy_file_reader_create_with_config constructor; the existing constructor and legacy Config adapter retain OS-default readahead.

Validation:

  • Linux CPU: 27/27 CTest checks passed; macOS: 28/28; CUDA: 39/39. Each suite includes Python tests.
  • Coverage includes per-reader isolation, default C constructor compatibility, cached and reopened files, failed advice and retry, argument validation, and CPU/CUDA output correctness with readahead on and off.
  • Release build, documentation build, ThreadSanitizer, and coverage checks passed.
  • Python lint/format checks and git diff --check passed.

@codecov

codecov Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.75758% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.11%. Comparing base (8582377) to head (26de64a).

Files with missing lines Patch % Lines
python/damacy/_components.c 0.00% 5 Missing ⚠️
src/executor/cuda_executor.c 0.00% 2 Missing ⚠️
src/pipeline/components.c 91.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #172      +/-   ##
==========================================
+ Coverage   60.03%   60.11%   +0.08%     
==========================================
  Files          82       82              
  Lines       13060    13082      +22     
  Branches     2331     2334       +3     
==========================================
+ Hits         7840     7864      +24     
- Misses       4280     4281       +1     
+ Partials      940      937       -3     
Flag Coverage Δ
unittests 60.11% <75.75%> (+0.08%) ⬆️

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.26% <100.00%> (+0.01%) ⬆️
src/platform/platform_io.posix.c 60.21% <100.00%> (+1.78%) ⬆️
src/store/store_fs.c 77.51% <100.00%> (+0.19%) ⬆️
src/pipeline/components.c 73.57% <91.66%> (+3.64%) ⬆️
src/executor/cuda_executor.c 77.61% <0.00%> (ø)
python/damacy/_components.c 0.00% <0.00%> (ø)

... and 4 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 marked this pull request as ready for review September 28, 2026 18:24
nclack and others added 3 commits October 3, 2026 17:33
Rename the C flag to enable_readahead and apply it to GDS file descriptors before cuFile registration. Cover reader-to-executor propagation, cached and reopened files, and failed advice cleanup and retry in forced compatibility mode.
Expose CMAKE_CUDA_ARCHITECTURES as a Docker build argument and pass it to both the native and editable Python builds. Select 120-real for the RTX 5070 Laptop GPU on auk while retaining the portable image default.

Validated a Debug+coverage image on auk: native build 21.3s, editable install 23.9s, all 39 CTest tests passed, Python import smoke passed, and both assembly objects contain sm_120 code. Workflow lint and git diff --check passed.
Wait for callback progress on an independent condition variable and establish waiter readiness under the scheduler lock. Keep a 30-second CTest watchdog for deadlocks.

Validated 100 repeated scheduler runs, delayed callbacks that fail the old test and pass the new test, and a stalled callback that fails through the watchdog.
@nclack
nclack merged commit 3ccabe4 into main Oct 6, 2026
8 checks passed
@nclack
nclack deleted the feat/file-reader-readahead branch October 6, 2026 21:34
github-actions Bot added a commit that referenced this pull request Oct 6, 2026
Scattered crops can benefit from disabling OS file readahead, but scans
can slow down substantially. Add `FileReader(readahead=False)` so
applications can choose per reader. The default remains `True`,
preserving existing behavior.

The option applies to buffered bulk reads with both CPU and CUDA
executors. Linux uses `POSIX_FADV_RANDOM`; macOS uses `F_RDAHEAD=0`.
Each newly opened chunk or shard gets the setting, including after cache
eviction. A rejected OS request fails the read with an I/O error.
Metadata readers remain independent; this option does not clear or
bypass the page cache.

C callers get an additive `damacy_file_reader_create_with_config`
constructor; the existing constructor and legacy `Config` adapter retain
OS-default readahead.

Validation:
- Linux CPU: 27/27 CTest checks passed; macOS: 28/28; CUDA: 39/39. Each
suite includes Python tests.
- Coverage includes per-reader isolation, default C constructor
compatibility, cached and reopened files, failed advice and retry,
argument validation, and CPU/CUDA output correctness with readahead on
and off.
- Release build, documentation build, ThreadSanitizer, and coverage
checks passed.
- Python lint/format checks and `git diff --check` passed.

---------

Co-authored-by: Nathan Clack <nclack@biohub.org> 3ccabe4
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