Repository navigation
[WebGPU] Move copy implementation into data transfer - #33410
Open
Xiaofei Han (xiaofeihan1) wants to merge 1 commit into
Open
Xiaofei Han (xiaofeihan1) wants to merge 1 commit into
Xiaofei Han (xiaofeihan1) wants to merge 1 commit into
Conversation
Keep the explicit BufferManager dependency and existing flush/retirement behavior while separating upload, GPU copy, and download implementation from buffer allocation and caching. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 49a8cb48-b647-4474-996c-4819f8171941
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The focused extraction preserves existing behavior, consistently migrates callers and regression tests, and has no identified actionable findings.
0 open findings
What changed in this PR
Separates WebGPU data transfers from buffer management while preserving existing copy, submission, and buffer-lifetime behavior.
Changes:
- Moves upload, GPU copy, and download implementations into
DataTransferImpl. - Updates native and plugin callers while retaining caller-selected buffer managers.
- Migrates existing regression tests to the relocated implementation.
| File | Description |
|---|---|
| onnxruntime/test/providers/webgpu/webgpu_stream_regression_test.cc | Migrates buffer-clear test transfers. |
| onnxruntime/test/providers/webgpu/webgpu_context_test.cc | Migrates ordering, allocation, and replay tests. |
| onnxruntime/test/providers/webgpu/plugin/test_webgpu_allocators.cc | Updates a comment naming the copy implementation. |
| onnxruntime/test/providers/webgpu/dispatch_normalization_test.cc | Migrates test uploads and readbacks. |
| onnxruntime/test/providers/webgpu/contrib_ops/moe_test.cc | Migrates gate-index test transfers. |
| onnxruntime/test/providers/webgpu/concurrent_context_test.cc | Updates concurrent transfer construction. |
| onnxruntime/core/providers/webgpu/webgpu_provider_factory.cc | Updates plugin transfer construction and explanatory comment. |
| onnxruntime/core/providers/webgpu/webgpu_execution_provider.cc | Passes context to native transfers. |
| onnxruntime/core/providers/webgpu/webgpu_context.h | Grants transfer implementation access. |
| onnxruntime/core/providers/webgpu/ep/sync_stream.cc | Passes context to stream transfers. |
| onnxruntime/core/providers/webgpu/data_transfer.h | Declares copy methods and context dependency. |
| onnxruntime/core/providers/webgpu/data_transfer.cc | Houses the relocated copy implementations. |
| onnxruntime/core/providers/webgpu/buffer_manager.h | Removes copy declarations and clarifies responsibilities. |
| onnxruntime/core/providers/webgpu/buffer_manager.cc | Removes relocated copy implementations. |
🧠 Review effort: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This branch has not been deployed
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.
Description
Move
Upload,MemCpy, andDownloadfromBufferManagerinto the existingDataTransferImpl, and route the existing native, plugin, and test callers through that implementation.This is a copy-only extraction from #33286:
DataTransferImplstill receives the caller-selectedBufferManager, together with its context and recording.context.Flush(buffer_manager, recording). Submission order, staging-buffer behavior, validation, and copy-size alignment are preserved.BufferManagerretains allocation, caching, and release responsibilities.Flushimplementation/signatures, recording-to-pool association, Run/replay lifecycle, or initializer-pool retirement.Validation: Windows Release, WebGPU/D3D12. Existing tests were migrated to exercise the relocated copy implementation.
Native configuration and build, both passed:
From
W:\b\Release, native regressions passed: 76 passed, 1 capability-related skip (WebGpuContextTest.AdapterIndexRejectsUnsupportedBuild).Plugin configuration and build, both passed:
From
W:\b\Release, modern plugin regressions passed: 57 passed; 2 pre-existing disabled tests were not run.The supported legacy-mode suite also passed: 41 passed (CTest: 1/1 passed).
WASM was not built locally.
Motivation and Context
Separate data-copy implementation from buffer allocation/cache management without coupling that cleanup to a buffer-retirement redesign. Keeping the explicit
BufferManagerdependency preserves the existing flush behavior and makes this refactor independently reviewable.