Re: [PATCH v5 0/4] MR292: vkd3d: Write Vulkan descriptors in a worker thread.

[Henri Verbeet (＠hverbeet)]

> +static HRESULT device_worker_stop(struct d3d12_device *device)
> +{
> +    HRESULT hr;
> +
> +    TRACE("device %p.\n", device);
> +
> +    vkd3d_mutex_lock(&device->worker_mutex);
> +
> +    device->worker_should_exit = true;
> +    vkd3d_cond_signal(&device->worker_cond);
> +
> +    vkd3d_mutex_unlock(&device->worker_mutex);
> +
> +    if (FAILED(hr = vkd3d_join_thread(device->vkd3d_instance, &device->worker_thread)))
> +        return hr;
> +
> +    vkd3d_mutex_destroy(&device->worker_mutex);
> +    vkd3d_cond_destroy(&device->worker_cond);
> +
> +    return S_OK;
> +}
>  ...
> +static void *device_worker_main(void *arg)
> +{
> +    struct d3d12_descriptor_heap *heap;
> +    struct d3d12_device *device = arg;
> +    size_t i;
> +
> +    vkd3d_set_thread_name("device_worker");
> +
> +    vkd3d_mutex_lock(&device->worker_mutex);
> +
> +    for (;;)
> +    {
> +        for (i = 0; i < device->heap_count && !device->worker_should_exit; ++i)
> +        {
> +            heap = device->heaps[i];
> +            if (heap->dirty_list_head == UINT_MAX)
> +                continue;
> +            vkd3d_mutex_lock(&heap->vk_sets_mutex);
> +            d3d12_desc_flush_vk_heap_updates_locked(heap, device);
> +            vkd3d_mutex_unlock(&heap->vk_sets_mutex);
> +        }
> +
> +        if (device->worker_should_exit)
> +            break;
> +
> +        vkd3d_cond_wait(&device->worker_cond, &device->worker_mutex);
> +    }
> +
> +    vkd3d_mutex_unlock(&device->worker_mutex);
> +
> +    return NULL;
> +}

Does it make sense to check device->worker_should_exit on each loop iteration above? device->worker_mutex should prevent device_worker_stop() from modifying device->worker_should_exit before the vkd3d_cond_wait() call, right?

I also still think it would be helpful to explain the problem we're trying to solve (i.e., descriptor writes getting delayed until command list submission, and then potentially becoming a bottleneck) somewhere in the actual commit. Perhaps as a comment in device_worker_main().