feat(cuda): expose row block and scan batch size controls

[0ax1]

No description provided.

[0ax1]

@claude review

[github-actions]

Claude finished @0ax1's task in 1m 57s —— View job

---

PR Review: perf(cuda): support aligned scan batch rows

• Gather context
• Review vortex-file/src/strategy.rs
• Review vortex-cuda/ffi/src/lib.rs
• Review vortex-cuda/ffi/cinclude/vortex_cuda.h
• Post review feedback

Reviewed the single commit (4574925) against develop. This is a clean, well-scoped change that exposes two new knobs through the CUDA C API so on-disk row blocks can be aligned with scan batch sizes. Overall it looks correct and backwards-compatible. A few notes below.

---

Correctness

Backwards compatibility is preserved. The old entry points (vx_cuda_array_sink_open_file, vx_cuda_scan_path_arrow_device_stream) now delegate to the new _with_* variants passing 0, and 0 short-circuits to the previous behavior in both cases. The default WriteStrategyBuilder still sets data_block_size_target: Some(ONE_MEG), so block_size_minimum/block_size_target are identical to before for existing callers. 👍

SplitBy::RowCount(0) panic is correctly avoided. SplitBy::RowCount(n) is implemented with step_by(n) (vortex-layout/src/scan/split_by.rs:61), which panics if n == 0. The FFI guards this with the batch_size == 0 check that falls back to SplitBy::Layout, so a zero batch size can never reach step_by. Good — but this invariant is load-bearing and implicit; a one-line comment noting that 0 must be filtered before constructing RowCount would protect it against future refactors.

Design / API surface

The write-side coupling is undocumented and slightly surprising. In vx_cuda_array_sink_open_file_with_row_block_size, passing a nonzero row_block_size also disables byte-size coalescing (with_data_block_size_target(None)):

if row_block_size > 0 {
    strategy = strategy
        .with_row_block_size(row_block_size)
        .with_data_block_size_target(None);
}

This means calling with row_block_size = 8192 (the default value) is not equivalent to calling with 0 — the former disables the 1 MiB coalescing while the latter keeps it. The header doc only says "Pass zero to use the default writer strategy," which doesn't convey that any nonzero value silently changes a second, unrelated knob. Worth calling this out explicitly in the C header doc so callers understand the two behaviors are coupled. Fix this →

No enforced relationship between write row_block_size and scan batch_size. The PR title/description is about aligning on-disk row blocks with scan batches, but the write granularity and the scan batch size are two independent parameters on two independent FFI calls. Alignment is entirely the caller's responsibility (they must pass matching values, and read back with a matching batch_size). That's a reasonable choice for a low-level C API, but it's worth a sentence in the header docs pointing callers at the companion function so the intended pairing is discoverable.

Tests

No tests were added. The FFI wrappers are thin and awkward to test directly, but the strategy.rs change is easily unit-testable in vortex-file: a test asserting that with_data_block_size_target(None) yields blocks at row_block_size row granularity (vs. coalesced 1 MiB blocks with the default) would lock in the core behavior this PR depends on, and guard the coupling from regressing. Per CLAUDE.md, tests are strongly encouraged for behavior changes like this.

Minor / nits

• vortex_cuda.h: adding #include <stddef.h> for size_t is the right call. 👍
• The new scan function doc says "Scan a local Vortex file"; the sibling function and title emphasize "CUDA-compatible" — minor wording inconsistency.
• Rust doc comments on the new functions are thorough and include proper # Safety sections. 👍

---

Summary: Correct, backwards-compatible, and cleanly implemented. The main actionable items are (1) documenting the row_block_size ⇒ disables coalescing coupling in the C header, and (2) adding a small strategy.rs unit test for with_data_block_size_target(None). Neither is blocking.
• Branch: ad/batch-rows-pr

[codspeed-hq]

Merging this PR will not alter performance

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

✅ 1892 untouched benchmarks
⏩ 3 skipped benchmarks¹

---

_{Comparing ad/batch-rows-pr (a5a12f3) with develop (3048533)}

Footnotes

1. 3 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. ↩