Queue creation improvements

Going off my comment on Discord, a few random ideas:

It would be nice to have easier access to specific queues, or the queues that were requested in create_param::queue_info:

expose the list of queues exactly in the order of create_param::queue_info_list, something like get_queues()
(if no special queues are requested? always?) let lava try to find dedicated queues for compute and transfer (some GPUs have those, and they could potentionally be faster for async operations). this could reuse the existing functions like get_compute_queue() but then return the dedicated ones, not any queue with compute support

There's also a bug with device::create_param::queue_info::flags where it can end up creating a queue with just one of the flag bits set:

liblava/liblava/base/device.cpp

Line 22 in 6a36737

    
           if (!physical_device->get_queue_family(index, param.queue_info_list[i].flags)) {

which then only checks if any flag is set, not all of them:

liblava/liblava/base/physical_device.cpp

Line 42 in 6a36737

if (queue_family_properties[i].queueFlags & flags) {

🔨 created branch queue-creation

fix queue family flags check #43

Several hardware platforms have more than one VkQueue per queue family. This can be useful by being able to submit work to the same queue family from separate queues. While there can be advantages, it isn't necessarily better to create or use the extra queues. For specific performance recommendations, refer to hardware vendors' best practices guides.

https://github.com/KhronosGroup/Vulkan-Guide/blob/master/chapters/common_pitfalls.md#multiple-queues-per-queue-family

Does anyone have experience or information about the hardware vendors' best practices guides (links?)

I like the suggestion in 1. (first post) - should be easy doable.
And for 2. I would like to change the default behavior so that it runs like the proposal.

add device::get_queues/queues #43

Re: 2. I think a sensible implementation would still expose the default queue, but maybe put any found dedicated queues at the front of the list.

E.g. requesting:

GRAPHICS | COMPUTE | TRANSFER
COMPUTE
TRANSFER
becomes:

graphics_queues: [1]
compute_queues: [2, 1]
transfer_queues: [3, 1]

add queues #43

Here are some examples how to use add_queues

    app.manager.on_create_param = [](device::create_param& param) {
        param.set_default_queues(); // (1)
        
        param.set_all_queues(); // (2)
        
        param.add_dedicated_queues(0.5f); // (3)

        param.add_queues(VK_QUEUE_COMPUTE_BIT | VK_QUEUE_TRANSFER_BIT, 2); // (4)

        param.add_queue(VK_QUEUE_COMPUTE_BIT | VK_QUEUE_TRANSFER_BIT); // (5)
        param.add_queue(VK_QUEUE_COMPUTE_BIT | VK_QUEUE_TRANSFER_BIT); // (6)
        
        param.add_queues(VK_QUEUE_COMPUTE_BIT, 2); // (7)
        
        param.add_queue(VK_QUEUE_COMPUTE_BIT);  // (8)
        param.add_queue(VK_QUEUE_TRANSFER_BIT);  // (9)
    };

This is the default handling, even if you do not specify param
Shows how to set all queues, param can still be adjusted afterwards
And this will add all dedicated queues (priority: 0.5), if any are available, else will return false
Shows how to add a specific combination of 2 queues (same queue family)
But if 4. fails because you don't get it from one queue family, you can still call...
... it twice and hopefully get it from different queue families
Just a simple example how to request 2 dedicated queues using one flag
Well, now back to your last post: If you just set this line and...
... that of course, it will add the 2 dedicated queues to the front of the list 🤠

Added some tests in lava-unit, check tests/unit.cpp

Awesome, thanks for putting in the time to work on this 🙏
I'm busy with something else currently, but I'll check this out and report back over the weekend.

You're welcome! No hurry, if something stands out when testing, just let me know.

Just took a look at your additions, and they're great. 👌

I think that the dedicated queue stuff could use some changes, however. Currently it assumes anything but the first queue family is special, but the Vulkan spec makes no guarantees about the order.

What I have in mind is something like this:

// finds a queue family that has compute, but no graphics bit
// return false if there is no such queue family
param.add_dedicated_queue(VK_QUEUE_COMPUTE_BIT, 0.5f); // and [...]_queues() to create multiple queues
// finds a queue family that has transfer but no graphics or compute
param.add_dedicated_queue(VK_QUEUE_TRANSFER_BIT, 0.5f);

and probably remove the old add_dedicated_queues. Might also be worth supporting VK_QUEUE_GRAPHICS_BIT without compute, but that should be it. Combinations of flags for dedicated queues don't really make sense.

I don't know if sorting is still required, depending on how queues are added to params. device::create pushes queues to the front of the graphics/compute/transfer_queue_list but if in the example above the two calls were switched (transfer added before compute), this would happen:

graphics: [default]
compute: [dedicated_compute, default]
transfer: [dedicated_compute, dedicated_transfer, default]

Or maybe it's ok to expect the user to know that the last queue added with a specific bit will be at the front of that list 🤷

Navigation Menu

Repository navigation

Queue creation improvements #43

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Queue creation improvements #43

Description

Activity

TheLavaBlock commented on Jan 1, 2021

TheLavaBlock commented on Jan 2, 2021

pezcode commented on Jan 2, 2021

TheLavaBlock commented on Jan 5, 2021

pezcode commented on Jan 7, 2021

TheLavaBlock commented on Jan 7, 2021

pezcode commented on Feb 12, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions