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

VulkanPushPool - more efficient replacement for 3x VulkanPushBuffer #17122

Merged
merged 12 commits into from
Mar 15, 2023

Conversation

hrydgard
Copy link
Owner

@hrydgard hrydgard commented Mar 14, 2023

This new VulkanPushPool is similar to 3x VulkanPushBuffer, except that unused buffers are shared among the (up to) 3 frame contexts - any frame can grab them if they're free and haven't been used in the last couple frames.

The result is that excessive allocations from spikes can be more efficiently re-used. When using texture packs with very large textures, VRAM consumption is cut by more than a third.

Anyway, the result will be good VRAM savings, effectively leading to less out-of-memory conditions. Though the effect is really mostly there when using large texture replacements.

The shrinking heuristics can be tweaked further, but it's doing a better job than the old code already. It's very non-aggressive to avoid excessive re-allocation, while shaving off really huge spikes over time.

Also changes the old VulkanPushBuffer to have the same allocation interface as the new one.

@hrydgard hrydgard added this to the v1.15.0 milestone Mar 14, 2023
@hrydgard hrydgard marked this pull request as ready for review March 15, 2023 00:26
Common/GPU/Vulkan/VulkanMemory.cpp Outdated Show resolved Hide resolved
Comment on lines 355 to 357
stats_.pushUBOSpaceUsed = 0; // (int)pushUBO->GetOffset();
stats_.pushVertexSpaceUsed = 0; // (int)frame->pushVertex->GetOffset();
stats_.pushIndexSpaceUsed = 0; // (int)frame->pushIndex->GetOffset();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think these stats are still TODO? Or should we remove?

-[Unknown]

Copy link
Owner Author

Choose a reason for hiding this comment

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

First one is wildly inaccurate since we have a lot of other stuff in that buffer. I'll fix up the other ones.

GPU/Vulkan/DrawEngineVulkan.cpp Outdated Show resolved Hide resolved
Common/GPU/Vulkan/VulkanMemory.cpp Outdated Show resolved Hide resolved
Common/GPU/Vulkan/VulkanMemory.cpp Outdated Show resolved Hide resolved
@hrydgard hrydgard merged commit bcd6f4a into master Mar 15, 2023
@hrydgard hrydgard deleted the vulkan-push-pool branch March 15, 2023 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants