Re: [Mesa-dev] [PATCH] glsl: Fix memory leak with known glsl_type instances
On 06.03.2018 14:07, Emil Velikov wrote:
On 6 March 2018 at 07:32, Tapani Pälli wrote:
Hi;
On 01.03.2018 03:12, Kenneth Graunke wrote:
On Thursday, February 15, 2018 1:12:54 AM PST Tapani Pälli wrote:
From: Simon Hausmann
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
Signed-off-by: Tapani Pälli
---
src/compiler/glsl_types.cpp | 83
-
src/compiler/glsl_types.h | 44 +++-
2 files changed, 41 insertions(+), 86 deletions(-)
diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp
index 3cc5eb0495..81ff97b1f7 100644
--- a/src/compiler/glsl_types.cpp
+++ b/src/compiler/glsl_types.cpp
@@ -28,23 +28,12 @@
#include "util/hash_table.h"
-mtx_t glsl_type::mem_mutex = _MTX_INITIALIZER_NP;
mtx_t glsl_type::hash_mutex = _MTX_INITIALIZER_NP;
hash_table *glsl_type::array_types = NULL;
hash_table *glsl_type::record_types = NULL;
hash_table *glsl_type::interface_types = NULL;
hash_table *glsl_type::function_types = NULL;
hash_table *glsl_type::subroutine_types = NULL;
-void *glsl_type::mem_ctx = NULL;
-
-void
-glsl_type::init_ralloc_type_ctx(void)
-{
- if (glsl_type::mem_ctx == NULL) {
- glsl_type::mem_ctx = ralloc_context(NULL);
- assert(glsl_type::mem_ctx != NULL);
- }
-}
glsl_type::glsl_type(GLenum gl_type,
glsl_base_type base_type, unsigned
vector_elements,
@@ -63,14 +52,12 @@ glsl_type::glsl_type(GLenum gl_type,
STATIC_ASSERT((unsigned(GLSL_TYPE_INT) & 3) ==
unsigned(GLSL_TYPE_INT));
STATIC_ASSERT((unsigned(GLSL_TYPE_FLOAT) & 3) ==
unsigned(GLSL_TYPE_FLOAT));
- mtx_lock(&glsl_type::mem_mutex);
+ this->mem_ctx = ralloc_context(NULL);
+ assert(this->mem_ctx != NULL);
- init_ralloc_type_ctx();
assert(name != NULL);
this->name = ralloc_strdup(this->mem_ctx, name);
- mtx_unlock(&glsl_type::mem_mutex);
-
/* Neither dimension is zero or both dimensions are zero.
*/
assert((vector_elements == 0) == (matrix_columns == 0));
@@ -86,14 +73,12 @@ glsl_type::glsl_type(GLenum gl_type, glsl_base_type
base_type,
sampler_array(array), interface_packing(0),
interface_row_major(0), length(0)
{
- mtx_lock(&glsl_type::mem_mutex);
+ this->mem_ctx = ralloc_context(NULL);
+ assert(this->mem_ctx != NULL);
- init_ralloc_type_ctx();
assert(name != NULL);
this->name = ralloc_strdup(this->mem_ctx, name);
- mtx_unlock(&glsl_type::mem_mutex);
-
memset(& fields, 0, sizeof(fields));
matrix_columns = vector_elements = 1;
@@ -110,9 +95,9 @@ glsl_type::glsl_type(const glsl_struct_field *fields,
unsigned num_fields,
{
unsigned int i;
- mtx_lock(&glsl_type::mem_mutex);
+ this->mem_ctx = ralloc_context(NULL);
+ assert(this->mem_ctx != NULL);
- init_ralloc_type_ctx();
assert(name != NULL);
this->name = ralloc_strdup(this->mem_ctx, name);
this->fields.structure = ralloc_array(this->mem_ctx,
@@ -123,8 +108,6 @@ glsl_type::glsl_type(const glsl_struct_field *fields,
unsigned num_fields,
this->fields.structure[i].name =
ralloc_strdup(this->fields.structure,
fields[i].name);
}
-
- mtx_unlock(&glsl_type::mem_mutex);
}
glsl_type::glsl_type(const glsl_struct_field *fields, unsigned
num_fields,
@@ -140,9 +123,9 @@ glsl_type::glsl_type(const glsl_struct_field *fields,
unsigned num_fields,
{
unsigned int i;
- mtx_lock(&glsl_type::mem_mutex);
+ this->mem_ctx = ralloc_context(NULL);
+ assert(this->mem_ctx != NULL);
- init_ralloc_type_ctx();
assert(name != NULL);
this->name = ralloc_strdup(this->mem_ctx, name);
this->fields.structure = rzalloc_array(this->mem_ctx,
@@ -152,8 +135,6 @@ glsl_type::glsl_type(const glsl_struct_field *fields,
unsigned num_fields,
this->fields.structure[i].name =
ralloc_strdup(this->fields.structure,
fields[i].name);
}
-
- mtx_unlock(&glsl_type::mem_mutex);
}
glsl_type::glsl_type(const glsl_type *return_type,
@@ -167,9 +148,8 @@ glsl_type::glsl_type(const glsl_type *return_type,
{
unsigned int i;
- mtx_lock(&glsl_type::mem_mutex);
-
- init_ralloc_type_ctx();
+ this->mem_ctx = ralloc_context(NULL);
+ assert(this->mem_ctx != NULL);
this->fields.parameters = rzalloc_array(this->mem_ctx,
glsl_function_param,
num_params + 1);
@@ -185,8 +165,6
Re: [Mesa-dev] [PATCH] glsl: Fix memory leak with known glsl_type instances
On 6 March 2018 at 07:32, Tapani Pälli wrote:
> Hi;
>
>
> On 01.03.2018 03:12, Kenneth Graunke wrote:
>>
>> On Thursday, February 15, 2018 1:12:54 AM PST Tapani Pälli wrote:
>>>
>>> From: Simon Hausmann
>>>
>>> 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
>>> Signed-off-by: Tapani Pälli
>>> ---
>>> src/compiler/glsl_types.cpp | 83
>>> -
>>> src/compiler/glsl_types.h | 44 +++-
>>> 2 files changed, 41 insertions(+), 86 deletions(-)
>>>
>>> diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp
>>> index 3cc5eb0495..81ff97b1f7 100644
>>> --- a/src/compiler/glsl_types.cpp
>>> +++ b/src/compiler/glsl_types.cpp
>>> @@ -28,23 +28,12 @@
>>> #include "util/hash_table.h"
>>> -mtx_t glsl_type::mem_mutex = _MTX_INITIALIZER_NP;
>>> mtx_t glsl_type::hash_mutex = _MTX_INITIALIZER_NP;
>>> hash_table *glsl_type::array_types = NULL;
>>> hash_table *glsl_type::record_types = NULL;
>>> hash_table *glsl_type::interface_types = NULL;
>>> hash_table *glsl_type::function_types = NULL;
>>> hash_table *glsl_type::subroutine_types = NULL;
>>> -void *glsl_type::mem_ctx = NULL;
>>> -
>>> -void
>>> -glsl_type::init_ralloc_type_ctx(void)
>>> -{
>>> - if (glsl_type::mem_ctx == NULL) {
>>> - glsl_type::mem_ctx = ralloc_context(NULL);
>>> - assert(glsl_type::mem_ctx != NULL);
>>> - }
>>> -}
>>> glsl_type::glsl_type(GLenum gl_type,
>>>glsl_base_type base_type, unsigned
>>> vector_elements,
>>> @@ -63,14 +52,12 @@ glsl_type::glsl_type(GLenum gl_type,
>>> STATIC_ASSERT((unsigned(GLSL_TYPE_INT) & 3) ==
>>> unsigned(GLSL_TYPE_INT));
>>> STATIC_ASSERT((unsigned(GLSL_TYPE_FLOAT) & 3) ==
>>> unsigned(GLSL_TYPE_FLOAT));
>>> - mtx_lock(&glsl_type::mem_mutex);
>>> + this->mem_ctx = ralloc_context(NULL);
>>> + assert(this->mem_ctx != NULL);
>>> - init_ralloc_type_ctx();
>>> assert(name != NULL);
>>> this->name = ralloc_strdup(this->mem_ctx, name);
>>> - mtx_unlock(&glsl_type::mem_mutex);
>>> -
>>> /* Neither dimension is zero or both dimensions are zero.
>>> */
>>> assert((vector_elements == 0) == (matrix_columns == 0));
>>> @@ -86,14 +73,12 @@ glsl_type::glsl_type(GLenum gl_type, glsl_base_type
>>> base_type,
>>> sampler_array(array), interface_packing(0),
>>> interface_row_major(0), length(0)
>>> {
>>> - mtx_lock(&glsl_type::mem_mutex);
>>> + this->mem_ctx = ralloc_context(NULL);
>>> + assert(this->mem_ctx != NULL);
>>> - init_ralloc_type_ctx();
>>> assert(name != NULL);
>>> this->name = ralloc_strdup(this->mem_ctx, name);
>>> - mtx_unlock(&glsl_type::mem_mutex);
>>> -
>>> memset(& fields, 0, sizeof(fields));
>>>matrix_columns = vector_elements = 1;
>>> @@ -110,9 +95,9 @@ glsl_type::glsl_type(const glsl_struct_field *fields,
>>> unsigned num_fields,
>>> {
>>> unsigned int i;
>>> - mtx_lock(&glsl_type::mem_mutex);
>>> + this->mem_ctx = ralloc_context(NULL);
>>> + assert(this->mem_ctx != NULL);
>>> - init_ralloc_type_ctx();
>>> assert(name != NULL);
>>> this->name = ralloc_strdup(this->mem_ctx, name);
>>> this->fields.structure = ralloc_array(this->mem_ctx,
>>> @@ -123,8 +108,6 @@ glsl_type::glsl_type(const glsl_struct_field *fields,
>>> unsigned num_fields,
>>> this->fields.structure[i].name =
>>> ralloc_strdup(this->fields.structure,
>>>fields[i].name);
>>> }
>>> -
>>> - mtx_unlock(&glsl_type::mem_mutex);
>>> }
>>> glsl_type::glsl_type(const glsl_struct_field *fields, unsigned
>>> num_fields,
>>> @@ -140,9 +123,9 @@ glsl_type::glsl_type(const glsl_struct_field *fields,
>>> unsigned num_fields,
>>> {
>>> unsigned int i;
>>> - mtx_lock(&glsl_type::mem_mutex);
>>> + this->mem_ctx = ralloc_context(NULL);
>>> + assert(this->mem_ctx != NULL);
>>> - init_ralloc_type_ctx();
>>> assert(name != NULL);
>>> this->name = ralloc_strdup(this->mem_ctx, name);
>>> this->fields.structure = rzalloc_array(this->mem_ctx,
>>> @@ -152,8 +135,6 @@ glsl_type::glsl_type(const glsl_struct_field *fields,
>>> unsigned num_fields,
>>> this->fields.structure[i].name =
>>> ralloc_strdup(this->fields.structure,
>>>fields[i].name);
>>> }
>>> -
>>> - mtx_unlock(&glsl_type::mem_mutex);
>>> }
>>> glsl_type::glsl_type(const glsl_type *return_type,
>>> @@ -167,9 +148,8 @@ gls
Re: [Mesa-dev] [PATCH] glsl: Fix memory leak with known glsl_type instances
Hi;
On 01.03.2018 03:12, Kenneth Graunke wrote:
On Thursday, February 15, 2018 1:12:54 AM PST Tapani Pälli wrote:
From: Simon Hausmann
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
Signed-off-by: Tapani Pälli
---
src/compiler/glsl_types.cpp | 83 -
src/compiler/glsl_types.h | 44 +++-
2 files changed, 41 insertions(+), 86 deletions(-)
diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp
index 3cc5eb0495..81ff97b1f7 100644
--- a/src/compiler/glsl_types.cpp
+++ b/src/compiler/glsl_types.cpp
@@ -28,23 +28,12 @@
#include "util/hash_table.h"
-mtx_t glsl_type::mem_mutex = _MTX_INITIALIZER_NP;
mtx_t glsl_type::hash_mutex = _MTX_INITIALIZER_NP;
hash_table *glsl_type::array_types = NULL;
hash_table *glsl_type::record_types = NULL;
hash_table *glsl_type::interface_types = NULL;
hash_table *glsl_type::function_types = NULL;
hash_table *glsl_type::subroutine_types = NULL;
-void *glsl_type::mem_ctx = NULL;
-
-void
-glsl_type::init_ralloc_type_ctx(void)
-{
- if (glsl_type::mem_ctx == NULL) {
- glsl_type::mem_ctx = ralloc_context(NULL);
- assert(glsl_type::mem_ctx != NULL);
- }
-}
glsl_type::glsl_type(GLenum gl_type,
glsl_base_type base_type, unsigned vector_elements,
@@ -63,14 +52,12 @@ glsl_type::glsl_type(GLenum gl_type,
STATIC_ASSERT((unsigned(GLSL_TYPE_INT) & 3) == unsigned(GLSL_TYPE_INT));
STATIC_ASSERT((unsigned(GLSL_TYPE_FLOAT) & 3) ==
unsigned(GLSL_TYPE_FLOAT));
- mtx_lock(&glsl_type::mem_mutex);
+ this->mem_ctx = ralloc_context(NULL);
+ assert(this->mem_ctx != NULL);
- init_ralloc_type_ctx();
assert(name != NULL);
this->name = ralloc_strdup(this->mem_ctx, name);
- mtx_unlock(&glsl_type::mem_mutex);
-
/* Neither dimension is zero or both dimensions are zero.
*/
assert((vector_elements == 0) == (matrix_columns == 0));
@@ -86,14 +73,12 @@ glsl_type::glsl_type(GLenum gl_type, glsl_base_type
base_type,
sampler_array(array), interface_packing(0),
interface_row_major(0), length(0)
{
- mtx_lock(&glsl_type::mem_mutex);
+ this->mem_ctx = ralloc_context(NULL);
+ assert(this->mem_ctx != NULL);
- init_ralloc_type_ctx();
assert(name != NULL);
this->name = ralloc_strdup(this->mem_ctx, name);
- mtx_unlock(&glsl_type::mem_mutex);
-
memset(& fields, 0, sizeof(fields));
matrix_columns = vector_elements = 1;
@@ -110,9 +95,9 @@ glsl_type::glsl_type(const glsl_struct_field *fields,
unsigned num_fields,
{
unsigned int i;
- mtx_lock(&glsl_type::mem_mutex);
+ this->mem_ctx = ralloc_context(NULL);
+ assert(this->mem_ctx != NULL);
- init_ralloc_type_ctx();
assert(name != NULL);
this->name = ralloc_strdup(this->mem_ctx, name);
this->fields.structure = ralloc_array(this->mem_ctx,
@@ -123,8 +108,6 @@ glsl_type::glsl_type(const glsl_struct_field *fields,
unsigned num_fields,
this->fields.structure[i].name = ralloc_strdup(this->fields.structure,
fields[i].name);
}
-
- mtx_unlock(&glsl_type::mem_mutex);
}
glsl_type::glsl_type(const glsl_struct_field *fields, unsigned num_fields,
@@ -140,9 +123,9 @@ glsl_type::glsl_type(const glsl_struct_field *fields,
unsigned num_fields,
{
unsigned int i;
- mtx_lock(&glsl_type::mem_mutex);
+ this->mem_ctx = ralloc_context(NULL);
+ assert(this->mem_ctx != NULL);
- init_ralloc_type_ctx();
assert(name != NULL);
this->name = ralloc_strdup(this->mem_ctx, name);
this->fields.structure = rzalloc_array(this->mem_ctx,
@@ -152,8 +135,6 @@ glsl_type::glsl_type(const glsl_struct_field *fields,
unsigned num_fields,
this->fields.structure[i].name = ralloc_strdup(this->fields.structure,
fields[i].name);
}
-
- mtx_unlock(&glsl_type::mem_mutex);
}
glsl_type::glsl_type(const glsl_type *return_type,
@@ -167,9 +148,8 @@ glsl_type::glsl_type(const glsl_type *return_type,
{
unsigned int i;
- mtx_lock(&glsl_type::mem_mutex);
-
- init_ralloc_type_ctx();
+ this->mem_ctx = ralloc_context(NULL);
+ assert(this->mem_ctx != NULL);
this->fields.parameters = rzalloc_array(this->mem_ctx,
glsl_function_param, num_params +
1);
@@ -185,8 +165,6 @@ glsl_type::glsl_type(const glsl_type *return_type,
this->fields.parameters[i + 1].in = params[i].in;
Re: [Mesa-dev] [PATCH] glsl: Fix memory leak with known glsl_type instances
On Thursday, February 15, 2018 1:12:54 AM PST Tapani Pälli wrote:
> From: Simon Hausmann
>
> 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
> Signed-off-by: Tapani Pälli
> ---
> src/compiler/glsl_types.cpp | 83
> -
> src/compiler/glsl_types.h | 44 +++-
> 2 files changed, 41 insertions(+), 86 deletions(-)
>
> diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp
> index 3cc5eb0495..81ff97b1f7 100644
> --- a/src/compiler/glsl_types.cpp
> +++ b/src/compiler/glsl_types.cpp
> @@ -28,23 +28,12 @@
> #include "util/hash_table.h"
>
>
> -mtx_t glsl_type::mem_mutex = _MTX_INITIALIZER_NP;
> mtx_t glsl_type::hash_mutex = _MTX_INITIALIZER_NP;
> hash_table *glsl_type::array_types = NULL;
> hash_table *glsl_type::record_types = NULL;
> hash_table *glsl_type::interface_types = NULL;
> hash_table *glsl_type::function_types = NULL;
> hash_table *glsl_type::subroutine_types = NULL;
> -void *glsl_type::mem_ctx = NULL;
> -
> -void
> -glsl_type::init_ralloc_type_ctx(void)
> -{
> - if (glsl_type::mem_ctx == NULL) {
> - glsl_type::mem_ctx = ralloc_context(NULL);
> - assert(glsl_type::mem_ctx != NULL);
> - }
> -}
>
> glsl_type::glsl_type(GLenum gl_type,
> glsl_base_type base_type, unsigned vector_elements,
> @@ -63,14 +52,12 @@ glsl_type::glsl_type(GLenum gl_type,
> STATIC_ASSERT((unsigned(GLSL_TYPE_INT) & 3) == unsigned(GLSL_TYPE_INT));
> STATIC_ASSERT((unsigned(GLSL_TYPE_FLOAT) & 3) ==
> unsigned(GLSL_TYPE_FLOAT));
>
> - mtx_lock(&glsl_type::mem_mutex);
> + this->mem_ctx = ralloc_context(NULL);
> + assert(this->mem_ctx != NULL);
>
> - init_ralloc_type_ctx();
> assert(name != NULL);
> this->name = ralloc_strdup(this->mem_ctx, name);
>
> - mtx_unlock(&glsl_type::mem_mutex);
> -
> /* Neither dimension is zero or both dimensions are zero.
> */
> assert((vector_elements == 0) == (matrix_columns == 0));
> @@ -86,14 +73,12 @@ glsl_type::glsl_type(GLenum gl_type, glsl_base_type
> base_type,
> sampler_array(array), interface_packing(0),
> interface_row_major(0), length(0)
> {
> - mtx_lock(&glsl_type::mem_mutex);
> + this->mem_ctx = ralloc_context(NULL);
> + assert(this->mem_ctx != NULL);
>
> - init_ralloc_type_ctx();
> assert(name != NULL);
> this->name = ralloc_strdup(this->mem_ctx, name);
>
> - mtx_unlock(&glsl_type::mem_mutex);
> -
> memset(& fields, 0, sizeof(fields));
>
> matrix_columns = vector_elements = 1;
> @@ -110,9 +95,9 @@ glsl_type::glsl_type(const glsl_struct_field *fields,
> unsigned num_fields,
> {
> unsigned int i;
>
> - mtx_lock(&glsl_type::mem_mutex);
> + this->mem_ctx = ralloc_context(NULL);
> + assert(this->mem_ctx != NULL);
>
> - init_ralloc_type_ctx();
> assert(name != NULL);
> this->name = ralloc_strdup(this->mem_ctx, name);
> this->fields.structure = ralloc_array(this->mem_ctx,
> @@ -123,8 +108,6 @@ glsl_type::glsl_type(const glsl_struct_field *fields,
> unsigned num_fields,
>this->fields.structure[i].name = ralloc_strdup(this->fields.structure,
> fields[i].name);
> }
> -
> - mtx_unlock(&glsl_type::mem_mutex);
> }
>
> glsl_type::glsl_type(const glsl_struct_field *fields, unsigned num_fields,
> @@ -140,9 +123,9 @@ glsl_type::glsl_type(const glsl_struct_field *fields,
> unsigned num_fields,
> {
> unsigned int i;
>
> - mtx_lock(&glsl_type::mem_mutex);
> + this->mem_ctx = ralloc_context(NULL);
> + assert(this->mem_ctx != NULL);
>
> - init_ralloc_type_ctx();
> assert(name != NULL);
> this->name = ralloc_strdup(this->mem_ctx, name);
> this->fields.structure = rzalloc_array(this->mem_ctx,
> @@ -152,8 +135,6 @@ glsl_type::glsl_type(const glsl_struct_field *fields,
> unsigned num_fields,
>this->fields.structure[i].name = ralloc_strdup(this->fields.structure,
> fields[i].name);
> }
> -
> - mtx_unlock(&glsl_type::mem_mutex);
> }
>
> glsl_type::glsl_type(const glsl_type *return_type,
> @@ -167,9 +148,8 @@ glsl_type::glsl_type(const glsl_type *return_type,
> {
> unsigned int i;
>
> - mtx_lock(&glsl_type::mem_mutex);
> -
> - init_ralloc_type_ctx();
> + this->mem_ctx = ralloc_context(NULL);
> + assert(this->mem_ctx != NULL);
>
> this->fields.parameters = rzalloc_array(this->mem_ctx,
> glsl_function_param, num
Re: [Mesa-dev] [PATCH] glsl: Fix memory leak with known glsl_type instances
On 27 February 2018 at 05:43, Tapani Pälli wrote: > > > On 02/26/2018 07:55 PM, Emil Velikov wrote: >> >> On 15 February 2018 at 09:12, Tapani Pälli wrote: >>> >>> From: Simon Hausmann >>> >>> 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 >>> Signed-off-by: Tapani Pälli >>> --- >>> 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 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: Fix memory leak with known glsl_type instances
On 02/26/2018 07:55 PM, Emil Velikov wrote: On 15 February 2018 at 09:12, Tapani Pälli wrote: From: Simon Hausmann 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 Signed-off-by: Tapani Pälli --- 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. // Tapani ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: Fix memory leak with known glsl_type instances
On 15 February 2018 at 09:12, Tapani Pälli wrote: > From: Simon Hausmann > > 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 > Signed-off-by: Tapani Pälli > --- > 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. > @@ -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. -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: Fix memory leak with known glsl_type instances
ping ..
On 02/15/2018 11:12 AM, Tapani Pälli wrote:
From: Simon Hausmann
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
Signed-off-by: Tapani Pälli
---
src/compiler/glsl_types.cpp | 83 -
src/compiler/glsl_types.h | 44 +++-
2 files changed, 41 insertions(+), 86 deletions(-)
diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp
index 3cc5eb0495..81ff97b1f7 100644
--- a/src/compiler/glsl_types.cpp
+++ b/src/compiler/glsl_types.cpp
@@ -28,23 +28,12 @@
#include "util/hash_table.h"
-mtx_t glsl_type::mem_mutex = _MTX_INITIALIZER_NP;
mtx_t glsl_type::hash_mutex = _MTX_INITIALIZER_NP;
hash_table *glsl_type::array_types = NULL;
hash_table *glsl_type::record_types = NULL;
hash_table *glsl_type::interface_types = NULL;
hash_table *glsl_type::function_types = NULL;
hash_table *glsl_type::subroutine_types = NULL;
-void *glsl_type::mem_ctx = NULL;
-
-void
-glsl_type::init_ralloc_type_ctx(void)
-{
- if (glsl_type::mem_ctx == NULL) {
- glsl_type::mem_ctx = ralloc_context(NULL);
- assert(glsl_type::mem_ctx != NULL);
- }
-}
glsl_type::glsl_type(GLenum gl_type,
glsl_base_type base_type, unsigned vector_elements,
@@ -63,14 +52,12 @@ glsl_type::glsl_type(GLenum gl_type,
STATIC_ASSERT((unsigned(GLSL_TYPE_INT) & 3) == unsigned(GLSL_TYPE_INT));
STATIC_ASSERT((unsigned(GLSL_TYPE_FLOAT) & 3) ==
unsigned(GLSL_TYPE_FLOAT));
- mtx_lock(&glsl_type::mem_mutex);
+ this->mem_ctx = ralloc_context(NULL);
+ assert(this->mem_ctx != NULL);
- init_ralloc_type_ctx();
assert(name != NULL);
this->name = ralloc_strdup(this->mem_ctx, name);
- mtx_unlock(&glsl_type::mem_mutex);
-
/* Neither dimension is zero or both dimensions are zero.
*/
assert((vector_elements == 0) == (matrix_columns == 0));
@@ -86,14 +73,12 @@ glsl_type::glsl_type(GLenum gl_type, glsl_base_type
base_type,
sampler_array(array), interface_packing(0),
interface_row_major(0), length(0)
{
- mtx_lock(&glsl_type::mem_mutex);
+ this->mem_ctx = ralloc_context(NULL);
+ assert(this->mem_ctx != NULL);
- init_ralloc_type_ctx();
assert(name != NULL);
this->name = ralloc_strdup(this->mem_ctx, name);
- mtx_unlock(&glsl_type::mem_mutex);
-
memset(& fields, 0, sizeof(fields));
matrix_columns = vector_elements = 1;
@@ -110,9 +95,9 @@ glsl_type::glsl_type(const glsl_struct_field *fields,
unsigned num_fields,
{
unsigned int i;
- mtx_lock(&glsl_type::mem_mutex);
+ this->mem_ctx = ralloc_context(NULL);
+ assert(this->mem_ctx != NULL);
- init_ralloc_type_ctx();
assert(name != NULL);
this->name = ralloc_strdup(this->mem_ctx, name);
this->fields.structure = ralloc_array(this->mem_ctx,
@@ -123,8 +108,6 @@ glsl_type::glsl_type(const glsl_struct_field *fields,
unsigned num_fields,
this->fields.structure[i].name = ralloc_strdup(this->fields.structure,
fields[i].name);
}
-
- mtx_unlock(&glsl_type::mem_mutex);
}
glsl_type::glsl_type(const glsl_struct_field *fields, unsigned num_fields,
@@ -140,9 +123,9 @@ glsl_type::glsl_type(const glsl_struct_field *fields,
unsigned num_fields,
{
unsigned int i;
- mtx_lock(&glsl_type::mem_mutex);
+ this->mem_ctx = ralloc_context(NULL);
+ assert(this->mem_ctx != NULL);
- init_ralloc_type_ctx();
assert(name != NULL);
this->name = ralloc_strdup(this->mem_ctx, name);
this->fields.structure = rzalloc_array(this->mem_ctx,
@@ -152,8 +135,6 @@ glsl_type::glsl_type(const glsl_struct_field *fields,
unsigned num_fields,
this->fields.structure[i].name = ralloc_strdup(this->fields.structure,
fields[i].name);
}
-
- mtx_unlock(&glsl_type::mem_mutex);
}
glsl_type::glsl_type(const glsl_type *return_type,
@@ -167,9 +148,8 @@ glsl_type::glsl_type(const glsl_type *return_type,
{
unsigned int i;
- mtx_lock(&glsl_type::mem_mutex);
-
- init_ralloc_type_ctx();
+ this->mem_ctx = ralloc_context(NULL);
+ assert(this->mem_ctx != NULL);
this->fields.parameters = rzalloc_array(this->mem_ctx,
glsl_function_param, num_params +
1);
@@ -185,8 +165,6 @@ glsl_type::glsl_type(const glsl_type *return_type,
this->fields.parameters[i + 1].in = params[i].in;
this->fields.parameters[i + 1].out = params[i].out;
}
