On 27 February 2018 at 05:43, Tapani Pälli <tapani.pa...@intel.com> wrote: > > > On 02/26/2018 07:55 PM, Emil Velikov wrote: >> >> On 15 February 2018 at 09:12, Tapani Pälli <tapani.pa...@intel.com> wrote: >>> >>> From: Simon Hausmann <simon.hausm...@qt.io> >>> >>> When looking up known glsl_type instances in the various hash tables, we >>> end up leaking the key instances used for the lookup, as the glsl_type >>> constructor allocates memory on the global mem_ctx. This patch changes >>> glsl_type to manage its own memory, which fixes the leak and also allows >>> getting rid of the global mem_ctx and its mutex. >>> >>> v2: remove lambda usage (Tapani) >>> >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104884 >>> Signed-off-by: Simon Hausmann <simon.hausm...@qt.io> >>> Signed-off-by: Tapani Pälli <tapani.pa...@intel.com> >>> --- >>> src/compiler/glsl_types.cpp | 83 >>> ++++++++++++++++++++------------------------- >>> src/compiler/glsl_types.h | 44 +++--------------------- >>> 2 files changed, 41 insertions(+), 86 deletions(-) >>> >> Great overall result, there's a small question below. >> >> Can you include the mesa-stable tag and/or the commit that introduced >> this shared memory pool. >> ... Gut feeling says it was before the file started using ralloc. > > > Sure, can add this. > >> >>> @@ -970,7 +961,7 @@ glsl_type::get_array_instance(const glsl_type *base, >>> unsigned array_size) >>> const glsl_type *t = new glsl_type(base, array_size); >>> >>> entry = _mesa_hash_table_insert(array_types, >>> - ralloc_strdup(mem_ctx, key), >>> + strdup(key), >>> (void *) t); >>> } >>> >> >> Are you sure we need the strdup here? AFAICT nothing clears it so it >> will be leaked. >> > > Yes I believe we do need it, see comment about base type name some rows > before. It is being freed during _mesa_glsl_release_types() by the > hash_free_type_function when array_types hash is iterated. > Right - could swear I saw _mesa_hash_table_insert duplicating the key. That's not the case, so everything will be torn as you mentioned.
Thanks Tapani! With a stable tag. the patch is Reviewed-by: Emil Velikov <emil.veli...@collabora.com> -Emil _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
