Mesa (master): compiler/glsl: handle case where we have multiple users for types

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Tue Apr 16 10:12:19 UTC 2019


Module: Mesa
Branch: master
Commit: 624789e3708c87ea2a4c8d2266266b489b421cba
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=624789e3708c87ea2a4c8d2266266b489b421cba

Author: Tapani Pälli <tapani.palli at intel.com>
Date:   Fri Mar 15 09:47:49 2019 +0200

compiler/glsl: handle case where we have multiple users for types

Both Vulkan and OpenGL might be using glsl_types simultaneously or we
can also have multiple concurrent Vulkan instances using glsl_types.
Patch adds a one time init to track number of users and will release
types only when last user calls _glsl_type_singleton_decref().

This change fixes glsl_type memory leaks we have with anv driver.

v2: reuse hash_mutex, cleanup, apply fix also to radv driver and
    rename helper functions (Jason)

v3: move init, destroy to happen on GL context init and destroy

Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>
Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>

---

 src/amd/vulkan/radv_device.c             |  3 +++
 src/compiler/glsl/glsl_parser_extras.cpp | 20 ++++++++++++++++++--
 src/compiler/glsl/glsl_parser_extras.h   |  2 ++
 src/compiler/glsl/standalone.cpp         |  3 ++-
 src/compiler/glsl_types.cpp              | 32 +++++++++++++++++++++++++++-----
 src/compiler/glsl_types.h                | 10 +++++++---
 src/intel/vulkan/anv_device.c            |  3 +++
 src/mesa/main/context.c                  |  4 ++++
 8 files changed, 66 insertions(+), 11 deletions(-)

diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
index 1f77dcadb17..1ad972a195d 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -48,6 +48,7 @@
 #include "util/build_id.h"
 #include "util/debug.h"
 #include "util/mesa-sha1.h"
+#include "compiler/glsl_types.h"
 
 static int
 radv_device_get_cache_uuid(enum radeon_family family, void *uuid)
@@ -582,6 +583,7 @@ VkResult radv_CreateInstance(
 	}
 
 	_mesa_locale_init();
+	glsl_type_singleton_init_or_ref();
 
 	VG(VALGRIND_CREATE_MEMPOOL(instance, 0, false));
 
