[PATCH] wined3d: Prevent buildup of retired buffer objects' memory.
Submit current command buffer in wined3d_context_vk_get_command_buffer() and return a new one if the total size of retired buffer objects on that command buffer crosses a predefined threshold. Signed-off-by: Jan Sikorski <jsikorski(a)codeweavers.com> --- dlls/wined3d/context_vk.c | 15 ++++++++++++--- dlls/wined3d/wined3d_private.h | 2 ++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/dlls/wined3d/context_vk.c b/dlls/wined3d/context_vk.c index 45133eabb69..89706d455f5 100644 --- a/dlls/wined3d/context_vk.c +++ b/dlls/wined3d/context_vk.c @@ -921,6 +921,9 @@ void wined3d_context_vk_destroy_bo(struct wined3d_context_vk *context_vk, const if (bo->map_ptr) VK_CALL(vkUnmapMemory(device_vk->vk_device, bo->vk_memory)); wined3d_context_vk_destroy_vk_memory(context_vk, bo->vk_memory, bo->command_buffer_id); + + if (bo->command_buffer_id == context_vk->current_command_buffer.id) + context_vk->retired_bo_counter += bo->size; } void wined3d_context_vk_poll_command_buffers(struct wined3d_context_vk *context_vk) @@ -1509,9 +1512,14 @@ VkCommandBuffer wined3d_context_vk_get_command_buffer(struct wined3d_context_vk buffer = &context_vk->current_command_buffer; if (buffer->vk_command_buffer) { - TRACE("Returning existing command buffer %p with id 0x%s.\n", - buffer->vk_command_buffer, wine_dbgstr_longlong(buffer->id)); - return buffer->vk_command_buffer; + if (context_vk->retired_bo_counter > WINED3D_RETIRED_BO_COUNTER_THRESHOLD) + wined3d_context_vk_submit_command_buffer(context_vk, 0, NULL, NULL, 0, NULL); + else + { + TRACE("Returning existing command buffer %p with id 0x%s.\n", + buffer->vk_command_buffer, wine_dbgstr_longlong(buffer->id)); + return buffer->vk_command_buffer; + } } command_buffer_info.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO; @@ -1628,6 +1636,7 @@ void wined3d_context_vk_submit_command_buffer(struct wined3d_context_vk *context context_vk->completed_command_buffer_id = 0; buffer->id = 1; } + context_vk->retired_bo_counter = 0; wined3d_context_vk_cleanup_resources(context_vk); } diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index c3e3752ed71..694436ddad2 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -2549,6 +2549,8 @@ struct wined3d_context_vk VkCommandPool vk_command_pool; struct wined3d_command_buffer_vk current_command_buffer; uint64_t completed_command_buffer_id; +#define WINED3D_RETIRED_BO_COUNTER_THRESHOLD (64 << 20) + size_t retired_bo_counter; struct { -- 2.31.0
On Tue, 20 Apr 2021 at 11:37, Jan Sikorski <jsikorski(a)codeweavers.com> wrote:
@@ -921,6 +921,9 @@ void wined3d_context_vk_destroy_bo(struct wined3d_context_vk *context_vk, const if (bo->map_ptr) VK_CALL(vkUnmapMemory(device_vk->vk_device, bo->vk_memory)); wined3d_context_vk_destroy_vk_memory(context_vk, bo->vk_memory, bo->command_buffer_id); + + if (bo->command_buffer_id == context_vk->current_command_buffer.id) + context_vk->retired_bo_counter += bo->size; } Should we guard "retired_bo_counter" against overflow? On 32-bit builds that has a maximum of 4GiB, which is perhaps not as much as it once was in terms of VRAM.
diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index c3e3752ed71..694436ddad2 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -2549,6 +2549,8 @@ struct wined3d_context_vk VkCommandPool vk_command_pool; struct wined3d_command_buffer_vk current_command_buffer; uint64_t completed_command_buffer_id; +#define WINED3D_RETIRED_BO_COUNTER_THRESHOLD (64 << 20) + size_t retired_bo_counter;
I'd prefer not having the WINED3D_RETIRED_BO_COUNTER_THRESHOLD #define in the middle of a structure. Perhaps next to the other allocator constants (i.e., WINED3D_ALLOCATOR_CHUNK_SIZE and the like) would be a good place. I think "retired_bo_counter" is perhaps a little awkward, in the sense that it doesn't count the number of retired bo's, but tracks their size; "retired_bo_size", perhaps? In terms of the commit message, it's perhaps helpful to mention that one way this can occur is by the application doing resource updates without flushing either explicitly or implicitly, causing retired staging bo's for the current command buffer to accumulate.
On 20 Apr 2021, at 15:30, Henri Verbeet <hverbeet(a)gmail.com> wrote:
On Tue, 20 Apr 2021 at 11:37, Jan Sikorski <jsikorski(a)codeweavers.com> wrote:
@@ -921,6 +921,9 @@ void wined3d_context_vk_destroy_bo(struct wined3d_context_vk *context_vk, const if (bo->map_ptr) VK_CALL(vkUnmapMemory(device_vk->vk_device, bo->vk_memory)); wined3d_context_vk_destroy_vk_memory(context_vk, bo->vk_memory, bo->command_buffer_id); + + if (bo->command_buffer_id == context_vk->current_command_buffer.id) + context_vk->retired_bo_counter += bo->size; } Should we guard "retired_bo_counter" against overflow? On 32-bit builds that has a maximum of 4GiB, which is perhaps not as much as it once was in terms of VRAM.
Right, that wouldn’t hurt even if it’s not that likely to happen, would you mind if I simply changed it to VkDeviceSize instead of doing overflow checks?
diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index c3e3752ed71..694436ddad2 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -2549,6 +2549,8 @@ struct wined3d_context_vk VkCommandPool vk_command_pool; struct wined3d_command_buffer_vk current_command_buffer; uint64_t completed_command_buffer_id; +#define WINED3D_RETIRED_BO_COUNTER_THRESHOLD (64 << 20) + size_t retired_bo_counter;
I'd prefer not having the WINED3D_RETIRED_BO_COUNTER_THRESHOLD #define in the middle of a structure. Perhaps next to the other allocator constants (i.e., WINED3D_ALLOCATOR_CHUNK_SIZE and the like) would be a good place. I think "retired_bo_counter" is perhaps a little awkward, in the sense that it doesn't count the number of retired bo's, but tracks their size; "retired_bo_size", perhaps?
In terms of the commit message, it's perhaps helpful to mention that one way this can occur is by the application doing resource updates without flushing either explicitly or implicitly, causing retired staging bo's for the current command buffer to accumulate.
Sure
On Tue, 20 Apr 2021 at 16:10, Jan Sikorski <jsikorski(a)codeweavers.com> wrote:
On 20 Apr 2021, at 15:30, Henri Verbeet <hverbeet(a)gmail.com> wrote: On Tue, 20 Apr 2021 at 11:37, Jan Sikorski <jsikorski(a)codeweavers.com> wrote:
@@ -921,6 +921,9 @@ void wined3d_context_vk_destroy_bo(struct wined3d_context_vk *context_vk, const if (bo->map_ptr) VK_CALL(vkUnmapMemory(device_vk->vk_device, bo->vk_memory)); wined3d_context_vk_destroy_vk_memory(context_vk, bo->vk_memory, bo->command_buffer_id); + + if (bo->command_buffer_id == context_vk->current_command_buffer.id) + context_vk->retired_bo_counter += bo->size; } Should we guard "retired_bo_counter" against overflow? On 32-bit builds that has a maximum of 4GiB, which is perhaps not as much as it once was in terms of VRAM.
Right, that wouldn’t hurt even if it’s not that likely to happen, would you mind if I simply changed it to VkDeviceSize instead of doing overflow checks?
I think that's fine too.
participants (2)
-
Henri Verbeet -
Jan Sikorski