From 128cbcd3a7ba543d644ed3189dcd668900b270f4 Mon Sep 17 00:00:00 2001 From: Kenneth Graunke Date: Wed, 5 Jun 2019 13:12:58 -0700 Subject: [PATCH] iris: Delete shader variants when deleting the API-facing shader We were space-leaking iris_compiled_shader objects, leaving them around basically forever - long after the associated iris_uncompiled_shader was deleted. Perhaps even more importantly, this left the BO containing the assembly referenced, meaning those were never reclaimed either. For long running applications, this can leak quite a bit of memory. Now, when freeing iris_uncompiled_shader, we hunt down any associated iris_compiled_shader objects and pitch those (and their BO) as well. One issue is that the shader variants can still be bound, because we haven't done a draw that updates the compiled shaders yet. This can cause issues because state changes want to look at the old program to know what to flag dirty. It's a bit tricky to get right, so instead we defer variant deletion until the shaders are properly unbound, by stashing them on a "dead" list and tidying that each time we try and delete some shader variants. This ensures long running programs delete their shaders eventually. Fixes: ed4ffb97158 ("iris: rework program cache interface") Reviewed-by: Matt Turner Part-of: --- src/gallium/drivers/iris/iris_context.h | 7 +++ src/gallium/drivers/iris/iris_program.c | 5 +-- src/gallium/drivers/iris/iris_program_cache.c | 63 +++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 3 deletions(-) diff --git a/src/gallium/drivers/iris/iris_context.h b/src/gallium/drivers/iris/iris_context.h index 730df63..361c63c 100644 --- a/src/gallium/drivers/iris/iris_context.h +++ b/src/gallium/drivers/iris/iris_context.h @@ -422,6 +422,8 @@ struct iris_binding_table { * (iris_uncompiled_shader), due to state-based recompiles (brw_*_prog_key). */ struct iris_compiled_shader { + struct list_head link; + /** Reference to the uploaded assembly. */ struct iris_state_ref assembly; @@ -594,6 +596,9 @@ struct iris_context { struct iris_compiled_shader *prog[MESA_SHADER_STAGES]; struct brw_vue_map *last_vue_map; + /** List of shader variants whose deletion has been deferred for now */ + struct list_head deleted_variants[MESA_SHADER_STAGES]; + struct u_upload_mgr *uploader; struct hash_table *cache; @@ -882,6 +887,8 @@ struct iris_compiled_shader *iris_upload_shader(struct iris_context *ice, const void *iris_find_previous_compile(const struct iris_context *ice, enum iris_program_cache_id cache_id, unsigned program_string_id); +void iris_delete_shader_variants(struct iris_context *ice, + struct iris_uncompiled_shader *ish); bool iris_blorp_lookup_shader(struct blorp_batch *blorp_batch, const void *key, uint32_t key_size, diff --git a/src/gallium/drivers/iris/iris_program.c b/src/gallium/drivers/iris/iris_program.c index ce037f7..a9e464b 100644 --- a/src/gallium/drivers/iris/iris_program.c +++ b/src/gallium/drivers/iris/iris_program.c @@ -1835,9 +1835,6 @@ get_vue_prog_data(struct iris_context *ice, gl_shader_stage stage) return (void *) ice->shaders.prog[stage]->prog_data; } -// XXX: iris_compiled_shaders are space-leaking :( -// XXX: do remember to unbind them if deleting them. - /** * Update the current shader variants for the given state. * @@ -2419,6 +2416,8 @@ iris_delete_shader_state(struct pipe_context *ctx, void *state, gl_shader_stage pipe_resource_reference(&ish->const_data_state.res, NULL); } + iris_delete_shader_variants(ice, ish); + ralloc_free(ish->nir); free(ish); } diff --git a/src/gallium/drivers/iris/iris_program_cache.c b/src/gallium/drivers/iris/iris_program_cache.c index aa0d5c2..085f4a5 100644 --- a/src/gallium/drivers/iris/iris_program_cache.c +++ b/src/gallium/drivers/iris/iris_program_cache.c @@ -115,6 +115,59 @@ iris_find_previous_compile(const struct iris_context *ice, return NULL; } +void +iris_delete_shader_variants(struct iris_context *ice, + struct iris_uncompiled_shader *ish) +{ + struct hash_table *cache = ice->shaders.cache; + gl_shader_stage stage = ish->nir->info.stage; + enum iris_program_cache_id cache_id = stage; + + hash_table_foreach(cache, entry) { + const struct keybox *keybox = entry->key; + const struct brw_base_prog_key *key = (const void *)keybox->data; + + if (keybox->cache_id == cache_id && + key->program_string_id == ish->program_id) { + struct iris_compiled_shader *shader = entry->data; + + _mesa_hash_table_remove(cache, entry); + + /* Shader variants may still be bound in the context even after + * the API-facing shader has been deleted. In particular, a draw + * may not have triggered iris_update_compiled_shaders() yet. In + * that case, we may be referring to that shader's VUE map, stream + * output settings, and so on. We also like to compare the old and + * new shader programs when swapping them out to flag dirty state. + * + * So, it's hazardous to delete a bound shader variant. We avoid + * doing so, choosing to instead move "deleted" shader variants to + * a list, deferring the actual deletion until they're not bound. + * + * For simplicity, we always move deleted variants to the list, + * even if we could delete them immediately. We'll then process + * the list, catching both these variants and any others. + */ + list_addtail(&shader->link, &ice->shaders.deleted_variants[stage]); + } + } + + /* Process any pending deferred variant deletions. */ + list_for_each_entry_safe(struct iris_compiled_shader, shader, + &ice->shaders.deleted_variants[stage], link) { + /* If the shader is still bound, defer deletion. */ + if (ice->shaders.prog[stage] == shader) + continue; + + list_del(&shader->link); + + /* Actually delete the variant. */ + pipe_resource_reference(&shader->assembly.res, NULL); + ralloc_free(shader); + } +} + + /** * Look for an existing entry in the cache that has identical assembly code. * @@ -175,6 +228,8 @@ iris_upload_shader(struct iris_context *ice, memcpy(shader->map, assembly, prog_data->program_size); } + list_inithead(&shader->link); + shader->prog_data = prog_data; shader->streamout = streamout; shader->system_values = system_values; @@ -262,6 +317,9 @@ iris_init_program_cache(struct iris_context *ice) ice->shaders.uploader = u_upload_create(&ice->ctx, 16384, PIPE_BIND_CUSTOM, PIPE_USAGE_IMMUTABLE, IRIS_RESOURCE_FLAG_SHADER_MEMZONE); + + for (int i = 0; i < MESA_SHADER_STAGES; i++) + list_inithead(&ice->shaders.deleted_variants[i]); } void @@ -269,6 +327,11 @@ iris_destroy_program_cache(struct iris_context *ice) { for (int i = 0; i < MESA_SHADER_STAGES; i++) { ice->shaders.prog[i] = NULL; + + list_for_each_entry_safe(struct iris_compiled_shader, shader, + &ice->shaders.deleted_variants[i], link) { + pipe_resource_reference(&shader->assembly.res, NULL); + } } hash_table_foreach(ice->shaders.cache, entry) { -- 2.7.4