[PATCH v2 0/1] MR384: Draft: vkd3d: Introduce a thread-local descriptor object cache.

[Conor McCarthy (＠cmccarthy)]

A TLS index is allocated for each device object. We could use a global TLS, but that would share caches for all devices. I'm not sure if that's a problem. One index per device means freed indices must be recycled, which requires a global mutex, but a global TLS index would also need a global mutex.

This currently leaks cache memory when a device is freed. To fix this, allocations must be tracked. A global cache is more difficult to free, which is not normally an issue, but it is when using valgrind.

--
  v2: vkd3d: Replace the descriptor object cache with a thread-local implementation.
 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/384

[PATCH v2 1/1] vkd3d: Replace the descriptor object cache with a thread-local implementation.

[Conor McCarthy]

From: Conor McCarthy <cmccarthy(a)codeweavers.com>

---
 libs/vkd3d/device.c        |  98 ++++++++++++++++++++++++++++-----
 libs/vkd3d/resource.c      | 110 ++++++++++++++++++++-----------------
 libs/vkd3d/vkd3d_private.h |  78 +++++++++++++++++++++++---
 3 files changed, 213 insertions(+), 73 deletions(-)

diff --git a/libs/vkd3d/device.c b/libs/vkd3d/device.c
index 2d7051f3..ed0dc8aa 100644
--- a/libs/vkd3d/device.c
+++ b/libs/vkd3d/device.c
@@ -2434,28 +2434,88 @@ static void device_init_descriptor_pool_sizes(struct d3d12_device *device)
     device->vk_pool_count = 6;
 };
 
-static void vkd3d_desc_object_cache_init(struct vkd3d_desc_object_cache *cache, size_t size)
+static void vkd3d_desc_object_cache_init(struct vkd3d_desc_object_cache *cache, size_t size,
+        struct desc_rebalance *rebalance)
 {
-    memset(cache, 0, sizeof(*cache));
     cache->size = size;
+    cache->rebalance = rebalance;
 }
 
 static void vkd3d_desc_object_cache_cleanup(struct vkd3d_desc_object_cache *cache)
 {
-    union d3d12_desc_object u;
-    unsigned int i;
-    void *next;
+    size_t i;
+
+    for (i = 0; i < cache->count; ++i)
+        vkd3d_free(cache->data[i]);
+    vkd3d_free(cache->data);
+}
 
-    for (i = 0; i < ARRAY_SIZE(cache->heads); ++i)
+static void desc_rebalance_init(struct desc_rebalance *rebalance)
+{
+    vkd3d_mutex_init(&rebalance->mutex);
+}
+
+static void global_tls_key_destroy(void *value)
+{
+    struct desc_object_caches *caches = value;
+
+    if (caches)
     {
-        for (u.object = cache->heads[i].head; u.object; u.object = next)
-        {
-            next = u.header->next;
-            vkd3d_free(u.object);
-        }
+        vkd3d_desc_object_cache_cleanup(&caches->view_desc_cache);
+        vkd3d_desc_object_cache_cleanup(&caches->cbuffer_desc_cache);
+        vkd3d_free(caches);
     }
 }
 
