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 <mattst88@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6075>

diff --git a/src/gallium/drivers/iris/iris_context.h b/src/gallium/drivers/iris/iris_context.h
index 730df63b958b3308c76c7c985adfc8b419b7e014..361c63cc2adf83491947611ebe1ef4a177f37310 100644 (file)
--- 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 ce037f7632305b7172b1d40cb5670aaff16a09a9..a9e464bcb465005879e9e46f7b421ab4016deb49 100644 (file)
--- 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 aa0d5c29c373685518959303122f0045a7001b97..085f4a50b9543190e20d8bd6559e527bb94c2100 100644 (file)
--- 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) {