ping ..

On 02/15/2018 11:12 AM, Tapani Pälli 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(-)

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;
     }
-
-   mtx_unlock(&glsl_type::mem_mutex);
  }
glsl_type::glsl_type(const char *subroutine_name) :
@@ -197,12 +175,16 @@ glsl_type::glsl_type(const char *subroutine_name) :
     vector_elements(1), matrix_columns(1),
     length(0)
  {
-   mtx_lock(&glsl_type::mem_mutex);
+   this->mem_ctx = ralloc_context(NULL);
+   assert(this->mem_ctx != NULL);
- init_ralloc_type_ctx();
     assert(subroutine_name != NULL);
     this->name = ralloc_strdup(this->mem_ctx, subroutine_name);
-   mtx_unlock(&glsl_type::mem_mutex);
+}
+
+glsl_type::~glsl_type()
+{
+    ralloc_free(this->mem_ctx);
  }
bool
@@ -416,6 +398,17 @@ const glsl_type *glsl_type::get_scalar_type() const
  }
+static void
+hash_free_type_function(struct hash_entry *entry)
+{
+   glsl_type *type = (glsl_type *) entry->data;
+
+   if (type->is_array())
+      free((void*)entry->key);
+
+   delete type;
+}
+
  void
  _mesa_glsl_release_types(void)
  {
@@ -424,32 +417,29 @@ _mesa_glsl_release_types(void)
      * necessary.
      */
     if (glsl_type::array_types != NULL) {
-      _mesa_hash_table_destroy(glsl_type::array_types, NULL);
+      _mesa_hash_table_destroy(glsl_type::array_types, 
hash_free_type_function);
        glsl_type::array_types = NULL;
     }
if (glsl_type::record_types != NULL) {
-      _mesa_hash_table_destroy(glsl_type::record_types, NULL);
+      _mesa_hash_table_destroy(glsl_type::record_types, 
hash_free_type_function);
        glsl_type::record_types = NULL;
     }
if (glsl_type::interface_types != NULL) {
-      _mesa_hash_table_destroy(glsl_type::interface_types, NULL);
+      _mesa_hash_table_destroy(glsl_type::interface_types, 
hash_free_type_function);
        glsl_type::interface_types = NULL;
     }
if (glsl_type::function_types != NULL) {
-      _mesa_hash_table_destroy(glsl_type::function_types, NULL);
+      _mesa_hash_table_destroy(glsl_type::function_types, 
hash_free_type_function);
        glsl_type::function_types = NULL;
     }
if (glsl_type::subroutine_types != NULL) {
-      _mesa_hash_table_destroy(glsl_type::subroutine_types, NULL);
+      _mesa_hash_table_destroy(glsl_type::subroutine_types, 
hash_free_type_function);
        glsl_type::subroutine_types = NULL;
     }
-
-   ralloc_free(glsl_type::mem_ctx);
-   glsl_type::mem_ctx = NULL;
  }
@@ -473,9 +463,10 @@ glsl_type::glsl_type(const glsl_type *array, unsigned length) :
      */
     const unsigned name_length = strlen(array->name) + 10 + 3;
- mtx_lock(&glsl_type::mem_mutex);
+   this->mem_ctx = ralloc_context(NULL);
+   assert(this->mem_ctx != NULL);
+
     char *const n = (char *) ralloc_size(this->mem_ctx, name_length);
-   mtx_unlock(&glsl_type::mem_mutex);
if (length == 0)
        snprintf(n, name_length, "%s[]", array->name);
@@ -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);
     }
diff --git a/src/compiler/glsl_types.h b/src/compiler/glsl_types.h
index ab0b263764..e016835695 100644
--- a/src/compiler/glsl_types.h
+++ b/src/compiler/glsl_types.h
@@ -169,39 +169,6 @@ private:
     }
public:
-   /* Callers of this ralloc-based new need not call delete. It's
-    * easier to just ralloc_free 'mem_ctx' (or any of its ancestors). */
-   static void* operator new(size_t size)
-   {
-      ASSERT_BITFIELD_SIZE(glsl_type, base_type, GLSL_TYPE_ERROR);
-      ASSERT_BITFIELD_SIZE(glsl_type, sampled_type, GLSL_TYPE_ERROR);
-      ASSERT_BITFIELD_SIZE(glsl_type, sampler_dimensionality,
-                           GLSL_SAMPLER_DIM_SUBPASS_MS);
-
-      mtx_lock(&glsl_type::mem_mutex);
-
-      /* mem_ctx should have been created by the static members */
-      assert(glsl_type::mem_ctx != NULL);
-
-      void *type;
-
-      type = ralloc_size(glsl_type::mem_ctx, size);
-      assert(type != NULL);
-
-      mtx_unlock(&glsl_type::mem_mutex);
-
-      return type;
-   }
-
-   /* If the user *does* call delete, that's OK, we will just
-    * ralloc_free in that case. */
-   static void operator delete(void *type)
-   {
-      mtx_lock(&glsl_type::mem_mutex);
-      ralloc_free(type);
-      mtx_unlock(&glsl_type::mem_mutex);
-   }
-
     /**
      * \name Vector and matrix element counts
      *
@@ -873,19 +840,16 @@ public:
        return (bool) interface_row_major;
     }
+ ~glsl_type();
+
  private:
- static mtx_t mem_mutex;
     static mtx_t hash_mutex;
/**
-    * ralloc context for all glsl_type allocations
-    *
-    * Set on the first call to \c glsl_type::new.
+    * ralloc context for the type itself.
      */
-   static void *mem_ctx;
-
-   void init_ralloc_type_ctx(void);
+   void *mem_ctx;
/** Constructor for vector and matrix types */
     glsl_type(GLenum gl_type,

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to