Dynamic management of vsg::DescriptorPool(s) to address paged database and loading/creation of data during running of the application.

[Robert Osfield]

Yesterday I began work on refactoring how vkDescriptorSet are allocated and freed from the associated vkDescrptorPool to improve the VSG's ability to handle dynamic scene graphs such as paged databases or where applications load new data on the fly.

At this point I'm still getting a feel for the "lie of the land" doing experiments with possible other routes, my initial experiment has been to move DescriptorSet::Implementation out of the nested DescriptorSet scope and do it as a class in vsg:: scope, I've put this into a throw away branch of the VSG.

https://github.com/vsg-dev/VulkanSceneGraph/compare/DescriptorSetExperiement

My current direction of experiment is to move requests of new vkDescriptorSet that needs to be done within the vsg::DescirptorSet::compile(Context&) call into vsg::Context and have vsg::Context manage a list of vsg::DescriptorPool. 

With this approach vsg::DescriptorPool would keep track of all the VkDescriptorSet all the vkAllocateDescriptorSets(..) and vkFreeDescriptorSets(..) calls counting all the use and availability of descriptors of each descriptorType.  When a DescriptorPool is empty of available descriptors as required by an vkAllocateDescriptorSets(..) vsg::Context would then go allocate a new vsg::DescriptorPool.

It may also be possible to skip the vkFreeDescriptorSets(..) and just reuse the previously allocated but not no longer required vkDescriptorSet and use the vkUpdateDescriptorSets(..) to repurpose them.

My plan today is flesh out more of this experiment and see what I learn about what works well and what doesn't.  Once I have a good idea of what is going to work I'll rewrite the code in it's final form.

My hope is at the end, for end users it'll be simpler to create new scene graphs on the fly as users won't need to worry about DescriptorPool's, and if we can get reuse working then it should be lighter weight to delete and create new subgraphs as the resources on the CPU and GPU will be more stable.

Cheers,

Robert.

[Robert Osfield]

I have now add tracking of available descriptor's in vsg::DescriptorPool and allocation of new DescriptorPool when there aren't enough available  in existing DescrtiptorPool.  It's super crude, so view as just experimental.  Latest changes are:

   https://github.com/vsg-dev/VulkanSceneGraph/commit/0bd89e145a41f2d362654c73cc6cfc6d78d16c86

This afternoon I'll experiment with reusing no longer required DescriptorSet.

[Robert Osfield]

On Wed, 18 May 2022 at 12:25, Robert Osfield <robert....@gmail.com> wrote:

This afternoon I'll experiment with reusing no longer required DescriptorSet.

I have now implemented recycling of vkDescriptorSet in my DescriptorSetExperiement branch.  It looks to be working without any glitches as far as I can tell.  Seems to have gone very smoothly so far.  The implementation is a bit hacky but that's fine as I'm just trying to learn about the problem and possible solutions at this stage.  The changes to implement the recyling is:

    https://github.com/vsg-dev/VulkanSceneGraph/commit/11c3ef259dba05d5f3b327f4363982e073d9f6ca

I have have changed vsg::Context to have a list of ref_ptr<vsg::DescriptorPool> rather than just a single one, this means code that was manually setting up the DescriptorPool and then assigning it to vsg::Context will no longer work, there were examples of this in vsgExamples so I've had to create DescriptorSetExprment branch of vsgExamples.  The changes are just commenting out code as now vsg::Context will allocate DescriptorPool on demand so these steps are no longer required:

    https://github.com/vsg-dev/vsgExamples/commit/c77226990a655a972650243e4e99291d2f5887e7

I think there is still value in checking newly loaded/created subgraphs for the descriptors/descriptor sets they'll need, but this can be used as an advisory about what resources are likely to be needed, rather than a step that will be essentia, and allocation of the DescriptPools can be deferred till the compile traverseral.  I haven't figured out exactly how this will be done yet, just know that it's something that really should be added.

It's the end of the working day here so I'll wrap up.  Tomorrow I'll do a code review of the experimental work and figure out what the final code should look like, I'll create a new branch for the final implementation.   I'd love to be able to wrap it up tomorrow but coding is coding, one doesn't know how long it'll take to complete till you get everything tested and checked in.

The code I've checked into the DescriptorSetExpreiment branches is hacky but is probably far enough along for others to check it out and see it works with their applications and data.  Feedback and suggestions are welcome.

Cheers,

Robert.

[role...@gmail.com]

Hi Robert,

I just downloaded and built the DescriptorSetExperiement branch. Just one warning, other than that, no problems.

/home/roland/Source/VulkanSceneGraph/src/vsg/utils/Builder.cpp:28:14: warning: unused variable ‘maxSets’ [-Wunused-var
iable]
  28 |     uint32_t maxSets = maxNumTextures;
     |              ^~~~~~~

vsgdynamicload worked without issue with the teapot.

