Adding ONNX Runtime C-API for WebGPU EP - #21838
Conversation
|
went quickly over it - lgtm in general and will go over it in more detail in a little. |
thank you for the review. I am waiting for the answer of whether/how should pass the proc table information before finalizing the interface. The build is broken - yes because the implementation is not included in the build. I will add a stub implementation. |
|
We need to pass this table: Knowing about DawnProcTable in onnxruntime is not practical so maybe it would need to be void * and cast. Not great. We could #ifdef acting on that that field because we need it only in one specific scenario. |
| * \see OrtApi::SessionOptionsAppendExecutionProvider_WebGPU | ||
| */ | ||
| typedef struct OrtWebGPUProviderOptions { | ||
| int device_id; // WebGPU device id. |
There was a problem hiding this comment.
What's this device_id for? Does it only mean the device (and adapter, instance) is not owned by ORT?
There was a problem hiding this comment.
It's just a number used as key. when the key is the same, it represents the same set of [WGPUInstance, WGPUAdapter, WGPUDevice], and all resources based on the set, including buffer cache manager, program manager and so on.
a better name suggestion is welcome
| * | "storageBufferCacheMode" | "disabled", "lazyRelease", "simple", "bucket" | "bucket" | | ||
| * | "uniformBufferCacheMode" | "disabled", "lazyRelease", "simple", "bucket" | "lazyRelease" | | ||
| * | "queryResolveBufferCacheMode" | "disabled", "lazyRelease", "simple", "bucket" | "disabled" | | ||
| * | "defaultBufferCacheMode" | "disabled", "lazyRelease", "simple", "bucket" | "disabled" | |
There was a problem hiding this comment.
enableGraphCapture is a general session option, not WebGPU specific.
To explain all these caches and different modes could be a little challenging. We should try our best to keep them internal.
There was a problem hiding this comment.
enableGraphCapture is a general session option, not WebGPU specific.
I agree. But unfortunately, in current API this is EP specific configuration for:
- CUDA/TensorRT:
ProviderOptions["enable_cuda_graph"] - ROCM:
OrtROCMProviderOptions::enable_hip_graph
considering backward compatibility this may not be easy change... @pranavsharma what do you think?
There was a problem hiding this comment.
no need to explain the details of each config entry - these should sit somewhere else in documentation.
however, keep a reference to a full list of available config entries is helpful.
There was a problem hiding this comment.
The graph capture functionality referred here is indeed specific to CUDA and TensorRT. Please do not make any backward incompat changes.
| */ | ||
| typedef struct OrtWebGPUProviderOptions { | ||
| int device_id; // WebGPU device id. | ||
| void* instance_handle; // WebGPU instance handle. |
There was a problem hiding this comment.
Question: How do we use instance in ORT?
There was a problem hiding this comment.
In my understanding, it's the only way to create a WGPUAdapter using a WGPUInstance by calling wgpuInstanceRequestAdapter{2|F}.
We also need to call wgpuInstanceWaitAny with a WGPUInstance.
|
Updated with proc table.
It maybe not a good idea to make struct size based on this certain macro considering it's a part of API. But to make users easier to use, I put the proc table member as the last member in the struct. They can still do OrtWebGPUProviderOptions webgpu_options{};or OrtWebGPUProviderOptions webgpu_options{
1, //device ID
instance,
adapter,
device
}; |
| * | ||
| * \see OrtApi::SessionOptionsAppendExecutionProvider_WGPU | ||
| */ | ||
| typedef struct OrtWGPUProviderOptions { |
There was a problem hiding this comment.
Move it to an internal place, and use public functions in this file to create/manipulate the struct. Otherwise you cannot maintain backward binary compatibility
There was a problem hiding this comment.
I understand why you have this concern. extending an existing struct may cause memory issue. Do you mean to have a set of API like this:
CreateWGPUProviderOptions
ReleaseWGPUProviderOptions
WGPUProviderOptionsSetDeviceId
WGPUProviderOptionsSetAdapter
WGPUProviderOptionsSetDevice
WGPUProviderOptionsSetInstance
WGPUProviderOptionsSetProcTable
I have considered this option. Actually currently WebGPU options are put into 2 places: This WGPUProviderOptions struct and a string-to-string kvp. By carefully inspect the requirement, I am convinced that there will be no requirement for adding a new pointer type options in near future. Other types of possible options can be put in the general kvp. Considering the reduce of complexity I would prefer just define the POD structure in the header file.
I am OK to add a void * unused; to buy us a buffer if you still have concern.
There was a problem hiding this comment.
Then people may directly create an object from this struct:
OrtWGPUProviderOptions option;
And, by default the vars in this object are not initialized.
Generally speaking, all our APIs should have a consistent style.
There was a problem hiding this comment.
sure will make this change.
There was a problem hiding this comment.
Can you clarify the expected usage on Android and iOS? Does the user need to create all these instances/handles in their app (which could be Java/Kotlin/Objective-C/Swift)? If so, what do they need to do that?
It would be better if the user can pass in parameters and the EP creates the instances so we minimize the user code required.
There was a problem hiding this comment.
There are 3 scenarios that of I can think may need to pass at least one handle (pointer):
- when user's application static link ORT and dawn, they need to pass the proc table pointer so that ORT can call the webgpu API.
- when user want to use multiple device instance. This may happen when they have multiple models to run and want to use separate buffer management.
- when user want to use the pre-created device. This may happen when user's input is directly from GPU (IOBinding)
User need to figure out how before they use ORT in these scenarios. Usually the handles are created by WebGPU API. User's application may need to dynamic link to dawn, and since onnxruntime.dll dynamic link to dawn as well, they can just include the webgpu header and use the exported functions to create those handles.
For language binding - it's maybe too early stage for users to pass the handles. Anyway, this is optional; they can still use WebGPU EP without passing any handles to have the default behavior
| * 2. Use a custom WebGPU device. The user should create their own handles of `WGPUInstance`, `WGPUAdapter`, and | ||
| * `WGPUDevice` and use arbitrary number in [1..65536) as the device id. The user should provide the handles | ||
| * and the device id in the options. |
There was a problem hiding this comment.
nit: Do we need to shorten WebGPU to WGPU in the API?
There was a problem hiding this comment.
according to #21838 (comment), it seems that the existing webgpu libraries prefer to use wgpu instead of webgpu in native code (C/C++/rust), so making API consistent with what users may be more familiar with.
Description
This PR is for the API change for WebGPU EP.
This PR is for review purpose. The build may fail because of lack implementation of the interface.
Use scenarios
There are 2 scenarios:
design
Introducing this
Device IDconcept (different from the one that is insideMemoryInfo):There is a concern to pass pointers through string: the integer serialization and deserialization is difficult to enforce using the same set of API, which means it's error prone here. And it is also inefficient. So WebGPU EP options include 2 parts:
Consideration
struct OrtWebGPUProviderOptionsmay not be modified once released for ABI compatibility considerations. So try to put minimized items in this and use the string based key-value-pair as much as possible. for example, the buffer cache mode may extend in future and this does not need to modify the API.SessionOptionsAppendExecutionProvider()for default device as well (all options are string based in this use case)Example:
Use default device:
... OrtWebGPUProviderOptions webgpu_options{}; ... std::vector<std::string> keys { "storageBufferCacheMode" }; std::vector<std::string> values { "simple" }; auto status = SessionOptionsAppendExecutionProvider_WebGPU( &session_options, &webgpu_options, keys.data(), values.data(), 1);Use custom device: