fix(cuda): reach pinned-DMA bandwidth for large host-to-device uploads#1404
Open
matteospanio wants to merge 2 commits into
Open
fix(cuda): reach pinned-DMA bandwidth for large host-to-device uploads#1404matteospanio wants to merge 2 commits into
matteospanio wants to merge 2 commits into
Conversation
Three related changes on the upload path: 1. write_to_gpu: contiguous transfers >= 100 MB from unpinned host memory were handed to memcpy_htod_async from pageable memory, which the driver stages internally and serially at a fraction of pinned DMA bandwidth. They are now pipelined through two pooled 8 MB pinned staging chunks, each chunk's host memcpy overlapping the previous chunk's DMA (falls back to the direct path when the pinned pool cannot serve the chunks). Behavior for < 100 MB and already-pinned buffers is unchanged. 2. create_from_slices: every slice was re-copied into a fresh Vec before being wrapped into Bytes, although it is already an owned Vec at that point - a full-size host copy per upload. 3. create_with_data: route the staging allocation through reserve_cpu so the existing 100 MB pinned-pool size policy applies instead of page-locking a full-transfer-size staging buffer. Measured on an RTX 3070 (PCIe 3.0), 268 MB f32 upload (cubecl 0.10.0 + this patch, backported): - create(Bytes::from_shared(..)) + sync: 298 ms -> 42.3 ms (0.9 -> 6.35 GB/s, now symmetric with the 6.5 GB/s read path). - Byte-exact roundtrips verified at 4 KB, 8 MiB, 8 MiB + 1, 16 MiB - 3, 100 MiB + 4 KiB + 1, and 268 MB, via both create_from_slice and create(Bytes::from_shared(..)).
Member
|
I'm not sure the current fix is the right one. We already haven a staging utility exposed from the client, I think we should use that instead, since that will improve performance across all backends. |
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.
Problem
Large uploads (
create_from_slice/create) reach only a fraction of the device's transfer bandwidth. On an RTX 3070 (PCIe 3.0), a 268 MB f32 upload moves at ~0.9 GB/s, while the read path moves 6.5 GB/s on the same box — a 7× H2D/D2H asymmetry.Cause: in
write_to_gpu, contiguous transfers ≥ 100 MB from unpinned host memory go straight tomemcpy_htod_asyncfrom pageable memory, which the CUDA driver stages internally and serially. Two smaller costs compound it:create_from_slicesre-copies its (already owned)Vecinto a freshVecbefore wrapping it intoBytes, andcreate_with_datapage-locks a full-transfer-size staging buffer, bypassing the 100 MB size policy thatreserve_cpualready encodes.Fix
write_to_gpu: large contiguous transfers from unpinned memory are pipelined through two pooled 8 MB pinned staging chunks — each chunk's host memcpy overlaps the previous chunk's DMA. Falls back to the direct path when the pinned pool cannot serve the chunks. Behavior for < 100 MB and already-pinned buffers is unchanged.create_from_slices: drop the redundant full-size host copy (the slice data is already an ownedVecat that point).create_with_data: route staging throughreserve_cpuso the existing size policy applies.Measurements (RTX 3070, PCIe 3.0, 268 MB f32)
create(Bytes::from_shared(..))+ syncMeasured with the same patch backported onto v0.10.0 (identical code in this area). Byte-exact roundtrips verified at 4 KB, 8 MiB, 8 MiB+1, 16 MiB−3, 100 MiB+4 KiB+1, and 268 MB via both
create_from_sliceandcreate(Bytes::from_shared(..)); also validated by a downstream GPU DSP test suite (bit-parity of kernel results vs CPU reference).🤖 Generated with Claude Code