Fix family queue selection in tests#448
Conversation
Signed-off-by: Matthias Möller <m_moeller@live.de>
TinyTinni
force-pushed
the
fix-familyQueue-selection-in-tests
branch
2 times, most recently
from
89a125f to
acbb3d6
Compare
| // * Queue family 0 and 2 have compute capabilities | ||
| // This test assumes: | ||
| // * There are at least 2 different Queue families with compute capabilities | ||
| // * GPU is able to process parallel shader code across different families |
There was a problem hiding this comment.
This requires not only compute enabled queues, but it actually requres parallel enabled queues. This is something not all GPUs support, would be interesting if there's a way to actually check for parallel enabled queues
There was a problem hiding this comment.
You mean GPUs which have the second implementation referenced in the vulkan doc for the queues?
How a VkQueue is mapped to the underlying hardware is implementation-defined. Some implementations will have multiple hardware queues and submitting work to multiple VkQueues will proceed independently and concurrently. Some implementations will do scheduling at a kernel driver level before submitting work to the hardware. There is no current way in Vulkan to expose the exact details how each VkQueue is mapped.
(emphasize mine) source: https://docs.vulkan.org/guide/latest/queues.html
There was a problem hiding this comment.
No - I mean queues that can actually run workloads in parallel (ref https://towardsdatascience.com/parallelizing-heavy-gpu-workloads-via-multi-queue-operations-50a38b15a1dc)
There was a problem hiding this comment.
I think I see the problem. If the queues are not executed in parallel, the test can fail because of the failed timing measurement.
kompute/test/TestAsyncOperations.cpp
Lines 120 to 121 in ec7f816
I didn't expect timing measurement in a test. Wouldn't it be better to declare this test as a benchmark? Or copy it and just add this assert to the benchmark. Logically, the test should run fine even on hardware with just serial execution, except that it is probably not that much faster, which doesn't invalidate the correctness.
There was a problem hiding this comment.
Correct - indeed it's commented explicitly in the test. I think it'd be valid to mark it as benchmark, but indeed it's not a bug as initially assumed.
We have a benchmark section that it can be moved to - would also be interesting if its possible to identify parallel enabled queues, but previously i had to look at GPU manuals to find out which are the parallel supported queues.
There was a problem hiding this comment.
I think there is currently no way on how to get this information. There is some discussion on the vulkan github [1] about it, but it is from 2020 and it is probably not going forward.
What do you think, should we move the whole test to benchmark or alternatively, we add at least a message to the assert. When it fails, at least there will be an explanation to the user on why it fails and that maybe his hardware does not support parallel execution (or the 2 specific queues cannot run in parallel, which can also happen on AMD systems when they are queued next to each other, like someone mentioned in [1]).
Something like
EXPECT_LT(durationAsync, durationSync * 0.6) << "There was no speedup in using multiple queues from different QueueFamilies. Maybe your GPU does not support parallel execution.";
There was a problem hiding this comment.
Ok that sounds like a good suggestion!
There was a problem hiding this comment.
Added the message to the assertion.
Signed-off-by: Matthias Möller <m_moeller@live.de>
TinyTinni
force-pushed
the
fix-familyQueue-selection-in-tests
branch
from
acbb3d6 to
e35633f
Compare
* checks if given family queue has compute capability Signed-off-by: Matthias Möller <m_moeller@live.de> * test will search for queues with compute capabilities Signed-off-by: Matthias Möller <m_moeller@live.de> --------- Signed-off-by: Matthias Möller <m_moeller@live.de> Signed-off-by: evanokeeffe <evan.okeeffe@tas-2.com>
* checks if given family queue has compute capability Signed-off-by: Matthias Möller <m_moeller@live.de> * test will search for queues with compute capabilities Signed-off-by: Matthias Möller <m_moeller@live.de> --------- Signed-off-by: Matthias Möller <m_moeller@live.de> Signed-off-by: evanokeeffe <evan.okeeffe@tas-2.com> Signed-off-by: Alejandro Saucedo <alejandro.saucedo@zalando.de>
* checks if given family queue has compute capability Signed-off-by: Matthias Möller <m_moeller@live.de> * test will search for queues with compute capabilities Signed-off-by: Matthias Möller <m_moeller@live.de> --------- Signed-off-by: Matthias Möller <m_moeller@live.de> Signed-off-by: evanokeeffe <evan.okeeffe@tas-2.com>
* checks if given family queue has compute capability Signed-off-by: Matthias Möller <m_moeller@live.de> * test will search for queues with compute capabilities Signed-off-by: Matthias Möller <m_moeller@live.de> --------- Signed-off-by: Matthias Möller <m_moeller@live.de> Signed-off-by: evanokeeffe <evan.okeeffe@tas-2.com>
2 participants
Closes #445.
This PR will add a check in the
Managerclass to check, if the given family queues exists and have compute capabilities.If the selected queue has no compute capability, it can lead to problems e.g. a display freeze on my current AMD Linux system.
The PR also tries to generalize the test
TestAsyncOperationsfor a wider range of GPUs. The test will be skipped, if the GPU does not support multiple queue families with compute capabilities.