tqchen commented on a change in pull request #7833:
URL: https://github.com/apache/tvm/pull/7833#discussion_r612757068
##########
File path: src/runtime/vulkan/vulkan.cc
##########
@@ -60,35 +91,21 @@ class VulkanThreadEntry {
pool.reset();
streams_.clear();
for (const auto& kv : staging_buffers_) {
- if (!kv.second) {
- continue;
- }
- auto& buf = *(kv.second);
- if (buf.host_addr != nullptr) {
- vkUnmapMemory(buf.device, buf.memory);
- }
- if (buf.memory != VK_NULL_HANDLE) {
- vkFreeMemory(buf.device, buf.memory, nullptr);
- }
- if (buf.buffer != VK_NULL_HANDLE) {
- vkDestroyBuffer(buf.device, buf.buffer, nullptr);
- }
+ DeleteHostVisibleBuffer(kv.second.get());
Review comment:
The main point is that we only need to synchronize when re-allocation
happens(otherwise we might have deferred operation on the previous ubo buffers
that did not get executed and we already deleted the buffer).
The previous staging buffer avoid this problem by synchronize at copy
point(so such problem won't occur). In the case of UBO we don't want to
synchronize as frequent since we want to keep things as async as possible.
When we get to a stable state of the UBO staging buffer being big enough,
then we won't need to synchronize.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
