From 0e6ef05878f3629021fa9b7c5c9dc60897ec6647 Mon Sep 17 00:00:00 2001 From: Mike Blumenkrantz Date: Fri, 2 Oct 2020 13:22:41 -0400 Subject: [PATCH] zink: move descriptor sets/pools from batches to programs this lets us much more accurately create descriptor sets using the exact size required by a given program instead of creating gigantic monolithic sets it does (temporarily) incur a perf hit since sets are now freed after each use rather than being reset Reviewed-by: Bas Nieuwenhuizen Part-of: --- src/gallium/drivers/zink/zink_batch.c | 31 +++++++++++++---- src/gallium/drivers/zink/zink_batch.h | 8 +++-- src/gallium/drivers/zink/zink_context.c | 24 ++------------ src/gallium/drivers/zink/zink_context.h | 2 ++ src/gallium/drivers/zink/zink_draw.c | 21 ++++++------ src/gallium/drivers/zink/zink_program.c | 44 ++++++++++++++++++++++--- src/gallium/drivers/zink/zink_program.h | 5 +++ 7 files changed, 89 insertions(+), 46 deletions(-) diff --git a/src/gallium/drivers/zink/zink_batch.c b/src/gallium/drivers/zink/zink_batch.c index 406febec047..3dcf3fee25c 100644 --- a/src/gallium/drivers/zink/zink_batch.c +++ b/src/gallium/drivers/zink/zink_batch.c @@ -50,7 +50,14 @@ zink_reset_batch(struct zink_context *ctx, struct zink_batch *batch) util_dynarray_clear(&batch->zombie_samplers); util_dynarray_clear(&batch->persistent_resources); - set_foreach(batch->programs, entry) { + hash_table_foreach(batch->programs, entry) { + struct zink_program *pg = (struct zink_program*)entry->key; + struct set *desc_sets = (struct set*)entry->data; + set_foreach(desc_sets, sentry) { + VkDescriptorSet desc_set = (VkDescriptorSet)sentry->key; + vkFreeDescriptorSets(screen->dev, pg->descpool, 1, &desc_set); + } + _mesa_set_destroy(desc_sets, NULL); if (batch->batch_id == ZINK_COMPUTE_BATCH_ID) { struct zink_compute_program *comp = (struct zink_compute_program*)entry->key; zink_compute_program_reference(screen, &comp, NULL); @@ -59,7 +66,7 @@ zink_reset_batch(struct zink_context *ctx, struct zink_batch *batch) zink_gfx_program_reference(screen, &prog, NULL); } } - _mesa_set_clear(batch->programs, NULL); + _mesa_hash_table_clear(batch->programs, NULL); set_foreach(batch->fbs, entry) { struct zink_framebuffer *fb = (void*)entry->key; @@ -67,9 +74,6 @@ zink_reset_batch(struct zink_context *ctx, struct zink_batch *batch) _mesa_set_remove(batch->fbs, entry); } - if (vkResetDescriptorPool(screen->dev, batch->descpool, 0) != VK_SUCCESS) - debug_printf("vkResetDescriptorPool failed\n"); - if (vkResetCommandPool(screen->dev, batch->cmdpool, 0) != VK_SUCCESS) debug_printf("vkResetCommandPool failed\n"); batch->submitted = batch->has_work = false; @@ -218,14 +222,27 @@ void zink_batch_reference_program(struct zink_batch *batch, struct zink_program *pg) { - struct set_entry *entry = _mesa_set_search(batch->programs, pg); + struct hash_entry *entry = _mesa_hash_table_search(batch->programs, pg); if (!entry) { - entry = _mesa_set_add(batch->programs, pg); + entry = _mesa_hash_table_insert(batch->programs, pg, _mesa_pointer_set_create(NULL)); pipe_reference(NULL, &pg->reference); } batch->has_work = true; } +bool +zink_batch_add_desc_set(struct zink_batch *batch, struct zink_program *pg, VkDescriptorSet desc_set) +{ + struct hash_entry *entry = _mesa_hash_table_search(batch->programs, pg); + assert(entry); + struct set *desc_sets = (void*)entry->data; + if (!_mesa_set_search(desc_sets, desc_set)) { + _mesa_set_add(desc_sets, desc_set); + return true; + } + return false; +} + void zink_batch_reference_surface(struct zink_batch *batch, struct zink_surface *surface) diff --git a/src/gallium/drivers/zink/zink_batch.h b/src/gallium/drivers/zink/zink_batch.h index dd9d97a6f39..b205e23bd92 100644 --- a/src/gallium/drivers/zink/zink_batch.h +++ b/src/gallium/drivers/zink/zink_batch.h @@ -32,6 +32,7 @@ struct pipe_reference; struct zink_context; +struct zink_descriptor_set; struct zink_fence; struct zink_framebuffer; struct zink_program; @@ -46,13 +47,13 @@ struct zink_batch { unsigned batch_id : 3; VkCommandPool cmdpool; VkCommandBuffer cmdbuf; - VkDescriptorPool descpool; + unsigned short max_descs; //set if the device gives oom when allocating a new desc set unsigned short descs_used; //number of descriptors currently allocated struct zink_fence *fence; struct set *fbs; - struct set *programs; + struct hash_table *programs; struct set *resources; struct set *sampler_views; @@ -97,4 +98,7 @@ zink_batch_reference_program(struct zink_batch *batch, void zink_batch_reference_surface(struct zink_batch *batch, struct zink_surface *surface); + +bool +zink_batch_add_desc_set(struct zink_batch *batch, struct zink_program *pg, VkDescriptorSet desc_set); #endif diff --git a/src/gallium/drivers/zink/zink_context.c b/src/gallium/drivers/zink/zink_context.c index 96f2d5300db..284b2f61a7d 100644 --- a/src/gallium/drivers/zink/zink_context.c +++ b/src/gallium/drivers/zink/zink_context.c @@ -57,7 +57,6 @@ destroy_batch(struct zink_context* ctx, struct zink_batch* batch) struct zink_screen *screen = zink_screen(ctx->base.screen); zink_reset_batch(ctx, batch); - vkDestroyDescriptorPool(screen->dev, batch->descpool, NULL); vkFreeCommandBuffers(screen->dev, batch->cmdpool, 1, &batch->cmdbuf); vkDestroyCommandPool(screen->dev, batch->cmdpool, NULL); zink_fence_reference(screen, &batch->fence, NULL); @@ -66,7 +65,7 @@ destroy_batch(struct zink_context* ctx, struct zink_batch* batch) _mesa_set_destroy(batch->sampler_views, NULL); util_dynarray_fini(&batch->zombie_samplers); _mesa_set_destroy(batch->surfaces, NULL); - _mesa_set_destroy(batch->programs, NULL); + _mesa_hash_table_destroy(batch->programs, NULL); _mesa_set_destroy(batch->active_queries, NULL); } @@ -1763,28 +1762,13 @@ init_batch(struct zink_context *ctx, struct zink_batch *batch, unsigned idx) cbai.level = VK_COMMAND_BUFFER_LEVEL_PRIMARY; cbai.commandBufferCount = 1; - VkDescriptorPoolSize sizes[] = { - {VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER, ZINK_BATCH_DESC_SIZE}, - {VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER, ZINK_BATCH_DESC_SIZE}, - {VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, ZINK_BATCH_DESC_SIZE}, - {VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER, ZINK_BATCH_DESC_SIZE}, - {VK_DESCRIPTOR_TYPE_STORAGE_IMAGE, ZINK_BATCH_DESC_SIZE}, - {VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, ZINK_BATCH_DESC_SIZE}, - }; - VkDescriptorPoolCreateInfo dpci = {}; - dpci.sType = VK_STRUCTURE_TYPE_DESCRIPTOR_POOL_CREATE_INFO; - dpci.pPoolSizes = sizes; - dpci.poolSizeCount = ARRAY_SIZE(sizes); - dpci.flags = 0; - dpci.maxSets = ZINK_BATCH_DESC_SIZE; - if (vkAllocateCommandBuffers(screen->dev, &cbai, &batch->cmdbuf) != VK_SUCCESS) return false; batch->fbs = _mesa_pointer_set_create(NULL); batch->resources = _mesa_pointer_set_create(NULL); batch->sampler_views = _mesa_pointer_set_create(NULL); - batch->programs = _mesa_pointer_set_create(NULL); + batch->programs = _mesa_pointer_hash_table_create(NULL); batch->surfaces = _mesa_pointer_set_create(NULL); if (!batch->resources || !batch->sampler_views || @@ -1794,10 +1778,6 @@ init_batch(struct zink_context *ctx, struct zink_batch *batch, unsigned idx) util_dynarray_init(&batch->zombie_samplers, NULL); util_dynarray_init(&batch->persistent_resources, NULL); - if (vkCreateDescriptorPool(screen->dev, &dpci, 0, - &batch->descpool) != VK_SUCCESS) - return false; - batch->batch_id = idx; batch->fence = zink_create_fence(ctx->base.screen, batch); diff --git a/src/gallium/drivers/zink/zink_context.h b/src/gallium/drivers/zink/zink_context.h index f7c068dd6b5..0e286c3c937 100644 --- a/src/gallium/drivers/zink/zink_context.h +++ b/src/gallium/drivers/zink/zink_context.h @@ -105,6 +105,8 @@ struct zink_viewport_state { #define ZINK_COMPUTE_BATCH_COUNT 1 #define ZINK_NUM_BATCHES (ZINK_NUM_GFX_BATCHES + 1) +#define ZINK_DEFAULT_MAX_DESCS 5000 + struct zink_context { struct pipe_context base; struct slab_child_pool transfer_pool; diff --git a/src/gallium/drivers/zink/zink_draw.c b/src/gallium/drivers/zink/zink_draw.c index 59c0cd81dc8..37f6885ae60 100644 --- a/src/gallium/drivers/zink/zink_draw.c +++ b/src/gallium/drivers/zink/zink_draw.c @@ -16,27 +16,28 @@ #include "util/u_prim.h" #include "util/u_prim_restart.h" + static VkDescriptorSet allocate_descriptor_set(struct zink_screen *screen, struct zink_batch *batch, - VkDescriptorSetLayout dsl, - unsigned num_descriptors) + struct zink_program *pg) { VkDescriptorSetAllocateInfo dsai; memset((void *)&dsai, 0, sizeof(dsai)); dsai.sType = VK_STRUCTURE_TYPE_DESCRIPTOR_SET_ALLOCATE_INFO; dsai.pNext = NULL; - dsai.descriptorPool = batch->descpool; + dsai.descriptorPool = pg->descpool; dsai.descriptorSetCount = 1; - dsai.pSetLayouts = &dsl; + dsai.pSetLayouts = &pg->dsl; VkDescriptorSet desc_set; if (vkAllocateDescriptorSets(screen->dev, &dsai, &desc_set) != VK_SUCCESS) { - debug_printf("ZINK: failed to allocate descriptor set :/\n"); + debug_printf("ZINK: %p failed to allocate descriptor set :/\n", pg); return VK_NULL_HANDLE; } + if (zink_batch_add_desc_set(batch, pg, desc_set)) + batch->descs_used += pg->num_descriptors; - batch->descs_used += num_descriptors; return desc_set; } @@ -528,9 +529,8 @@ update_descriptors(struct zink_context *ctx, struct zink_screen *screen, bool is struct zink_program *pg = is_compute ? &ctx->curr_compute->base : &ctx->curr_program->base; zink_batch_reference_program(batch, pg); - - VkDescriptorSet desc_set = allocate_descriptor_set(screen, batch, - pg->dsl, num_descriptors); + assert(pg->num_descriptors == num_descriptors); + VkDescriptorSet desc_set = allocate_descriptor_set(screen, batch, pg); /* probably oom, so we need to stall until we free up some descriptors */ if (!desc_set) { /* update our max descriptor count so we can try and avoid this happening again */ @@ -553,7 +553,8 @@ update_descriptors(struct zink_context *ctx, struct zink_screen *screen, bool is zink_reset_batch(ctx, &ctx->batches[i]); } } - desc_set = allocate_descriptor_set(screen, batch, pg->dsl, num_descriptors); + zink_batch_reference_program(batch, pg); + desc_set = allocate_descriptor_set(screen, batch, pg); } assert(desc_set != VK_NULL_HANDLE); diff --git a/src/gallium/drivers/zink/zink_program.c b/src/gallium/drivers/zink/zink_program.c index aa6ce49c3f9..b9c88803ad8 100644 --- a/src/gallium/drivers/zink/zink_program.c +++ b/src/gallium/drivers/zink/zink_program.c @@ -26,6 +26,7 @@ #include "zink_compiler.h" #include "zink_context.h" #include "zink_render_pass.h" +#include "zink_resource.h" #include "zink_screen.h" #include "zink_state.h" @@ -112,7 +113,8 @@ keybox_equals(const void *void_a, const void *void_b) static VkDescriptorSetLayout create_desc_set_layout(VkDevice dev, struct zink_shader *stages[ZINK_SHADER_COUNT], - unsigned *num_descriptors) + unsigned *num_descriptors, + VkDescriptorPool *descpool) { VkDescriptorSetLayoutBinding bindings[(PIPE_SHADER_TYPES * (PIPE_MAX_CONSTANT_BUFFERS + PIPE_MAX_SAMPLERS + PIPE_MAX_SHADER_BUFFERS + PIPE_MAX_SHADER_IMAGES))]; int num_bindings = 0; @@ -150,6 +152,25 @@ create_desc_set_layout(VkDevice dev, debug_printf("vkCreateDescriptorSetLayout failed\n"); return VK_NULL_HANDLE; } + VkDescriptorPoolSize sizes[] = { + {VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER, ZINK_BATCH_DESC_SIZE}, + {VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER, ZINK_BATCH_DESC_SIZE}, + {VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, ZINK_BATCH_DESC_SIZE}, + {VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER, ZINK_BATCH_DESC_SIZE}, + {VK_DESCRIPTOR_TYPE_STORAGE_IMAGE, ZINK_BATCH_DESC_SIZE}, + {VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, ZINK_BATCH_DESC_SIZE}, + }; + + VkDescriptorPoolCreateInfo dpci = {}; + dpci.sType = VK_STRUCTURE_TYPE_DESCRIPTOR_POOL_CREATE_INFO; + dpci.pPoolSizes = sizes; + dpci.poolSizeCount = ARRAY_SIZE(sizes); + dpci.flags = VK_DESCRIPTOR_POOL_CREATE_FREE_DESCRIPTOR_SET_BIT; + dpci.maxSets = ZINK_BATCH_DESC_SIZE; + if (vkCreateDescriptorPool(dev, &dpci, 0, descpool) != VK_SUCCESS) { + vkDestroyDescriptorSetLayout(dev, dsl, NULL); + return VK_NULL_HANDLE; + } return dsl; } @@ -530,14 +551,15 @@ zink_create_gfx_program(struct zink_context *ctx, } prog->base.dsl = create_desc_set_layout(screen->dev, stages, - &prog->base.num_descriptors); - if (prog->base.num_descriptors && !prog->base.dsl) + &prog->base.num_descriptors, &prog->base.descpool); + if (prog->base.num_descriptors && (!prog->base.dsl || !prog->base.descpool)) goto fail; prog->layout = create_gfx_pipeline_layout(screen->dev, prog->base.dsl); if (!prog->layout) goto fail; + util_dynarray_init(&prog->base.alloc_desc_sets, NULL); return prog; fail: @@ -630,14 +652,16 @@ zink_create_compute_program(struct zink_context *ctx, struct zink_shader *shader struct zink_shader *stages[ZINK_SHADER_COUNT] = {}; stages[0] = shader; comp->base.dsl = create_desc_set_layout(screen->dev, stages, - &comp->base.num_descriptors); - if (comp->base.num_descriptors && !comp->base.dsl) + &comp->base.num_descriptors, &comp->base.descpool); + if (comp->base.num_descriptors && (!comp->base.dsl || !comp->base.descpool)) goto fail; comp->layout = create_compute_pipeline_layout(screen->dev, comp->base.dsl); if (!comp->layout) goto fail; + util_dynarray_init(&comp->base.alloc_desc_sets, NULL); + return comp; fail: @@ -684,6 +708,11 @@ zink_destroy_gfx_program(struct zink_screen *screen, } zink_shader_cache_reference(screen, &prog->shader_cache, NULL); + if (prog->base.descpool) + vkDestroyDescriptorPool(screen->dev, prog->base.descpool, NULL); + + util_dynarray_fini(&prog->base.alloc_desc_sets); + ralloc_free(prog); } @@ -711,6 +740,11 @@ zink_destroy_compute_program(struct zink_screen *screen, _mesa_hash_table_destroy(comp->pipelines, NULL); zink_shader_cache_reference(screen, &comp->shader_cache, NULL); + if (comp->base.descpool) + vkDestroyDescriptorPool(screen->dev, comp->base.descpool, NULL); + + util_dynarray_fini(&comp->base.alloc_desc_sets); + ralloc_free(comp); } diff --git a/src/gallium/drivers/zink/zink_program.h b/src/gallium/drivers/zink/zink_program.h index 2f4b610c022..46802be4799 100644 --- a/src/gallium/drivers/zink/zink_program.h +++ b/src/gallium/drivers/zink/zink_program.h @@ -39,6 +39,7 @@ struct zink_gfx_pipeline_state; struct hash_table; struct set; +struct util_dynarray; struct zink_push_constant { unsigned draw_mode_is_indexed; @@ -65,8 +66,11 @@ struct zink_shader_cache { struct zink_program { struct pipe_reference reference; + struct util_dynarray alloc_desc_sets; + VkDescriptorPool descpool; VkDescriptorSetLayout dsl; unsigned num_descriptors; + unsigned descs_used; }; struct zink_gfx_program { @@ -156,4 +160,5 @@ VkPipeline zink_get_compute_pipeline(struct zink_screen *screen, struct zink_compute_program *comp, struct zink_compute_pipeline_state *state); + #endif