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

Rework semaphore waiting #2226

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

doitsujin
Copy link
Collaborator

@doitsujin doitsujin commented Nov 25, 2024

Workaround for #2215.

This moves fence wait tracking from vkd3d_queue to the virtual queues and optimizes away most of the empty submissions that only do a semaphore wait and a semaphore signal, which is what caused the massive performance degradation in Cyberpunk since AMDGPU does not handle this very well.

Additionally, sparse binding serialization now uses the queue's timeline instead of the binary semaphore. At worst, there will now be one extra vkQueueSubmit on the first sparse binding call in order to flush waiters, rather than doing a round-trip over the graphics queue each time.

One potential concern is that this may delay fence signals in scenarios where we set NO_STAGGERED_SUBMIT in case the app does something like queue->Wait(...); nothing for quite some time; queue->Signal(...) with no commands being submitted. The only solution I can think of would involve stalling the signaling queue worker until all pending waits have completed in that case, which is something that we're kind of trying to avoid, but I'm not sure if this is even an issue in practice - Black Myth Wukong doesn't seem to run into this problem with FSR framegen enabled at all, so at least UE5 should be fine.

Most of this code was written on four hours of sleep, so this will need extensive testing.

First step in moving wait logic to the logical queue.

Signed-off-by: Philip Rebohle <philip.rebohle@tu-dortmund.de>
We still need to support waits on the physical queue for memory clears
and sparse initialization.

Signed-off-by: Philip Rebohle <philip.rebohle@tu-dortmund.de>
Signed-off-by: Philip Rebohle <philip.rebohle@tu-dortmund.de>
This was only necessary due to what seems to be a Halo Infinite game bug.
Still flush before waiting on a shared fence to preserve the order for
debug purposes.

Signed-off-by: Philip Rebohle <philip.rebohle@tu-dortmund.de>
Signed-off-by: Philip Rebohle <philip.rebohle@tu-dortmund.de>
Use the submission timeline instead of a round-trip with the
binary semaphore. Reduces number of queue submissions, and
potentially reduces bubbles in single-queue scenarios.

Signed-off-by: Philip Rebohle <philip.rebohle@tu-dortmund.de>
May in some cases avoid a submission and subsequent signal delays.

Signed-off-by: Philip Rebohle <philip.rebohle@tu-dortmund.de>
Any memory clear or sparse init that happens after command execution
on a virtual queue should not delay fence signals on said queue.

Signed-off-by: Philip Rebohle <philip.rebohle@tu-dortmund.de>
Signed-off-by: Philip Rebohle <philip.rebohle@tu-dortmund.de>
@doitsujin doitsujin force-pushed the queue-waiter-rework branch 2 times, most recently from 56c1b59 to ebe548a Compare November 25, 2024 18:51
Reduces number of vkQueueSubmit calls.

Signed-off-by: Philip Rebohle <philip.rebohle@tu-dortmund.de>
@doitsujin doitsujin marked this pull request as ready for review November 25, 2024 21:16
@@ -3298,6 +3298,10 @@ struct d3d12_command_queue
struct vkd3d_fence_virtual_wait *wait_fences;
size_t wait_fences_size;
size_t wait_fence_count;

VkSemaphoreSubmitInfo *wait_semaphores;

Choose a reason for hiding this comment

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

When is this freed?

@@ -18142,6 +18154,9 @@ static void d3d12_command_queue_flush_waiters(struct d3d12_command_queue *comman

if ((vr = VK_CALL(vkQueueSubmit2(vk_queue, 1, &submit_info, VK_NULL_HANDLE))))
ERR("Failed to submit semaphore waits, vr %d.\n");

Choose a reason for hiding this comment

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

Missing "vr" here.

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