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

Add Vulkan Sampler Cache #6847

Merged
merged 1 commit into from
Jan 7, 2025

Conversation

cwfitzgerald
Copy link
Member

There is a similar limit to the limit we have on DX12 that there is a limit to the number of samplers that can be alive at once. On nvidia it's 4000 and is hit by the sampler tests in #6766. This ensures that there is a minimal amount of samplers by stealing a very similar cache structure to the DX12 work.

I have checked the mozilla tree and OrderedFloat 3.4 is vendored in there, so adding the dependency shouldn't be an auditing problem.

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy. If applicable, add:
    • --target wasm32-unknown-unknown
    • --target wasm32-unknown-emscripten
  • Run cargo xtask test to run tests.
  • Add change to CHANGELOG.md. See simple instructions inside file.

@cwfitzgerald cwfitzgerald requested a review from a team as a code owner January 2, 2025 20:18
@cwfitzgerald cwfitzgerald force-pushed the cw/vulkan-sampler-cache branch from 4d8aab5 to 667314f Compare January 2, 2025 20:25
@jimblandy jimblandy self-assigned this Jan 3, 2025
Copy link
Member

@jimblandy jimblandy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine to take this PR, but it is too bad that we incur caching overhead and lose labeling even on GPUs that don't have any limit on samplers. Would it work to have the cache just sort of act as a pass-through when the limit is whatever ordinary GPUs return (i32::MAX??)?

Two other minor comments.

Cargo.toml Show resolved Hide resolved
wgpu-hal/src/vulkan/device.rs Outdated Show resolved Hide resolved
@jimblandy
Copy link
Member

I think the pass-through idea could be implemented entirely locally to vulkan/sampler.rs. There'd just be a shortcut at the top of the create and destroy functions that checks the limit and skips the hash table altogether.

@jimblandy
Copy link
Member

Here's a summary of the values GPUs have reported for maxSamplerAllocationCount:

https://www.vulkan.gpuinfo.org/displaydevicelimit.php?name=maxSamplerAllocationCount&platform=all

Value	        Reports
96	        77
1024	        110
2048	        42
4000	        19414
9999	        34
16384	        82
32768	        1686
65536	        7087
500000	        36
1000000	        4
1048576	        3463
1048580	        101
8388608	        10      
1073741824	944
4294967295	1451

@cwfitzgerald cwfitzgerald force-pushed the cw/vulkan-sampler-cache branch from 69118fb to 8209198 Compare January 6, 2025 23:51
@cwfitzgerald cwfitzgerald force-pushed the cw/vulkan-sampler-cache branch from 8209198 to 4446b17 Compare January 6, 2025 23:53
@cwfitzgerald cwfitzgerald merged commit cb6dbb8 into gfx-rs:trunk Jan 7, 2025
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants