vk: add resource ref-counting alternative #8220
Conversation
|
|
||
| void destroy(ResourceType type, HandleId id); | ||
|
|
||
| HandleId id; // 8 |
There was a problem hiding this comment.
HandleId is a uint32_t
There was a problem hiding this comment.
Also I wonder why the object needs its own handle?
There was a problem hiding this comment.
HandleId is a uint32_t
corrected.
Also I wonder why the object needs its own handle?
Because by the time the count reaches 0, I no longer have access to the handle, and I need it in order to destroy. Would it preferable to keep it in resource_ptr ? I can pass it in as dec(HandleId id). That would increase the size overall (multiple references to one handle object) but reduce the increase to the Handle objects.
There was a problem hiding this comment.
Okay, it's probably okay to keep it where you have it, because due to padding, you'll always have space for it I think.
| } | ||
|
|
||
| template<bool THREAD_SAFE = false> | ||
| struct ResourceImpl { |
There was a problem hiding this comment.
is it all backend handles that now will inherit from ResourceImpl or only those that need reference counting? (Asking because the size overhead is not trivial).
There was a problem hiding this comment.
yes, for simplicity all backend handles will inherit from ResourceImpl. With the previous change where I moved bits from VulkanRenderTarget (the largest handle) to the heap, we might be ok with this increase.
There was a problem hiding this comment.
but that means that all handles will increase by at least 16 bytes. I think 8 bytes is the minimum (the cound + some meta data). What is really sad is to have to store a pointer. Would it be too painful to pass it around everywhere it's needed? Or maybe we can think of doing that in another change?
There was a problem hiding this comment.
Yeah, I think for the pointer to the manager, we can definitely revisit.
poweifeng
force-pushed
the
pf/vk-add-ref-count-resource
branch
from
956a0ef
to
0bbea89
Compare
We add a set of classes to the vk backend that will enable cleaner ref-counting. In particular, all allocation from the HandleAllocator will be wrapped within a resource_ptr<> smart pointer. This struct will maintain a count that will increment/decrement with respect to references (similar to std::shared_ptr). This commit only introduces the new classes/structs and does not actually make any functional difference. A follow-up commit will make the switch to use the smart pointer. We also put VulkanFence and VulkanTimerQuery in a different header because we will need to depend on them separately in the follow-up (reason: they are accessed on the backend thread but allocated on "sync"/filament thread).
poweifeng
force-pushed
the
pf/vk-add-ref-count-resource
branch
from
f3a7311
to
e4825e1
Compare
Continuing from PR #8220. Here we change all of the references from the old ref-counting ways to the new ref-counting structs and mechanisms. There should be no functional change.
Continuing from PR #8220. Here we change all of the references from the old ref-counting ways to the new ref-counting structs and mechanisms. There should be no functional change.
Continuing from PR #8220. Here we change all of the references from the old ref-counting ways to the new ref-counting structs and mechanisms. There should be no functional change.
Continuing from PR #8220. Here we change all of the references from the old ref-counting ways to the new ref-counting structs and mechanisms. There should be no functional change.
Continuing from PR #8220. Here we change all of the references from the old ref-counting ways to the new ref-counting structs and mechanisms. There should be no functional change.
Continuing from PR #8220. Here we change all of the references from the old ref-counting ways to the new ref-counting structs and mechanisms. There should be no functional change.
We add a set of classes to the vk backend that will enable cleaner ref-counting.
In particular, all allocation from the HandleAllocator will be wrapped within a resource_ptr<> smart pointer. This struct will maintain a count that will increment/decrement with respect to references (similar to std::shared_ptr).
This commit only introduces the new classes/structs and does not actually make any functional difference. A follow-up commit will make the switch to use the smart pointer.
We also put VulkanFence and VulkanTimerQuery in a different header because we will need to depend on them separately in the follow-up (reason: they are accessed on the backend thread but allocated on "sync"/filament thread).