Load CUDA external data through pinned buffers - #32437
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Error paths can leave DMA in flight, and setup failures regress previously valid transfers.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds pinned-buffer staging to accelerate large synchronous pageable host-to-CUDA transfers.
Changes:
- Alternates 64 MiB chunks across two CUDA streams.
- Retains per-device staging resources.
- Adds a cold model-loading benchmark.
File summaries
| File | Description |
|---|---|
gpu_data_transfer.cc |
Implements staged CUDA transfers. |
gpu_data_transfer.h |
Declares staging state and synchronization. |
benchmark_cuda_model_loading.py |
Benchmarks CUDA session creation. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 5
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Controlled benchmark correction. The previous 10.63/7.07-second results mixed another PR, and the PyPI comparison used a different release. The final comparison uses the merge-base Model:
The PR reduces complete CUDA session creation by 14.2% cold and 12.1% warm. Direct sequential read throughput was 6.8 GB/s. CPU control, using matching CPU builds: base 463.86 s cold / 468.66 s warm; PR 464.51 s cold / 466.23 s warm. The differences (+0.14% cold, -0.52% warm) are measurement noise, as expected because CPU-only sessions do not register the CUDA loader. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…perf/cuda-pinned-initializer-staging
Make LoadTensor pure virtual so shared execution providers emit the interface type information they require. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fall back to pageable chunked copies when pinned staging setup fails, reject benchmark CPU fallback, and cover thresholds, reuse, and device restoration. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Additional profiling clarified the denominator for the loading-time improvement. The first CUDA Runtime call in the process (
The ~20-second CUDA startup is model-independent, paid once per process/device, and effectively fixed/incompressible from the perspective of this PR. Nsight Systems initializes CUDA before the Python timer, which also explains why profiled runs appeared to take only 7-8 seconds while normal fresh-process runs took 26-30 seconds. Therefore, the roughly 3-second improvement from this PR should be interpreted relative to the approximately 10-second model-loading portion, rather than relative to the full approximately 30-second fresh-process time. In other words, the relevant improvement is closer to 3 seconds out of 10, not 3 seconds out of 30. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Preserve the default not-implemented behavior without making LoadTensor a key function across shared-library boundaries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Expose the reader count as a CUDA provider option with a default of four and make a count of one use synchronous reads. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Treat a reading thread count of zero as a request to retain the existing framework loading path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Pageable fallback synchronization, stream-error cleanup, and direct-struct option validation remain incorrect.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 3
- Review effort level: Balanced
Synchronize pageable fallback copies, drain all staging streams on asynchronous errors, and validate direct CUDA provider struct options before creating reader tasks.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Reader task exceptions can bypass CUDA stream cleanup and permit reuse of a pinned buffer while its prior transfer remains active.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
onnxruntime/core/providers/cuda/cuda_external_data_loader.cc:54
- Repository guidance requires
InlinedVectorinstead ofstd::vectorfor internal C++ containers (AGENTS.md:81-90). This reader list is bounded by 64 entries, so useInlinedVector<std::future<common::Status>>and includecore/common/inlined_containers.h.
This issue also appears on line 83 of the same file.
onnxruntime/core/providers/cuda/cuda_external_data_loader.cc:83
- Repository guidance requires
InlinedVectorinstead ofstd::vectorfor internal C++ containers (AGENTS.md:81-90). Replace this pageable byte buffer with the ORT container alias as well, keeping the bounded allocation behavior unchanged.
std::vector<uint8_t> buffer(std::min(kBufferSize, length));
- Files reviewed: 13/13 changed files
- Comments generated: 1
- Review effort level: Balanced
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
This PR currently depends on #32672. The failing |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The CUDA plugin build has an incomplete Tensor type, and setup failures are retried on subsequent loads.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
onnxruntime/core/providers/cuda/cuda_external_data_loader.cc:215
- When pinned-buffer or stream setup fails, this path falls back successfully but leaves no state recording the failure. Every later external initializer retries the 128 MiB pinned allocation and stream creation before falling back again, which can add repeated setup overhead and memory pressure for models with many initializers under the same resource limit. Cache the setup failure for this loader (and use the pageable fallback directly for subsequent loads).
// TODO: Remember setup failures during initialization and report the first CUDA error
// so later initializers do not repeatedly retry unavailable pinned buffers or streams.
return LoadWithPageableBuffer(*file, data_offset, length, tensor, reading_thread_count_, reader_pool_);
- Files reviewed: 19/19 changed files
- Comments generated: 1
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved range-overflow and staging-resource handling issues require fixes.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
onnxruntime/core/providers/cuda/cuda_external_data_loader.cc:216
- Every CUDA external initializer, including a 1-byte or otherwise sub-threshold one, calls
EnsureResources()and allocates two 64 MiB pinned buffers plus two CUDA streams before reading it. That imposes a 128 MiB pinned-memory/setup cost on small models where the parallel loader provides no benefit; avoid creating the fixed staging resources for small requests (or otherwise size/lazily select the fallback path).
onnxruntime/core/framework/tensorprotoutils.cc:1846
- This new error message is missing a separator between the tensor name and the explanation, so it is emitted as
TensorProto for: weightsExpected to have external data. Add a leading space beforeexpectedso failures identify the problem clearly.
tensor_proto.name(), "Expected to have external data");
onnxruntime/core/providers/cuda/cuda_external_data_loader.cc:216
- After setup fails,
buffers_[0]remains null, so each later external initializer retries both pinned-buffer/stream setup and cleanup before taking the pageable fallback. A model with many external initializers can therefore repeat a costly 128 MiB pin/unpin attempt for every tensor; cache the setup failure for this loader and use the fallback directly for subsequent loads.
// TODO: Remember setup failures during initialization and report the first CUDA error
// so later initializers do not repeatedly retry unavailable pinned buffers or streams.
return LoadWithPageableBuffer(*file, data_offset, length, tensor, reading_thread_count_, reader_pool_);
- Files reviewed: 19/19 changed files
- Comments generated: 1
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
The repeated-load regression test cannot detect a skipped or stale second load because it reuses identical source and destination contents.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
onnxruntime/test/providers/cuda/test_cases/cuda_external_data_loader_test.cc:214
- This repeated-load case writes the same bytes into the same already-populated tensor, so it still passes if a later
LoadTensorcall fails to refill or copy the persistent staging buffers. Verify after each load and change the source bytes (or use a fresh sentinel-filled destination) before the second call so cross-call buffer reuse is actually exercised.
- Files reviewed: 19/19 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Addressed the latest suppressed Copilot review comment in |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use the head size instantiated by Flash Attention quick builds so the INT4 metadata case retains a reachable backend. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Description
Load large CUDA external initializers directly from their files into two reusable 64 MiB pinned host buffers.
The CUDA Execution Provider now supplies an
IExternalDataLoader. For each block, independent reads fill disjoint ranges of the next pinned buffer while the preceding buffer is transferred asynchronously to the GPU. The two buffers and CUDA streams are retained for the lifetime of the loader and synchronized before reuse and before returning.This avoids the previous
mmap -> pageable CPU memory -> pinned memory -> GPUpath. CPU and other execution providers keep their existing external-data behavior, and ordinary CUDA data transfers keep the existing CUDA-managed pageable-memory staging.The default
IExternalDataLoader::LoadTensorimplementation remains available but is defined inline, so eachprovider shared library can emit the interface type information it needs.
The PR also includes a standalone benchmark for CUDA
InferenceSessioncreation with targeted page-cache eviction.Configuration
The CUDA EP provider option
external_data_loader_reading_threadscontrols howmany independent CPU read tasks fill each 64 MiB pinned staging buffer. The
default is 4, which was the fastest setting on the benchmarked eight-disk
NVMe volume. Each task reads a disjoint range of the active buffer; once every
range is complete, the whole buffer is submitted to CUDA while the next buffer
is filled. Values from 0 through 64 are accepted because the best value depends
on the storage device, filesystem, and host:
0disables the custom CUDA external-data loader and uses the existingframework loading path.
1uses the pinned-buffer loader but reads synchronously on the callingthread, without creating a reader task.
2..64use that many parallel read tasks per 64 MiB block.Python example:
The benchmark script exposes the same setting as
--reading-threads.Synchronization and locking
entire initializer load. Initializers therefore cannot concurrently reuse the
same staging resources.
non-overlapping range of the active pinned buffer.
completely filled before its H2D copy is submitted.
buffer is reused, so CPU readers never overwrite memory still consumed by DMA.
Data path
The complete cold-cache path is:
There is no intermediate mmap-backed pageable tensor and no
mmap -> pinneduser-space copy. Standard buffered file I/O still necessarilycopies data from Linux page-cache pages into the pinned user-space buffer.
That CPU copy was not measured independently; the 4.6 GB/s figure covers
the complete cold buffered-read path from the NVMe file into pinned memory.
CUDA then performs one H2D DMA from that pinned buffer into the initializer's
device allocation at approximately 55.4 GB/s in the pinned-memory H2D
microbenchmark. Operators that prepack weights may subsequently read that CUDA
allocation and write a transformed CUDA allocation.
In the benchmarked default configuration, the H2D destination is the
initializer buffer planned and allocated from the CUDA BFC Arena. The new
loader writes each block directly into its final offset in that arena buffer.
This replaces the previous path:
The new path removes the full mmap-backed CPU tensor and the CUDA driver's
implicit pageable-memory staging. It replaces them with controlled parallel
read()calls directly into two persistent pinned buffers followed by explicitasynchronous H2D copies into the same CUDA BFC Arena destination.
Controlled loading benchmarks
Model:
qwen3.5-35b-cuda-int4/model.onnxPOSIX_FADV_DONTNEEDfor both model fileseebea690a079b8727e1fCMAKE_CUDA_ARCHITECTURES=nativeCUDA was verified as active in both builds. A held candidate session allocated approximately 39.4 GiB on the H200.
CUDA
eebea690a079b8727e1feebea690a079b8727e1fThe direct pinned loader reduces complete CUDA
InferenceSessioncreation time by 14.2% cold(4.41 seconds) and 12.1% warm (3.37 seconds).
A direct-I/O sequential read (
dd, 16 MiB blocks) read the external-data file in 3.09 seconds, or6.8 GB/s. The remaining session creation time includes graph initialization, CUDA allocation,
weight upload/preparation, and synchronization.
The earlier 10.63/7.07-second figures were invalid because that build included changes from another PR.
The earlier PyPI comparison was also removed in favor of this same-revision, same-toolchain comparison.
CUDA BF16 reading-thread scaling
Model:
qwen3.5-35b-cuda-bf16/model.onnxInferenceSessioncreation timeeebea690a0external_data_loader_reading_threadsset to each value from 0 through 6The
0case is the existing framework implementation, not a zero-workervariant of the new loader. Its mean is within 0.1% of the build without the PR,
confirming that it preserves the previous behavior. A single pinned-buffer
reader is 4.2% slower than the existing path, so pinned memory alone does not
provide the gain. The improvement comes from parallel reads: performance
plateaus at four to five readers, with the default of 4 producing the best
mean at 33.067 seconds, a 29.8% reduction from the baseline.
CPU control
The CUDA external-data loader is not registered in CPU-only sessions; CPU external initializers retain
the existing mmap path. The controlled CPU comparison confirms no meaningful change.
eebea690a079b8727e1fThe full cold difference is +0.14% and the warm difference is -0.52%, both within run-to-run
noise for an 8-minute CPU prepacking workload. With prepacking disabled, three-run means were
0.406/0.380 seconds (base cold/warm) and 0.418/0.376 seconds (PR cold/warm), likewise showing no
material CPU-path effect.
Validation