From 624789e3708c87ea2a4c8d2266266b489b421cba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tapani=20P=C3=A4lli?= Date: Fri, 15 Mar 2019 09:47:49 +0200 Subject: [PATCH] compiler/glsl: handle case where we have multiple users for types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Timothy Arceri Reviewed-by: Jason Ekstrand --- 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 @@ -2324,6 +2324,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. * @@ -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 -_mesa_glsl_release_types(void); +glsl_type_singleton_decref(); + +extern 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);