Skip to content

igl | vulkan | Reuse the cached VkDescriptorSet to reduce vkAllocateDescriptorSets(). #336

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.

Sign up for GitHub

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

Open
vinsentli wants to merge 2 commits into
base: main
Choose a base branch
from
+17 −3
Open

igl | vulkan | Reuse the cached VkDescriptorSet to reduce vkAllocateDescriptorSets(). #336

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2691669
igl | vulkan | Reuse the cached VkDescriptorSet to reduce vkAllocateD…
vinsentli Jan 10, 2026
5bd8ab5
code review
vinsentli Jan 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions src/igl/vulkan/VulkanContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,12 @@ class DescriptorPoolsArena final {
if (!numRemainingDSetsInPool_) {
switchToNewDescriptorPool(ic, nextSubmitHandle);
}
VK_ASSERT(ivkAllocateDescriptorSet(&ctx_.vf_, device_, pool_, dsl_, &dset));
if (isNewPool_) {
VK_ASSERT(ivkAllocateDescriptorSet(&ctx_.vf_, device_, pool_, dsl_, &dset));
allocatedDSet_.emplace_back(dset);
} else {
dset = allocatedDSet_[dsetCursor_++];
Copy link
Contributor

@corporateshark corporateshark Jan 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Potential out-of-bounds access when reusing a descriptor pool. When a pool is reused (line 277), allocatedDSet_ is restored from the extinct pool, but numRemainingDSetsInPool_ is always reset to kNumDSetsPerPool (64) at line 266. If the pool was previously switched before all 64 descriptor sets were allocated, allocatedDSet_ will have fewer than 64 elements. Subsequent calls to getNextDescriptorSet() could then access allocatedDSet_[dsetCursor_++] at line 257 beyond the vector's bounds. Consider either: (1) setting numRemainingDSetsInPool_ based on the actual size of the restored allocatedDSet_ vector, or (2) ensuring that pools always have exactly kNumDSetsPerPool descriptor sets allocated before being marked as extinct.

Copy link
Contributor Author

@vinsentli vinsentli Jan 14, 2026

Choose a reason for hiding this comment

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

Are you encountering an out-of-bounds access issue? According to line 249, a new descriptor pool will not switch to another pool before all 64 descriptors have been allocated. I have not encountered any out-of-bounds access issue.

    if (!numRemainingDSetsInPool_) {
      switchToNewDescriptorPool(ic, nextSubmitHandle);
    }

}
numRemainingDSetsInPool_--;
return dset;
}
Expand All @@ -260,16 +265,18 @@ class DescriptorPoolsArena final {
numRemainingDSetsInPool_ = kNumDSetsPerPool;

if (pool_ != VK_NULL_HANDLE) {
extinct_.push_back({pool_, nextSubmitHandle});
extinct_.push_back({pool_, nextSubmitHandle, allocatedDSet_});
}
// first, let's try to reuse the oldest extinct pool (never reuse pools that are tagged with the
// same SubmitHandle because they have not yet been submitted)
if (extinct_.size() > 1 && extinct_.front().handle != nextSubmitHandle) {
const ExtinctDescriptorPool p = extinct_.front();
if (ic.isReady(p.handle)) {
pool_ = p.pool;
allocatedDSet_ = p.allocatedDSet;
dsetCursor_ = 0;
isNewPool_ = false;
extinct_.pop_front();
VK_ASSERT(ctx_.vf_.vkResetDescriptorPool(device_, pool_, VkDescriptorPoolResetFlags{}));
return;
}
}
Expand All @@ -288,6 +295,9 @@ class DescriptorPoolsArena final {
&pool_));
VK_ASSERT(ivkSetDebugObjectName(
&ctx_.vf_, device_, VK_OBJECT_TYPE_DESCRIPTOR_POOL, (uint64_t)pool_, dpDebugName_.c_str()));
allocatedDSet_.clear();
dsetCursor_ = 0;
isNewPool_ = true;
}

private:
Expand All @@ -296,6 +306,9 @@ class DescriptorPoolsArena final {
const VulkanContext& ctx_;
VkDevice device_ = VK_NULL_HANDLE;
VkDescriptorPool pool_ = VK_NULL_HANDLE;
uint32_t dsetCursor_ = 0;
std::vector<VkDescriptorSet> allocatedDSet_;
bool isNewPool_ = true; // is a new created pool or an old cached pool
const uint32_t numTypes_ = 0;
VkDescriptorType types_[2] = {VK_DESCRIPTOR_TYPE_MAX_ENUM, VK_DESCRIPTOR_TYPE_MAX_ENUM};
const uint32_t numDescriptorsPerDSet_ = 0;
Expand All @@ -307,6 +320,7 @@ class DescriptorPoolsArena final {
struct ExtinctDescriptorPool {
VkDescriptorPool pool = VK_NULL_HANDLE;
VulkanImmediateCommands::SubmitHandle handle = {};
std::vector<VkDescriptorSet> allocatedDSet;
};

std::deque<ExtinctDescriptorPool> extinct_;
Expand Down
Loading