[0ax1]

Summary

#10716 copied cached file pages in 4 MiB chunks, alternating filling one
chunk's page tables with copying the previous one, and kept that size
independent of #10715, which grew staged file reads to 8 MiB chunks.
Both used FILE_READ_CHUNK_BYTES, so #10715 silently doubled the
mapped copy chunks as well, delaying the first copy of every mapped
read.

Give mapped copies their own 4 MiB chunk size.

Changes

• fix(cuda): restore 4 MiB chunks for mapped file copies

Performance

Intended improvement in latency to the first mapped copy, restoring the size #10716 tuned. The tradeoff is twice as many copy launches and page-table fill lock acquisitions per mapped read than with 8 MiB chunks.

Checks

Not compiled or tested locally. CI runs the checks.

[0ax1]

@claude review this pr in depth

[github-actions]

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

---

Review: restore 4 MiB chunks for mapped file copies

• Read diff
• Read surrounding code in file.rs, tests and kernels/src/mapped_copy.cu
• Check history (perf(cuda): read pooled files in 8 MiB chunks #10715 / perf(cuda): copy cached file pages directly on the GPU #10716). The checkout is shallow and gh pr diff wasn't permitted, so I couldn't confirm that perf(cuda): copy cached file pages directly on the GPU #10716 used 4 MiB. I took the PR description's word for it.
• Post review

Verdict: The fix is correct and small. I found no correctness problems. There's one test-coupling nit and a couple of optional suggestions.

Correctness

• Alignment holds. The kernel needs source and target to share the same offset within 16 bytes. Each chunk's source is start + chunk and its target is base + pad + chunk, with pad = start % 16. MAPPED_COPY_CHUNK_BYTES (4 MiB) is a multiple of 16, so every chunk keeps the same misalignment as the first one, just as it did with 8 MiB.
• Bounds hold. size = (length - chunk).min(MAPPED_COPY_CHUNK_BYTES) handles the tail exactly as before.
• The staged path is unchanged. read_at still uses FILE_READ_CHUNK_BYTES for the single-read threshold (file.rs:601) and the split reads (file.rs:608-609). Only MappedFile::copy_to_device uses the new constant, which is the right scope.
• The launch shape gets better. blocks = len.div_ceil(256 * 16) is exactly 1024 == MAPPED_COPY_BLOCKS_MAX for a full 4 MiB chunk. At 8 MiB the grid was clamped to 1024 blocks, so each thread ran two grid-stride iterations. At 4 MiB a full chunk gets exactly one 16-byte word per thread. This supports the change beyond first-copy latency, and the PR body could mention it.

Nits / suggestions

1. Test file size now depends on a different constant than one test that reads it (tests.rs:319 and tests.rs:558).
   MAPPED_FILE_BYTES dropped from 2 * 8 MiB + 4096 to 2 * 4 MiB + 4096 = 8 MiB + 4096. uncached_ranges_take_the_staged_path still reads (37, FILE_READ_CHUNK_BYTES + 5) from that same file, which ends at 8 MiB + 42. That fits today, but with only 4054 bytes to spare. If FILE_READ_CHUNK_BYTES grows again, which is what perf(cuda): read pooled files in 8 MiB chunks #10715 did, that test reads past EOF and fails for a reason unrelated to what it checks. Two ways to fix it:

   • size the file to cover both constants, e.g. 2 * max(FILE_READ_CHUNK_BYTES, MAPPED_COPY_CHUNK_BYTES) + 4096 as a const expression, or
   • add const _: () = assert!(37 + FILE_READ_CHUNK_BYTES + 5 <= MAPPED_FILE_BYTES); beside the test.

   Fix this →

2. Optionally make the chunk/grid relationship explicit. Point 4 under Correctness depends on MAPPED_COPY_CHUNK_BYTES == MAPPED_COPY_BLOCKS_MAX * MAPPED_COPY_THREADS * 16. If that's intended, a const _: () = assert!(...) or a line in the doc comment would stop a later edit from quietly bringing back the clamped multi-iteration grid. If it's a coincidence, skip this.

3. The doc comment could be clearer. "smaller chunks start the device sooner" is accurate. You could add that the first copy can't launch until the first chunk's PopulateRead returns under the global MAPPED_FILL lock, so chunk size directly sets first-copy latency and how long the lock is held per acquisition. Readers then see the lock-hold tradeoff the PR body mentions.

Tests

The mapped test cases now use MAPPED_COPY_CHUNK_BYTES, so chunk_aligned, chunks_with_tail, the concurrent reads and the read that crosses a chunk boundary (MAPPED_COPY_CHUNK_BYTES - 7) still exercise the boundaries of the new chunk size. That's the right update. The test file also shrinks from about 16 MiB to about 8 MiB, which makes these tests a bit faster.

Performance

The tradeoff in the PR body is reasonable: twice as many launches and lock acquisitions in exchange for an earlier first copy and shorter lock holds, which also helps fairness when several mapped readers compete for MAPPED_FILL. Nothing in the diff gives numbers. A before/after figure from a scan that is mostly mapped reads would back the changelog/performance label.

Checks

I only reviewed the code. I didn't compile or run anything, per the repo's verification guidance.

[codspeed]

Merging this PR will improve performance by 10.89%

1 warning

⚠️ Walltime results may be unreliable

Some benchmarks used the Walltime instrument outside CodSpeed Macro Runners, which can make the results inconsistent.

Learn more about Macro Runners

See affected benchmarks

⚡ 1 improved benchmark
✅ 2203 untouched benchmarks
⏩ 370 skipped benchmarks¹
🗄️ 30 archived benchmarks run²

Performance Changes

	Mode	Benchmark	BASE	HEAD	Efficiency
⚡	Walltime	threshold_avx2[80]	3 µs	2.7 µs	+10.89%

Tip

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

---

_{Comparing ad/cuda-mapped-copy-chunks (770c742) with develop (bdae6a7)}

Footnotes

1. 370 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. 30 benchmarks were run, but are now archived. If they were deleted in another branch, consider rebasing to remove them from the report. Instead if they were added back, click here to restore them. ↩