Batch resources initialization #2081
Conversation
|
CI gfxreconstruct build queued with queue ID 398542. |
panos-lunarg
force-pushed
the
batch_resources_initialization
branch
from
8085ffa to
fba1e95
Compare
|
CI gfxreconstruct build queued with queue ID 398544. |
panos-lunarg
force-pushed
the
batch_resources_initialization
branch
from
fba1e95 to
b87d9ea
Compare
|
CI gfxreconstruct build queued with queue ID 398549. |
|
CI gfxreconstruct build # 6367 running. |
|
CI gfxreconstruct build # 6367 failed. |
panos-lunarg
force-pushed
the
batch_resources_initialization
branch
from
b87d9ea to
ff46ca2
Compare
|
CI gfxreconstruct build queued with queue ID 398648. |
|
CI gfxreconstruct build # 6368 running. |
|
CI gfxreconstruct build # 6368 failed. |
panos-lunarg
force-pushed
the
batch_resources_initialization
branch
from
ff46ca2 to
552a3f4
Compare
|
CI gfxreconstruct build queued with queue ID 398739. |
|
CI gfxreconstruct build # 6369 running. |
|
CI gfxreconstruct build # 6369 passed. |
panos-lunarg
force-pushed
the
batch_resources_initialization
branch
from
552a3f4 to
dc8a0b3
Compare
|
CI gfxreconstruct build queued with queue ID 398888. |
|
CI gfxreconstruct build # 6370 running. |
|
CI gfxreconstruct build # 6370 passed. |
panos-lunarg
force-pushed
the
batch_resources_initialization
branch
from
dc8a0b3 to
fa940da
Compare
|
CI gfxreconstruct build queued with queue ID 399452. |
|
CI gfxreconstruct build # 6375 running. |
|
CI gfxreconstruct build # 6375 passed. |
panos-lunarg
force-pushed
the
batch_resources_initialization
branch
from
fa940da to
829393e
Compare
|
CI gfxreconstruct build queued with queue ID 399670. |
|
CI gfxreconstruct build # 6378 running. |
|
CI gfxreconstruct build # 6378 passed. |
panos-lunarg
force-pushed
the
batch_resources_initialization
branch
from
829393e to
b2a3d6d
Compare
|
CI gfxreconstruct build queued with queue ID 400745. |
|
CI gfxreconstruct build queued with queue ID 404029. |
|
CI gfxreconstruct build # 6403 running. |
|
CI gfxreconstruct build # 6403 failed. |
|
CI gfxreconstruct build queued with queue ID 404378. |
|
CI gfxreconstruct build # 404378 cancelled. |
| VkImageLayout final_layout, | ||
| uint32_t layer_count, | ||
| uint32_t level_count, | ||
| VkBufferImageCopy* level_copies) |
There was a problem hiding this comment.
Because this change alters the contract with the caller, I'd prefer to leave this const and then make a local copy with the offset, if that doesn't have a big performance impact. Maybe std::vector<VkBufferImageCopy> offsetted_level_copies(level_copies, level_copies, level_count); around line 322 and then add offset in the loop. Same for the change below to InitializeBuffer.
There was a problem hiding this comment.
This could be a member variable that is cleared and filled again from the passed array to reduce heap allocation costs.
There was a problem hiding this comment.
I recommend clear and then insert the elements into the vector rather than keep resizeing and std::copying it. The storage for vectors is never reduced so a clear and resize should pretty quickly reach steady state on the storage of the vector. The danger with resize and copy is that 1) if someone else modifies this code and doesn't change the resize, the vector could be undersized and the copy could overwrite memory.
There was a problem hiding this comment.
Right. I was under the impression that resize will shrink the vector's capacity if its current size is larger than the requested, but that's not the case. Your suggestion should work better
There was a problem hiding this comment.
Should be addressed now. I think clear can be skipped and just resize and overwrite over existing entries.
| begin_info.flags = 0; | ||
| begin_info.pInheritanceInfo = nullptr; | ||
|
|
||
| result = device_table_->BeginCommandBuffer(iter->second.command_buffer, &begin_info); |
There was a problem hiding this comment.
Is a failure fatal here and other places that operate on command buffers in this PR?
There was a problem hiding this comment.
What do you mean? In general I tried to do error checking. Have you spotted something I missed?
There was a problem hiding this comment.
I just mean, you're checking the return from BeginCommandBuffer here - if it can return something other than Vk_SUCCESS here, it seems like we should print an error and maybe a fatal error. Unless there's no realistic way it can ever fail. But I can imagine maybe we could get VK_ERROR_OUT_OF_HOST_MEMORY; do you think we should check for errors?
There was a problem hiding this comment.
In general I followed the error checking logic that is applied in the rest of the file. The callers of this function will check the return value and act accordingly which is basically to propagate the error higher.
VulkanResourceInitializer exposes 4 public functions. The user of the class (the vulkan replay consumer) checks the return value and prints an error if there is one.
| { | ||
| FlushStagingBuffer(); | ||
| VkResult result = VK_SUCCESS; | ||
| for (auto& exec_object : command_exec_objects_) |
There was a problem hiding this comment.
Can this be const and what other additions in this PR can be const?
panos-lunarg
force-pushed
the
batch_resources_initialization
branch
from
3650483 to
d4d17df
Compare
|
CI gfxreconstruct build queued with queue ID 404983. |
|
CI gfxreconstruct build # 6421 running. |
|
CI gfxreconstruct build # 6421 failed. |
panos-lunarg
force-pushed
the
batch_resources_initialization
branch
from
d4d17df to
78c60d7
Compare
|
CI gfxreconstruct build queued with queue ID 405323. |
|
CI gfxreconstruct build # 6427 running. |
|
CI gfxreconstruct build # 6427 passed. |
panos-lunarg
force-pushed
the
batch_resources_initialization
branch
from
78c60d7 to
8bda62d
Compare
|
CI gfxreconstruct build queued with queue ID 406010. |
|
CI gfxreconstruct build # 6435 running. |
|
CI gfxreconstruct build # 6435 passed. |
panos-lunarg
force-pushed
the
batch_resources_initialization
branch
from
8bda62d to
1be2cba
Compare
|
CI gfxreconstruct build queued with queue ID 414365. |
|
CI gfxreconstruct build # 6559 running. |
|
CI gfxreconstruct build # 6559 passed. |
AcquireInitializedStagingBuffer will call LoadData so InitializeImage does not need to explicitly call it again.
Instead of uploading each resource with a separate queue submission, batch several resources into a larger staging buffer and upload them with a single queue submit
panos-lunarg
force-pushed
the
batch_resources_initialization
branch
from
1be2cba to
3a33f75
Compare
|
CI gfxreconstruct build queued with queue ID 416925. |
|
CI gfxreconstruct build # 6580 running. |
|
CI gfxreconstruct build # 6580 passed. |
Instead of uploading each resource with a separate queue submission,
batch several resources into a larger staging buffer and upload them
with a single queue submit