Expose and enforce a maximum buffer size. #2776
Conversation
|
A test is failing, probably because of the 128MB value I naively set for dowlevel_defaults (it's the guaranteed supported SSBO size). Maybe I misunderstood what dowlevel_defaults was for but let's first discuss whether the general approach makes sense. |
| @@ -804,6 +808,7 @@ impl Limits { | |||
| max_compute_workgroup_size_y: 256, | |||
| max_compute_workgroup_size_z: 64, | |||
| max_compute_workgroups_per_dimension: 65535, | |||
| max_buffer_size: 134_217_728, // 128MB is the guaranteed supported SSBO size in GL | |||
There was a problem hiding this comment.
nit: I'd prefer 128 * 1024 * 1024, since that's more recognizably 128Mi.
| @@ -847,6 +847,7 @@ impl super::PrivateCapabilities { | |||
| max_compute_workgroup_size_y: self.max_threads_per_group, | |||
| max_compute_workgroup_size_z: self.max_threads_per_group, | |||
| max_compute_workgroups_per_dimension: 0xFFFF, | |||
| max_buffer_size: self.max_buffer_size as u32, | |||
There was a problem hiding this comment.
Let's use a try_from(...).unwrap() here.
| let max_size = device.limits.max_buffer_size as BufferAddress; | ||
| if desc.size > max_size { | ||
| break resource::CreateBufferError::MaxBufferSize { | ||
| requested: desc.size, | ||
| maximum: max_size, | ||
| }; | ||
| } | ||
|
|
There was a problem hiding this comment.
Would this check fit better in Device::create_buffer?
|
On second thought I think that completely forbidding large allocations is probably not the right thing to do in wgpu. Instead we should set a limit only for drivers that are known to be buggy or perhaps via a "safer" opt-in or opt-out mode, while letting programs/users that can afford to trust the graphics driver allocate huge buffers as they see fit. |
|
Oh, I thought these limits were going to reflect architectural limits of the underlying API. That's why |
|
Dropping this in favor of #2796. |
Connections
WebGPU spec discussion: gpuweb/gpuweb#1371
Description
A maximum buffer size will be exposed in the WebGPU specification. This is mostly because of metal which is officially restrictive in that area while other APIs (as far as I know), don't expose limits on the maximum buffer size and only limit the maximum binding size.
However in practice drivers have bugs. Mesa/lavapipe for example runs into trouble with sizes that don't fit into a signed 32 bit integer, and more generally my experience is that allocating gpu buffers and textures in the order of multiple gigabytes is a very good way to find driver bugs, in particular the kind that bring the OS on the mid/low end of the hardware ecosystem.
This PR exposes metal's maximum buffer size in Limits and sets the max buffer size to i32::MAX for all other backends.
I realize that the i32::MAX global limit might be controversial. It's very likely that the browser will override it with something even more conservative but on the native side, some might find it limiting.
The limit could depend on the hardware and driver with some more work.
Testing
None.