Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bind groups with binding arrays with high max count in their bind group layout take an extremely long time to allocate descriptor sets for #3729

Closed
JMS55 opened this issue Apr 25, 2023 · 2 comments
Labels
area: performance How fast things go backend: vulkan Issues with Vulkan

Comments

@JMS55
Copy link
Collaborator

JMS55 commented Apr 25, 2023

Description
Creating a bind group every frame with 3 binding arrays, each with a maximum count of 50_000 bindings in the bind group layout takes an unreasonable amount of time each frame. Note that I have less than 50 actual bindings in the bind group - the cost is scaling purely based on the bind group layout binding count. Setting it even higher, like 1million, results in insane times like 450ms.
image

Repro steps

  1. Create a bind group layout like
BindGroupLayoutEntry {
    binding: 3,
    visibility: ShaderStages::COMPUTE,
    ty: BindingType::Buffer {
        ty: BufferBindingType::Storage { read_only: true },
        has_dynamic_offset: false,
        min_binding_size: None, // TODO
    },
    count: Some(unsafe { NonZeroU32::new_unchecked(50_000) }),
},
BindGroupLayoutEntry {
    binding: 5,
    visibility: ShaderStages::COMPUTE,
    ty: BindingType::Texture {
        sample_type: TextureSampleType::Float { filterable: true },
        view_dimension: TextureViewDimension::D2,
        multisampled: false,
    },
    count: Some(unsafe { NonZeroU32::new_unchecked(50_000) }),
}
  1. Each frame, create a bind group based on this layout, with a binding array of a small number (<50) of buffers and textures.

Here's a C++ trace captured from Nsight, compressed via 7zip, and then into a zip because github won't let me upload 7z archives.
solari__2023_04_25__12_15_53 (2).zip

You need to be on a GPU that supports ray tracing. You can also run the code directly via cargo run --example solari from here https://github.com/JMS55/bevy/tree/solari, and see the code here https://github.com/JMS55/bevy/tree/solari/crates/bevy_solari/src/scene.

Misc
@danchia investigated and found that wgpu/gpu-descriptor was using the FREE_DESCRIPTOR_SET flag, which may be part of the cause https://github.com/zakarumych/gpu-descriptor/blob/413b91137138307afbede5bc918f77bae403493c/gpu-descriptor/src/allocator.rs#L278-L286. I'm not familiar enough with Vulkan to know.

Platform
Windows 11, Ryzen 5 2600, AdapterInfo { name: "NVIDIA GeForce RTX 3080", vendor: 4318, device: 8710, device_type: DiscreteGpu, driver: "NVIDIA", driver_info: "528.49", backend: Vulkan }

@JMS55
Copy link
Collaborator Author

JMS55 commented Apr 25, 2023

Should probably be added to #3637

@teoxoy teoxoy added area: performance How fast things go backend: vulkan Issues with Vulkan labels Jun 8, 2023
dominikh added a commit to dominikh/jello that referenced this issue Jul 19, 2024
Apparently the size is neither needed nor validated when we use
partially bound binding arrays. This will allow us to more gracefully
fail on GPUs that support fewer texture bindings than we have images.

For now we're still specifying a fixed limit when creating the bind
group layout because we don't have the code to work based off the device
limits. There is also gfx-rs/wgpu#3729 which
makes it infeasible to specify very large values.
@cwfitzgerald
Copy link
Member

This will be obsoleted by the creation of bind_group.update_bindings

@cwfitzgerald cwfitzgerald closed this as not planned Won't fix, can't repro, duplicate, stale Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: performance How fast things go backend: vulkan Issues with Vulkan
Projects
None yet
Development

No branches or pull requests

3 participants