+struct desc_rebalance view_desc_rebalance;
+struct desc_rebalance cbuffer_desc_rebalance;
+
+static int global_tls_key_get(struct vkd3d_tls_key *key)
+{
+    static struct vkd3d_mutex key_cache_mutex = VKD3D_MUTEX_INITIALIZER;
+    static struct vkd3d_tls_key tls_key;
+    static bool tls_initialised;
+    int rc = 0;
+
+    vkd3d_mutex_lock(&key_cache_mutex);
+
+    if (!tls_initialised)
+    {
+        rc = vkd3d_tls_key_create(&tls_key, global_tls_key_destroy);
+        desc_rebalance_init(&view_desc_rebalance);
+        desc_rebalance_init(&cbuffer_desc_rebalance);
+        tls_initialised = true;
+    }
+
+    vkd3d_mutex_unlock(&key_cache_mutex);
+
+    *key = tls_key;
+    return rc;
+}
+
+struct desc_object_caches *device_get_desc_object_caches(struct d3d12_device *device)
+{
+    struct desc_object_caches *caches = vkd3d_tls_key_get_value(&device->tls_key);
+
+    if (caches)
+        return caches;
+
+    if (!(caches = vkd3d_calloc(1, sizeof(*caches))))
+    {
+        ERR("Failed to allocate caches.\n");
+        /* Returning null here is not worth the cost of handling it in callers. */
+        abort();
+    }
+
+    vkd3d_desc_object_cache_init(&caches->view_desc_cache, sizeof(struct vkd3d_view), &view_desc_rebalance);
+    vkd3d_desc_object_cache_init(&caches->cbuffer_desc_cache, sizeof(struct vkd3d_cbuffer_desc),
+            &cbuffer_desc_rebalance);
+
+    vkd3d_tls_key_set_value(&device->tls_key, caches);
+
+    return caches;
+}
+
 /* ID3D12Device */
 static inline struct d3d12_device *impl_from_ID3D12Device5(ID3D12Device5 *iface)
 {
@@ -2520,8 +2580,6 @@ static ULONG STDMETHODCALLTYPE d3d12_device_Release(ID3D12Device5 *iface)
         vkd3d_render_pass_cache_cleanup(&device->render_pass_cache, device);
         d3d12_device_destroy_pipeline_cache(device);
         d3d12_device_destroy_vkd3d_queues(device);
-        vkd3d_desc_object_cache_cleanup(&device->view_desc_cache);
-        vkd3d_desc_object_cache_cleanup(&device->cbuffer_desc_cache);
         VK_CALL(vkDestroyDevice(device->vk_device, NULL));
         if (device->parent)
             IUnknown_Release(device->parent);
@@ -4234,6 +4292,7 @@ static HRESULT d3d12_device_init(struct d3d12_device *device,
 {
     const struct vkd3d_vk_device_procs *vk_procs;
     HRESULT hr;
+    int rc;
 
     device->ID3D12Device5_iface.lpVtbl = &d3d12_device_vtbl;
     device->refcount = 1;
@@ -4276,8 +4335,15 @@ static HRESULT d3d12_device_init(struct d3d12_device *device,
     device->blocked_queue_count = 0;
     vkd3d_mutex_init(&device->blocked_queues_mutex);
 
-    vkd3d_desc_object_cache_init(&device->view_desc_cache, sizeof(struct vkd3d_view));
-    vkd3d_desc_object_cache_init(&device->cbuffer_desc_cache, sizeof(struct vkd3d_cbuffer_desc));
+    /* Store a copy of the TLS key in the device object. The object is often accessed
+     * for other reasons after the key is read, so reading it is less likely to cause
+     * an additional cache miss than reading the global value would be. */
+    if ((rc = global_tls_key_get(&device->tls_key)) < 0)
+    {
+        ERR("Failed to allocate TLS key, rc %d\n.", rc);
+        hr = E_FAIL;
+        goto out_cleanup_descriptor_heap_layouts;
+    }
 
     device_init_descriptor_pool_sizes(device);
 
@@ -4286,6 +4352,8 @@ static HRESULT d3d12_device_init(struct d3d12_device *device,
 
     return S_OK;
 
+out_cleanup_descriptor_heap_layouts:
+    vkd3d_vk_descriptor_heap_layouts_cleanup(device);
 out_cleanup_uav_clear_state:
     vkd3d_uav_clear_state_cleanup(&device->uav_clear_state, device);
 out_destroy_null_resources:
diff --git a/libs/vkd3d/resource.c b/libs/vkd3d/resource.c
index 8898c247..fc8a719b 100644
--- a/libs/vkd3d/resource.c
+++ b/libs/vkd3d/resource.c
@@ -2290,72 +2290,77 @@ ULONG vkd3d_resource_decref(ID3D12Resource *resource)
     return d3d12_resource_decref(impl_from_ID3D12Resource(resource));
 }
 
-#define HEAD_INDEX_MASK (ARRAY_SIZE(cache->heads) - 1)
+static const unsigned int REBALANCE_SIZE = 64;
+/* Rebalance threshold expands by one extra REBALANCE_SIZE per (threshold / REBALANCE_DIVISOR). */
+static const unsigned int REBALANCE_DIVISOR = 512;
+/* Avoid some initial reallocations. */
+static const unsigned int REBALANCE_MIN_ALLOCATION = REBALANCE_SIZE * 16;
 
-/* Objects are cached so that vkd3d_view_incref() can safely check the refcount of an
- * object freed by another thread. This could be implemented as a single atomic linked
- * list, but it requires handling the ABA problem, which brings issues with cross-platform
- * support, compiler support, and non-universal x86-64 support for 128-bit CAS. */
 static void *vkd3d_desc_object_cache_get(struct vkd3d_desc_object_cache *cache)
 {
-    union d3d12_desc_object u;
-    unsigned int i;
+    struct desc_rebalance *rebalance;
+    void *object;
 
-    STATIC_ASSERT(!(ARRAY_SIZE(cache->heads) & HEAD_INDEX_MASK));
+    if (cache->count)
+        return cache->data[--cache->count];
 
-    i = (vkd3d_atomic_increment(&cache->next_index)) & HEAD_INDEX_MASK;
-    for (;;)
-    {
-        if (vkd3d_atomic_compare_exchange(&cache->heads[i].spinlock, 0, 1))
-        {
-            if ((u.object = cache->heads[i].head))
-            {
-                vkd3d_atomic_decrement(&cache->free_count);
-                cache->heads[i].head = u.header->next;
-                vkd3d_atomic_exchange(&cache->heads[i].spinlock, 0);
-                return u.object;
-            }
-            vkd3d_atomic_exchange(&cache->heads[i].spinlock, 0);
-        }
-        /* Keeping a free count avoids uncertainty over when this loop should terminate,
-         * which could result in excess allocations gradually increasing without limit. */
-        if (cache->free_count < ARRAY_SIZE(cache->heads))
-            return vkd3d_malloc(cache->size);
+    /* If the cache is empty then a previously sent rebalance chunk should not have been sent.
+     * Raise the threshold for sending chunks. */
+    cache->rebalance_threshold += cache->threshold_expansion;
+    cache->threshold_expansion = 0;
 
-        i = (i + 1) & HEAD_INDEX_MASK;
+    rebalance = cache->rebalance;
+    vkd3d_mutex_lock(&rebalance->mutex);
+
+    if (rebalance->count)
+    {
+        assert(rebalance->count >= REBALANCE_SIZE);
+        rebalance->count -= REBALANCE_SIZE;
+        vkd3d_array_reserve((void **)&cache->data, &cache->capacity, REBALANCE_SIZE, sizeof(*cache->data));
+        memcpy(cache->data, &rebalance->data[rebalance->count], REBALANCE_SIZE * sizeof(*cache->data));
+        cache->count = REBALANCE_SIZE - 1;
+        object = cache->data[REBALANCE_SIZE - 1];
+    }
+    else
+    {
+        object = vkd3d_malloc(cache->size);
     }
+
+    vkd3d_mutex_unlock(&rebalance->mutex);
+    return object;
 }
 
 static void vkd3d_desc_object_cache_push(struct vkd3d_desc_object_cache *cache, void *object)
 {
-    union d3d12_desc_object u = {object};
-    unsigned int i;
-    void *head;
+    struct desc_rebalance *rebalance;
 
-    /* Using the same index as above may result in a somewhat uneven distribution,
-     * but the main objective is to avoid costly spinlock contention. */
-    i = (vkd3d_atomic_increment(&cache->next_index)) & HEAD_INDEX_MASK;
-    for (;;)
-    {
-        if (vkd3d_atomic_compare_exchange(&cache->heads[i].spinlock, 0, 1))
-            break;
-        i = (i + 1) & HEAD_INDEX_MASK;
-    }
+    vkd3d_array_reserve((void **)&cache->data, &cache->capacity, max(cache->count + 1, REBALANCE_SIZE),
+            sizeof(*cache->data));
+    cache->data[cache->count++] = object;
 
-    head = cache->heads[i].head;
-    u.header->next = head;
-    cache->heads[i].head = u.object;
-    vkd3d_atomic_exchange(&cache->heads[i].spinlock, 0);
-    vkd3d_atomic_increment(&cache->free_count);
-}
+    if (cache->count < cache->rebalance_threshold + REBALANCE_SIZE)
+        return;
 
-#undef HEAD_INDEX_MASK
+    cache->count -= REBALANCE_SIZE;
+    cache->threshold_expansion = REBALANCE_SIZE * (1 + cache->rebalance_threshold / REBALANCE_DIVISOR);
+
+    rebalance = cache->rebalance;
+    vkd3d_mutex_lock(&rebalance->mutex);
+
+    vkd3d_array_reserve((void **)&rebalance->data, &rebalance->capacity,
+            max(rebalance->count + REBALANCE_SIZE, REBALANCE_MIN_ALLOCATION), sizeof(*rebalance->data));
+    memcpy(&rebalance->data[rebalance->count], &cache->data[cache->count], REBALANCE_SIZE * sizeof(*rebalance->data));
+    rebalance->count += REBALANCE_SIZE;
+
+    vkd3d_mutex_unlock(&rebalance->mutex);
+}
 
 static struct vkd3d_cbuffer_desc *vkd3d_cbuffer_desc_create(struct d3d12_device *device)
 {
+    struct desc_object_caches *caches = device_get_desc_object_caches(device);
     struct vkd3d_cbuffer_desc *desc;
 
-    if (!(desc = vkd3d_desc_object_cache_get(&device->cbuffer_desc_cache)))
+    if (!(desc = vkd3d_desc_object_cache_get(&caches->cbuffer_desc_cache)))
         return NULL;
 
     desc->h.magic = VKD3D_DESCRIPTOR_MAGIC_CBV;
@@ -2368,11 +2373,12 @@ static struct vkd3d_cbuffer_desc *vkd3d_cbuffer_desc_create(struct d3d12_device
 static struct vkd3d_view *vkd3d_view_create(uint32_t magic, VkDescriptorType vk_descriptor_type,
         enum vkd3d_view_type type, struct d3d12_device *device)
 {
+    struct desc_object_caches *caches = device_get_desc_object_caches(device);
     struct vkd3d_view *view;
 
     assert(magic);
 
-    if (!(view = vkd3d_desc_object_cache_get(&device->view_desc_cache)))
+    if (!(view = vkd3d_desc_object_cache_get(&caches->view_desc_cache)))
     {
         ERR("Failed to allocate descriptor object.\n");
         return NULL;
@@ -2390,6 +2396,7 @@ static struct vkd3d_view *vkd3d_view_create(uint32_t magic, VkDescriptorType vk_
 static void vkd3d_view_destroy(struct vkd3d_view *view, struct d3d12_device *device)
 {
     const struct vkd3d_vk_device_procs *vk_procs = &device->vk_procs;
+    struct desc_object_caches *caches = device_get_desc_object_caches(device);
 
     TRACE("Destroying view %p.\n", view);
 
@@ -2411,7 +2418,7 @@ static void vkd3d_view_destroy(struct vkd3d_view *view, struct d3d12_device *dev
     if (view->v.vk_counter_view)
         VK_CALL(vkDestroyBufferView(device->vk_device, view->v.vk_counter_view, NULL));
 
-    vkd3d_desc_object_cache_push(&device->view_desc_cache, view);
+    vkd3d_desc_object_cache_push(&caches->view_desc_cache, view);
 }
 
 void vkd3d_view_decref(void *view, struct d3d12_device *device)
@@ -2424,7 +2431,10 @@ void vkd3d_view_decref(void *view, struct d3d12_device *device)
     if (u.header->magic != VKD3D_DESCRIPTOR_MAGIC_CBV)
         vkd3d_view_destroy(u.view, device);
     else
-        vkd3d_desc_object_cache_push(&device->cbuffer_desc_cache, u.object);
+    {
+        struct desc_object_caches *caches = device_get_desc_object_caches(device);
+        vkd3d_desc_object_cache_push(&caches->cbuffer_desc_cache, u.object);
+    }
 }
 
 static inline void d3d12_desc_replace(struct d3d12_desc *dst, void *view, struct d3d12_device *device)
diff --git a/libs/vkd3d/vkd3d_private.h b/libs/vkd3d/vkd3d_private.h
index 2e9845df..97fa6b11 100644
--- a/libs/vkd3d/vkd3d_private.h
+++ b/libs/vkd3d/vkd3d_private.h
@@ -205,11 +205,18 @@ struct vkd3d_mutex
     CRITICAL_SECTION lock;
 };
 
+#define VKD3D_MUTEX_INITIALIZER {NULL, -1, 0, 0, 0, 0}
+
 struct vkd3d_cond
 {
     CONDITION_VARIABLE cond;
 };
 
+struct vkd3d_tls_key
+{
+    unsigned int key;
+};
+
 static inline void vkd3d_mutex_init(struct vkd3d_mutex *lock)
 {
     InitializeCriticalSection(&lock->lock);
@@ -255,6 +262,26 @@ static inline void vkd3d_cond_destroy(struct vkd3d_cond *cond)
 {
 }
 
+typedef PFLS_CALLBACK_FUNCTION vkd3d_tls_destructor_fn;
+
+static inline int vkd3d_tls_key_create(struct vkd3d_tls_key *key, vkd3d_tls_destructor_fn destructor)
+{
+    if ((key->key = FlsAlloc(destructor)) == FLS_OUT_OF_INDEXES)
+        return -1;
+    return 0;
+}
+
+static inline int vkd3d_tls_key_set_value(const struct vkd3d_tls_key *key, void *value)
+{
+    FlsSetValue(key->key, value);
+    return 0;
+}
+
+static inline void *vkd3d_tls_key_get_value(const struct vkd3d_tls_key *key)
+{
+    return FlsGetValue(key->key);
+}
+
 static inline unsigned int vkd3d_atomic_increment(unsigned int volatile *x)
 {
     return InterlockedIncrement((LONG volatile *)x);
@@ -300,11 +327,17 @@ struct vkd3d_mutex
     pthread_mutex_t lock;
 };
 
+#define VKD3D_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER
+
 struct vkd3d_cond
 {
     pthread_cond_t cond;
 };
 
+struct vkd3d_tls_key
+{
+    pthread_key_t key;
+};
 
 static inline void vkd3d_mutex_init(struct vkd3d_mutex *lock)
 {
@@ -387,6 +420,23 @@ static inline void vkd3d_cond_destroy(struct vkd3d_cond *cond)
         ERR("Could not destroy the condition variable, error %d.\n", ret);
 }
 
+typedef void (*vkd3d_tls_destructor_fn)(void *);
+
+static inline int vkd3d_tls_key_create(struct vkd3d_tls_key *key, vkd3d_tls_destructor_fn destructor)
+{
+    return pthread_key_create(&key->key, destructor);
+}
+
+static inline int vkd3d_tls_key_set_value(const struct vkd3d_tls_key *key, void *value)
+{
+    return pthread_setspecific(key->key, value);
+}
+
+static inline void *vkd3d_tls_key_get_value(const struct vkd3d_tls_key *key)
+{
+    return pthread_getspecific(key->key);
+}
+
 # if HAVE_SYNC_SUB_AND_FETCH
 static inline unsigned int vkd3d_atomic_decrement(unsigned int volatile *x)
 {
@@ -1694,20 +1744,31 @@ struct vkd3d_uav_clear_state
 HRESULT vkd3d_uav_clear_state_init(struct vkd3d_uav_clear_state *state, struct d3d12_device *device);
 void vkd3d_uav_clear_state_cleanup(struct vkd3d_uav_clear_state *state, struct d3d12_device *device);
 
-struct desc_object_cache_head
+struct desc_rebalance
 {
-    void *head;
-    unsigned int spinlock;
+    void **data;
+    size_t capacity;
+    size_t count;
+    struct vkd3d_mutex mutex;
 };
 
 struct vkd3d_desc_object_cache
 {
-    struct desc_object_cache_head heads[16];
-    unsigned int next_index;
-    unsigned int free_count;
+    void **data;
+    size_t capacity;
+    size_t count;
+    size_t rebalance_threshold;
+    size_t threshold_expansion;
+    struct desc_rebalance *rebalance;
     size_t size;
 };
 
+struct desc_object_caches
+{
+    struct vkd3d_desc_object_cache view_desc_cache;
+    struct vkd3d_desc_object_cache cbuffer_desc_cache;
+};
+
 #define VKD3D_DESCRIPTOR_POOL_COUNT 6
 
 /* ID3D12Device */
@@ -1722,11 +1783,11 @@ struct d3d12_device
     PFN_vkd3d_signal_event signal_event;
     size_t wchar_size;
 
+    struct vkd3d_tls_key tls_key;
+
     struct vkd3d_gpu_va_allocator gpu_va_allocator;
 
     struct vkd3d_mutex mutex;
-    struct vkd3d_desc_object_cache view_desc_cache;
-    struct vkd3d_desc_object_cache cbuffer_desc_cache;
     struct vkd3d_render_pass_cache render_pass_cache;
     VkPipelineCache vk_pipeline_cache;
 
@@ -1779,6 +1840,7 @@ struct vkd3d_queue *d3d12_device_get_vkd3d_queue(struct d3d12_device *device, D3
 bool d3d12_device_is_uma(struct d3d12_device *device, bool *coherent);
 void d3d12_device_mark_as_removed(struct d3d12_device *device, HRESULT reason,
         const char *message, ...) VKD3D_PRINTF_FUNC(3, 4);
+struct desc_object_caches *device_get_desc_object_caches(struct d3d12_device *device);
 struct d3d12_device *unsafe_impl_from_ID3D12Device5(ID3D12Device5 *iface);
 
 static inline HRESULT d3d12_device_query_interface(struct d3d12_device *device, REFIID iid, void **object)
-- 
GitLab

https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/384