I then modified my own code to use the new system. All I had to do was to delete (#if 0) the same section as you did in vsgdynamicload DynamicLoadAndCompile::CompileOperation::run() and it worked. It all looks good to me.

Thanks!,

Roland

[Robert Osfield]

Hi Roland,

On Thursday, 19 May 2022 at 09:31:30 UTC+1 role...@gmail.com wrote:

Hi Robert,

I just downloaded and built the DescriptorSetExperiement branch. Just one warning, other than that, no problems.

/home/roland/Source/VulkanSceneGraph/src/vsg/utils/Builder.cpp:28:14: warning: unused variable ‘maxSets’ [-Wunused-var
iable]
  28 |     uint32_t maxSets = maxNumTextures;
     |              ^~~~~~~

This was due to some previous API that is no longer required.  I've now checked in remove of the maxSets parameters and a few other bits of vsg::Builder and other classes that are no longer needed.

I am also now most the way through cleaning up the experimental code putting in a form that I'd be happy to merge with master.  I've not quite finished yet but it's getting close now.

I then modified my own code to use the new system. All I had to do was to delete (#if 0) the same section as you did in vsgdynamicload DynamicLoadAndCompile::CompileOperation::run() and it worked. It all looks good to me.

Just #if the code out for right now as I'm planning to look at adding functionality into vsg::Context for reserving spacing in descriptor pools, so rather than code like was originally in DynamicLoadAndCompile that had to collection the resource requirements and allocate the required DescriptorPool it'll collect the resource requirements and then pass these details onto the vsg::Context which can then see if there is enough space in existing DescriptorPool, but if not allocate more automatically.

This reservation approach is something I'm thinking would be appropriate for the first Viewer::compile() as this could be used to reserve a big vsg::DiscriptorPool that can handle most of the needs you expect during an applications life.  This way it should be possible to avoid creating lots of vkDescriptorPool.

At least that's the theory I'm working on :-)

I would be worth pulling the latest changes to DescriptorSetExperiement and testing against it.  It should just compile against you application if it's already compiling against this branch.  The code is far nearer final form with less debug output being spewed out on the console.

Cheers,

Robert.

[Robert Osfield]

The clean up of the experimental work has progressed to the point where it's now in a form that is almost ready to merge with master, so I've renamed the DescriptorSetExperiement branch to DynamicDescriptorPool to better reflect is status and function:

    https://github.com/vsg-dev/VulkanSceneGraph/tree/DynamicDescriptorPool

I'm now workung on the reserve feature.

[Robert Osfield]

I have add a Context::reserve(ResourceRequirements& requirements) method to enable preallocation of resources before a compile traversal is run.  At this point it just reports stats on the descrptor/set requirements rather than do any preallocation, but as it's end working of day here now I'll need to finish the implementation tomorrow.  Changes to VSG DynamicDescriptorPool to add Context::reserve(..) API:

    https://github.com/vsg-dev/VulkanSceneGraph/commit/0e96dacd6f8dad6c6a76a271030661fdcbe1a09d

Changes to vsgExamples DynamicDescriptorPool (renamed DescriptorSetExperiement) from branch:

   https://github.com/vsg-dev/vsgExamples/commit/071c0ab1486cdd21128d0f55e048283b0ee45e62

The API now feels pretty clean and coherent so I think we are pretty close to being able to merge with VSG master.  It should be safe to checkout the DynamicDescriptorPool branches of VSG and vsgExamples.  I've bumped the VSG version to 0.3.2 so this can be used in cmake find_package() if you require the new functionality.

[Robert Osfield]

I have now checked in support into vsg::Context::reserve(...) for checking availability of descriptor/descriptor sets in existing vsg::DescriptorPool, if enough are available then the reserve() just returns without any new allocations, but if there aren't enough then a new vsg::DescriptorPool is created to meet the short fall.

The nice thing about this scheme is you can allocated bigger DescriptorPool at start up then have this used until exhausted.  The automatic recycling of VkDescriptorSet/Descriptors also works with this so if you are loading/destroying subgraphs through the life of the application that same DescriptorPool resources can be reused lower overheads.  This is all done for you, so you don't have to worry about this.

Changes to support this are:

    https://github.com/vsg-dev/VulkanSceneGraph/commit/d9be42ac4ea0673ae115e45baca265262e458b6c

To allocate bigger DescriptorPool the scene graph initially required VSG has had been supported by assigning the vsg::ResourceHints to a node in your scene graph - typically the root node of subgraph.  This is how vsgGIS::TileReader does it:

    uint32_t tileMultiplier = std::min(estimatedNumOfTilesBelow, maxNumTilesBelow) + 1;

    // set up the ResourceHints required to make sure the VSG preallocates enough Vulkan resources for the paged database
    vsg::CollectResourceRequirements collectResourceRequirements;
    group->accept(collectResourceRequirements);
    group->setObject("ResourceHints", collectResourceRequirements.createResourceHints(tileMultiplier));

These are now available when the subgraph is loaded and the vsg::CollectResourceRequirements that is run on that subgraph after loading uses any ResourceHints it finds - this is how paged databases have had to work prior to the new dynamic DescriptorPool functionality.   This new functionality makes it possible to do without ResourceHints, as DescriptorPool will now be allocated on demand, but they are still helpful in avoiding too many DescriptorPools being allocated on the fly.

As we now have dynamic DiscriptorPool support the need for really large values in ResouceHint is reduced substantially.  I do testing here to see how much smaller we can go without forcing too many DescriptorPools to be required at rendering time.

Another item I'd like to add is a minimal DescriptorPool allocation size, so if your application wants to reserve  a small number of descriptor set after loading a new model it can over-allocate so that any follow on loads won't force each load to require a new DescriptorPool.

--

Another way of making sure there is plenty of base resources is to set up a vsg::ResourceRequirements struct with the settings you want to account for and pass this to the initial vsg::Viewer:compile(resourceRequirements) call.  I may need to refine some of the member names to help make some of this functionality clearer as this class has evolved since it's inception along with parts of the VSG.  Now that the VSG code base is settle down to it's 1.0 inconsistencies are apparent so there will be a bit of refinement that needs to happen, I expect this will continue even after 1.0 as we learn more about the needs of VSG users, the evolution of the Vulkan and the VSG.

If you are reading through the VSG/vsgXchange etc. code bases and see things that aren't clear, coherent and consistent let me know.  Even if the naming etc. is correct as is, if it's confusing/not clear then there may be other things that needed to tightened up/documented to improve clarity and easy of use.

Cheers,

Robert.

[Robert Osfield]

> Another item I'd like to add is a minimal DescriptorPool allocation size, so if your application wants to reserve  a small number of descriptor set after loading a new model it

> can over-allocate so that any follow on loads won't force each load to require a new DescriptorPool.

I have now completed this final item on the refactor of vsg::DescriptorPool/DescriptorSet/Context to provide dynamic allocation and reuse of vkDescriptorSet and the Descriptors associated with them.  Changes are checked into the DynamicDescriptorPool branch.

Now that vsg::DescriptorPool has be created on demand we no longer need to over-size the initial vsg::DescriptorPool when handling paged databases, so I've adjusted the vsg::ResouceHints settings to use 1024 as an upper limit, rather than 40,000 that vsgpagedlod and vsgXchange::OSG were using for paged databases.  This always was a crazy limit but I put it so high just to avoid crashes, during the development period, with the plan that eventually I'd implement dynamic DescriptorPool management.  Feels good to finally put it to bed.

vsg::Viewer::compile(..) already had an optional ref_ptr<ResourceHints> parameter. so this new dynamic DescriptorPool functionality can leverage this and now passes on the settings to the provide the minimum DescriptorPool dimensions that vsg::Context uses when creating new DescriptorPool.  These minimum values can help avoid overly fine grained allocation of DescriptorPool.

The vsgdyamicload example now passes in the ResourceHints via the Viewer::compile(resourceHnts) call rather than assigning the ResourceHints to a root node as meta data.  The code is in the form:

        {
            // To help reduce the number of vsg::DescriptorPool that need to be allocated we'll provided a minimim requirements via ResourceHints.
            resourceHints = vsg::ResourceHints::create();
            resourceHints->numDescriptorSets = 256;
            resourceHints->descriptorPoolSizes.push_back(VkDescriptorPoolSize{VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 256});
        }

        // configure the viewers rendering backend, initialize and compile Vulkan objects, passing in ResourceHints to guide the resources allocated.
        viewer->compile(resourceHints);

You can also read/write ResouceHints, so you can read from a .vsgt if you so wished.  The vsgdynamicload example already had code to read form a file, it looks like this:

       // provide setting of the resource hints on the command line
        vsg::ref_ptr<vsg::ResourceHints> resourceHints;
        if (vsg::Path resourceFile; arguments.read("--resource", resourceFile)) resourceHints = vsg::read_cast<vsg::ResourceHints>(resourceFile);

--

I now view this work as complete, so will do a code review of my changes to VSG and vsgExamples, if all looks good I'll merged with master.

If you spot any issues please holler.

Cheers.

Robert.

[Robert Osfield]

I have now merged the DynamicDescriptorPool branches of VSG and vsgExamples with master.  vsgGIS and vsgXchange are also updated.

    VulkanSceneGraph DynamicDescriptorPool changes.

    vsgExamples DynamicDescriptorPool changes

Cheers,

Robert.

[Robert Osfield]

I have added a std::mutex to  vsg::DescritproPool and usage of this in all the setting/getting of internal members and the vkDesctiproSet resources associated with it to handle the case where new DescriptorSet are being compiled by one thread at the same time as another thread is deleting older DescriptorSet  The changes are now merged with VSG master:

   https://github.com/vsg-dev/VulkanSceneGraph/pull/453