@@ -607,6 +609,7 @@ void radv_DestroyInstance(
 
 	VG(VALGRIND_DESTROY_MEMPOOL(instance));
 
+	glsl_type_singleton_decref();
 	_mesa_locale_fini();
 
 	vk_debug_report_instance_destroy(&instance->debug_report_callbacks);
diff --git a/src/compiler/glsl/glsl_parser_extras.cpp b/src/compiler/glsl/glsl_parser_extras.cpp
index 0ffad2d25a0..b30c638cdc9 100644
--- a/src/compiler/glsl/glsl_parser_extras.cpp
+++ b/src/compiler/glsl/glsl_parser_extras.cpp
@@ -2325,6 +2325,24 @@ do_common_optimization(exec_list *ir, bool linked,
 extern "C" {
 
 /**
+ * To be called at GL context ctor.
+ */
+void
+_mesa_init_shader_compiler_types(void)
+{
+   glsl_type_singleton_init_or_ref();
+}
+
+/**
+ * To be called at GL context dtor.
+ */
+void
+_mesa_destroy_shader_compiler_types(void)
+{
+   glsl_type_singleton_decref();
+}
+
+/**
  * To be called at GL teardown time, this frees compiler datastructures.
  *
  * After calling this, any previously compiled shaders and shader
@@ -2335,8 +2353,6 @@ void
 _mesa_destroy_shader_compiler(void)
 {
    _mesa_destroy_shader_compiler_caches();
-
-   _mesa_glsl_release_types();
 }
 
 /**
diff --git a/src/compiler/glsl/glsl_parser_extras.h b/src/compiler/glsl/glsl_parser_extras.h
index 8646ba6cadd..f92d2160aac 100644
--- a/src/compiler/glsl/glsl_parser_extras.h
+++ b/src/compiler/glsl/glsl_parser_extras.h
@@ -1010,6 +1010,8 @@ extern int glcpp_preprocess(void *ctx, const char **shader, char **info_log,
                             struct _mesa_glsl_parse_state *state,
                             struct gl_context *gl_ctx);
 
+extern void _mesa_init_shader_compiler_types(void);
+extern void _mesa_destroy_shader_compiler_types(void);
 extern void _mesa_destroy_shader_compiler(void);
 extern void _mesa_destroy_shader_compiler_caches(void);
 
diff --git a/src/compiler/glsl/standalone.cpp b/src/compiler/glsl/standalone.cpp
index 942b9ee4986..7b3d358ca96 100644
--- a/src/compiler/glsl/standalone.cpp
+++ b/src/compiler/glsl/standalone.cpp
@@ -132,6 +132,7 @@ static void
 initialize_context(struct gl_context *ctx, gl_api api)
 {
    initialize_context_to_defaults(ctx, api);
+   glsl_type_singleton_init_or_ref();
 
    /* The standalone compiler needs to claim support for almost
     * everything in order to compile the built-in functions.
@@ -617,6 +618,6 @@ standalone_compiler_cleanup(struct gl_shader_program *whole_program)
    delete whole_program->FragDataIndexBindings;
 
    ralloc_free(whole_program);
-   _mesa_glsl_release_types();
+   glsl_type_singleton_decref();
    _mesa_glsl_release_builtin_functions();
 }
diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp
index 66241b34281..9938b3df450 100644
--- a/src/compiler/glsl_types.cpp
+++ b/src/compiler/glsl_types.cpp
@@ -37,6 +37,12 @@ hash_table *glsl_type::interface_types = NULL;
 hash_table *glsl_type::function_types = NULL;
 hash_table *glsl_type::subroutine_types = NULL;
 
+/* There might be multiple users for types (e.g. application using OpenGL
+ * and Vulkan simultanously or app using multiple Vulkan instances). Counter
+ * is used to make sure we don't release the types if a user is still present.
+ */
+static uint32_t glsl_type_users = 0;
+
 glsl_type::glsl_type(GLenum gl_type,
                      glsl_base_type base_type, unsigned vector_elements,
                      unsigned matrix_columns, const char *name,
@@ -469,12 +475,26 @@ hash_free_type_function(struct hash_entry *entry)
 }
 
 void
-_mesa_glsl_release_types(void)
+glsl_type_singleton_init_or_ref()
 {
-   /* Should only be called during atexit (either when unloading shared
-    * object, or if process terminates), so no mutex-locking should be
-    * necessary.
-    */
+   mtx_lock(&glsl_type::hash_mutex);
+   glsl_type_users++;
+   mtx_unlock(&glsl_type::hash_mutex);
+}
+
+void
+glsl_type_singleton_decref()
+{
+   mtx_lock(&glsl_type::hash_mutex);
+
+   assert(glsl_type_users > 0);
+
+   /* Do not release glsl_types if they are still used. */
+   if (--glsl_type_users) {
+      mtx_unlock(&glsl_type::hash_mutex);
+      return;
+   }
+
    if (glsl_type::explicit_matrix_types != NULL) {
       _mesa_hash_table_destroy(glsl_type::explicit_matrix_types,
                                hash_free_type_function);
@@ -505,6 +525,8 @@ _mesa_glsl_release_types(void)
       _mesa_hash_table_destroy(glsl_type::subroutine_types, hash_free_type_function);
       glsl_type::subroutine_types = NULL;
    }
+
+   mtx_unlock(&glsl_type::hash_mutex);
 }
 
 
diff --git a/src/compiler/glsl_types.h b/src/compiler/glsl_types.h
index dd9ae657019..2fd93f7b945 100644
--- a/src/compiler/glsl_types.h
+++ b/src/compiler/glsl_types.h
@@ -47,10 +47,13 @@ struct _mesa_glsl_parse_state;
 struct glsl_symbol_table;
 
 extern void
-_mesa_glsl_initialize_types(struct _mesa_glsl_parse_state *state);
+glsl_type_singleton_init_or_ref();
+
+extern void
+glsl_type_singleton_decref();
 
 extern void
-_mesa_glsl_release_types(void);
+_mesa_glsl_initialize_types(struct _mesa_glsl_parse_state *state);
 
 void encode_type_to_blob(struct blob *blob, const struct glsl_type *type);
 
@@ -1062,8 +1065,9 @@ private:
     * data.
     */
    /*@{*/
+   friend void glsl_type_singleton_init_or_ref(void);
+   friend void glsl_type_singleton_decref(void);
    friend void _mesa_glsl_initialize_types(struct _mesa_glsl_parse_state *);
-   friend void _mesa_glsl_release_types(void);
    /*@}*/
 };
 
diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
index 2687c4699ca..74d4eebebc2 100644
--- a/src/intel/vulkan/anv_device.c
+++ b/src/intel/vulkan/anv_device.c
@@ -41,6 +41,7 @@
 #include "git_sha1.h"
 #include "vk_util.h"
 #include "common/gen_defines.h"
+#include "compiler/glsl_types.h"
 
 #include "genxml/gen7_pack.h"
 
@@ -716,6 +717,7 @@ VkResult anv_CreateInstance(
       env_var_as_boolean("ANV_ENABLE_PIPELINE_CACHE", true);
 
    _mesa_locale_init();
+   glsl_type_singleton_init_or_ref();
 
    VG(VALGRIND_CREATE_MEMPOOL(instance, 0, false));
 
@@ -746,6 +748,7 @@ void anv_DestroyInstance(
 
    vk_debug_report_instance_destroy(&instance->debug_report_callbacks);
 
+   glsl_type_singleton_decref();
    _mesa_locale_fini();
 
    vk_free(&instance->alloc, instance);
diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index e5a89d9c2fc..7300d2f3c46 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -1196,6 +1196,8 @@ _mesa_initialize_context(struct gl_context *ctx,
    /* misc one-time initializations */
    one_time_init(ctx);
 
+   _mesa_init_shader_compiler_types();
+
    /* Plug in driver functions and context pointer here.
     * This is important because when we call alloc_shared_state() below
     * we'll call ctx->Driver.NewTextureObject() to create the default
@@ -1383,6 +1385,8 @@ _mesa_free_context_data( struct gl_context *ctx )
 
    free(ctx->VersionString);
 
+   _mesa_destroy_shader_compiler_types();
+
    /* unbind the context if it's currently bound */
    if (ctx == _mesa_get_current_context()) {
       _mesa_make_current(NULL, NULL, NULL);




More information about the mesa-commit mailing list