[webgpu] revise implementation of buffer split support - #26429
Merged
Conversation
Yulong Wang (fs-eire)
requested review from
Copilot,
Jiajia Qin (qjia7) and
Xiaofei Han (xiaofeihan1)
October 29, 2025 00:42
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the WebGPU provider to replace the testing-only boolean flag small_storage_buffer_binding_size_for_testing with a more flexible max_storage_buffer_binding_size parameter that allows users to explicitly set the maximum storage buffer binding size.
Key changes:
- Replaced boolean testing flag with a configurable uint64_t parameter for controlling storage buffer binding size
- Moved segment calculation from ProgramBase member storage to separate computed vectors
- Made ProgramBase const-correct in Run operations to prevent unintended modifications
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| webgpu_provider_options.h | Renamed option key from testing-specific to production-ready name |
| webgpu_provider_factory.cc | Replaced boolean parsing with uint64_t parsing using std::from_chars |
| webgpu_context.h | Changed parameter type from bool to uint64_t with validation logic |
| webgpu_context.cc | Updated Run signature to const and moved indirect dispatch handling |
| shader_helper.h | Added spans for segments and removed FinalizeInputs method |
| shader_helper.cc | Updated to use segment spans instead of ProgramInput/Output members |
| program_manager.h | Changed CalculateSegmentsForInputsAndOutputs to return segments via out parameters |
| program_manager.cc | Implemented segment calculation into separate vectors |
| program_cache_key.h | Updated signature to accept segment spans |
| program_cache_key.cc | Updated to use segments from spans with optimization for default values |
| program.h | Removed segments members from ProgramInput/Output structs and related setters |
| program.cc | Moved indirect dispatch tensor registration to SetIndirectDispatchTensor |
| compute_context.h | Updated RunProgram to accept const reference |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Jiajia Qin (qjia7)
approved these changes
Nov 2, 2025
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
This PR addresses a few concerns:
const ProgramBase&->ProgramBase&: this itself is not doing something wrong but gives much more pressure for who reads the code to understand whether/where the program object is modified. It also can introduce further unexpected modifications to the program object (for example the indirect dispatch code)"ep.webgpuexecutionprovider.smallStorageBufferBindingSizeForTesting"to"ep.webgpuexecutionprovider.maxStorageBufferBindingSize"so now it's possible to set any value in option. (setting to <128MB will cause an assert failure)