Re: [PATCH vkd3d 3/4] vkd3d: Replace the signaled semaphore list with a resizable array.

[Henri Verbeet]

On Tue, 26 Apr 2022 at 15:48, Conor McCarthy  wrote:
> @@ -590,18 +590,19 @@ static void d3d12_fence_garbage_collect_vk_semaphores_locked(struct d3d12_fence
>  {
>      struct d3d12_device *device = fence->device;
>      const struct vkd3d_vk_device_procs *vk_procs = &device->vk_procs;
> - struct vkd3d_signaled_semaphore *current, *p;
> - unsigned int semaphore_count;
> -
>
> struct vkd3d_signaled_semaphore *current;
> -
>
> unsigned int i, semaphore_count;
>
> semaphore_count = fence->semaphore_count;
> if (!destroy_all && semaphore_count < VKD3D_MAX_VK_SYNC_OBJECTS)
>     return;
> - LIST_FOR_EACH_ENTRY_SAFE(current, p, &fence->semaphores, struct vkd3d_signaled_semaphore, entry)
> -
>
> for (i = 0; i < fence->semaphore_count; ++i)
> {
>     if (!destroy_all && fence->semaphore_count < VKD3D_MAX_VK_SYNC_OBJECTS)
>         break;
> -
>    current = &fence->semaphores[i];
>    /* The semaphore doesn't have a pending signal operation if the fence
>     * was signaled. */
>    if ((current->vk_fence || current->is_acquired) && !destroy_all)
>
> @@ -612,10 +613,7 @@ static void d3d12_fence_garbage_collect_vk_semaphores_locked(struct d3d12_fence
>          assert(!current->is_acquired);
>      VK_CALL(vkDestroySemaphore(device->vk_device, current->vk_semaphore, NULL));
> -
>    list_remove(&current->entry);
> -
>    vkd3d_free(current);
> -
> -
>    --fence->semaphore_count;
> -
>    fence->semaphores[i--] = fence->semaphores[--fence->semaphore_count];
> }

I suppose that works, although decrementing "i" here only to have the
loop control increment it again seems a bit awkward; more so for the
case where "i == 0". It seems tempting to suggest using a while-loop
here, only incrementing "i" when we take the "continue" path, and then
using "*current = fence->semaphores[--fence->semaphore_count];" for
the last line of the block.
> @@ -703,10 +704,8 @@ static void d3d12_fence_remove_vk_semaphore(struct d3d12_fence *fence, struct vk
>  assert(semaphore->is_acquired);
>
> - list_remove(&semaphore->entry);
> - vkd3d_free(semaphore);
> -
> - --fence->semaphore_count;
> - i = semaphore - fence->semaphores;
> - fence->semaphores[i] = fence->semaphores[--fence->semaphore_count];

I.e., "*semaphore = fence->semaphores[--fence->semaphore_count];", right?