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

Update shouldSubmit to correctly handle descriptorPoolOverallocation #4166

Merged

Conversation

esullivan-nvidia
Copy link
Contributor

Currently shouldSubmit will force the dxvk context to be flushed when too many descriptor pools have been allocated. This heuristic does not work when VK_NV_descriptor_pool_overallocation is in use because there will only ever be a single pool.

This change updates the heuristic to use the number of allocated sets when VK_NV_descriptor_pool_overallocation is in use.

Resolves the issue described in ValveSoftware/Proton#7862

…cation

Currently shouldSubmit will force the dxvk context to be flushed when
too many descriptor pools have been allocated. This heuristic does not
work when VK_NV_descriptor_pool_overallocation is in use because there
will only ever be a single pool.

This change updates the heuristic to use the number of allocated sets
when VK_NV_descriptor_pool_overallocation is in use.
@doitsujin
Copy link
Owner

Does this mean the underlying descriptor pool can grow indefinitely, or are we hitting a pathological case with this game where our lookup table gets too large?

@esullivan-nvidia
Copy link
Contributor Author

For the bug I included in the description the application ends up repeatedly calling vkAllocateDescriptorSets so the driver will eventually hit an OOM if the pool isn't reset. The game doesn't use d3d for presentation so in this case the descriptor pool ends up growing indefinitely.

@doitsujin
Copy link
Owner

alright, I was kind of expecting pool memory to still be limited in some way, good to know.

@doitsujin doitsujin merged commit c26b2ad into doitsujin:master Jul 25, 2024
4 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