[Mesa-dev] [PATCH] glsl: free interface_types and replace old hash_table uses
Iago Toral
itoral at igalia.com
Mon Jul 13 02:21:05 PDT 2015
On Sat, 2015-07-11 at 10:13 +1000, Timothy Arceri wrote:
> The util/hash_table was intended to be a fast hash table
> replacement for the program/hash_table see 35fd61bd99c1 and 72e55bb6888ff.
>
> This replaces some more uses of the old hash table and also
> destroys the interface_types hash table when _mesa_glsl_release_types()
> is called which wasn't previously being done.
> ---
> Was looking at the remaining program/hash_table uses and noticed that
> interface_types wasnt being freed so thought I'd fix that and replace the
> hash while I was there.
>
> No measurable compile time changes to the public shader-db
>
> src/glsl/glsl_types.cpp | 85 ++++++++++++++++++++++++++-----------------------
> src/glsl/glsl_types.h | 2 +-
> 2 files changed, 46 insertions(+), 41 deletions(-)
>
> diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
> index 281ff51..255bd69 100644
> --- a/src/glsl/glsl_types.cpp
> +++ b/src/glsl/glsl_types.cpp
> @@ -25,7 +25,7 @@
> #include "main/core.h" /* for Elements, MAX2 */
> #include "glsl_parser_extras.h"
> #include "glsl_types.h"
> -#include "program/hash_table.h"
> +#include "util/hash_table.h"
>
>
> mtx_t glsl_type::mutex = _MTX_INITIALIZER_NP;
> @@ -329,14 +329,19 @@ _mesa_glsl_release_types(void)
> * necessary.
> */
> if (glsl_type::array_types != NULL) {
> - hash_table_dtor(glsl_type::array_types);
> + _mesa_hash_table_destroy(glsl_type::array_types, NULL);
> glsl_type::array_types = NULL;
> }
>
> if (glsl_type::record_types != NULL) {
> - hash_table_dtor(glsl_type::record_types);
> + _mesa_hash_table_destroy(glsl_type::record_types, NULL);
> glsl_type::record_types = NULL;
> }
> +
> + if (glsl_type::interface_types != NULL) {
> + _mesa_hash_table_destroy(glsl_type::interface_types, NULL);
> + glsl_type::interface_types = NULL;
> + }
I think it is probably best to put the destruction of interface_types in
a separate patch, it is a different issue after all. You can add my
Reviewed-by on that patch.
With that and a couple of other minor nitpicks I mention below fixed,
this is:
Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>
> }
>
>
> @@ -648,27 +653,28 @@ glsl_type::get_array_instance(const glsl_type *base, unsigned array_size)
> mtx_lock(&glsl_type::mutex);
>
> if (array_types == NULL) {
> - array_types = hash_table_ctor(64, hash_table_string_hash,
> - hash_table_string_compare);
> + array_types = _mesa_hash_table_create(NULL, _mesa_key_hash_string,
> + _mesa_key_string_equal);
> }
>
> - const glsl_type *t = (glsl_type *) hash_table_find(array_types, key);
> -
> - if (t == NULL) {
> + const struct hash_entry *entry = _mesa_hash_table_search(array_types, key);
> + if (entry == NULL) {
> mtx_unlock(&glsl_type::mutex);
> - t = new glsl_type(base, array_size);
> + const glsl_type *t = new glsl_type(base, array_size);
> mtx_lock(&glsl_type::mutex);
>
> - hash_table_insert(array_types, (void *) t, ralloc_strdup(mem_ctx, key));
> + entry = _mesa_hash_table_insert(array_types,
> + ralloc_strdup(mem_ctx, key),
> + (void *) t);
> }
>
> - assert(t->base_type == GLSL_TYPE_ARRAY);
> - assert(t->length == array_size);
> - assert(t->fields.array == base);
> + assert(((glsl_type *)entry->data)->base_type == GLSL_TYPE_ARRAY);
> + assert(((glsl_type *)entry->data)->length == array_size);
> + assert(((glsl_type *)entry->data)->fields.array == base);
Other parts of this file put a blank between the type cast and the
variable, so I would add that here (and in all other places where you
cast entry to glsl_type* in this patch).
> mtx_unlock(&glsl_type::mutex);
>
> - return t;
> + return (glsl_type *)entry->data;
> }
>
>
> @@ -722,19 +728,13 @@ glsl_type::record_compare(const glsl_type *b) const
> }
>
>
> -int
> +bool
> glsl_type::record_key_compare(const void *a, const void *b)
> {
> const glsl_type *const key1 = (glsl_type *) a;
> const glsl_type *const key2 = (glsl_type *) b;
>
> - /* Return zero is the types match (there is zero difference) or non-zero
> - * otherwise.
> - */
> - if (strcmp(key1->name, key2->name) != 0)
> - return 1;
> -
> - return !key1->record_compare(key2);
> + return strcmp(key1->name, key2->name) == 0 && key1->record_compare(key2);
> }
>
>
> @@ -772,25 +772,27 @@ glsl_type::get_record_instance(const glsl_struct_field *fields,
> mtx_lock(&glsl_type::mutex);
>
> if (record_types == NULL) {
> - record_types = hash_table_ctor(64, record_key_hash, record_key_compare);
> + record_types = _mesa_hash_table_create(NULL, record_key_hash,
> + record_key_compare);
> }
>
> - const glsl_type *t = (glsl_type *) hash_table_find(record_types, & key);
> - if (t == NULL) {
> + const struct hash_entry *entry = _mesa_hash_table_search(record_types,
> + &key);
> + if (entry == NULL) {
> mtx_unlock(&glsl_type::mutex);
> - t = new glsl_type(fields, num_fields, name);
> + const glsl_type *t = new glsl_type(fields, num_fields, name);
> mtx_lock(&glsl_type::mutex);
>
> - hash_table_insert(record_types, (void *) t, t);
> + entry = _mesa_hash_table_insert(record_types, t, (void *) t);
> }
>
> - assert(t->base_type == GLSL_TYPE_STRUCT);
> - assert(t->length == num_fields);
> - assert(strcmp(t->name, name) == 0);
> + assert(((glsl_type *)entry->data)->base_type == GLSL_TYPE_STRUCT);
> + assert(((glsl_type *)entry->data)->length == num_fields);
> + assert(strcmp(((glsl_type *)entry->data)->name, name) == 0);
>
> mtx_unlock(&glsl_type::mutex);
>
> - return t;
> + return (glsl_type *)entry->data;
> }
>
>
> @@ -805,25 +807,28 @@ glsl_type::get_interface_instance(const glsl_struct_field *fields,
> mtx_lock(&glsl_type::mutex);
>
> if (interface_types == NULL) {
> - interface_types = hash_table_ctor(64, record_key_hash, record_key_compare);
> + interface_types = _mesa_hash_table_create(NULL, record_key_hash,
> + record_key_compare);
> }
>
> - const glsl_type *t = (glsl_type *) hash_table_find(interface_types, & key);
> - if (t == NULL) {
> + const struct hash_entry *entry = _mesa_hash_table_search(interface_types,
> + &key);
> + if (entry == NULL) {
> mtx_unlock(&glsl_type::mutex);
> - t = new glsl_type(fields, num_fields, packing, block_name);
> + const glsl_type *t = new glsl_type(fields, num_fields,
> + packing, block_name);
Indentation seems to be wrong here.
> mtx_lock(&glsl_type::mutex);
>
> - hash_table_insert(interface_types, (void *) t, t);
> + entry = _mesa_hash_table_insert(interface_types, t, (void *) t);
> }
>
> - assert(t->base_type == GLSL_TYPE_INTERFACE);
> - assert(t->length == num_fields);
> - assert(strcmp(t->name, block_name) == 0);
> + assert(((glsl_type *)entry->data)->base_type == GLSL_TYPE_INTERFACE);
> + assert(((glsl_type *)entry->data)->length == num_fields);
> + assert(strcmp(((glsl_type *)entry->data)->name, block_name) == 0);
>
> mtx_unlock(&glsl_type::mutex);
>
> - return t;
> + return (glsl_type *)entry->data;
> }
>
>
> diff --git a/src/glsl/glsl_types.h b/src/glsl/glsl_types.h
> index c48977c..87104cb 100644
> --- a/src/glsl/glsl_types.h
> +++ b/src/glsl/glsl_types.h
> @@ -707,7 +707,7 @@ private:
> /** Hash table containing the known interface types. */
> static struct hash_table *interface_types;
>
> - static int record_key_compare(const void *a, const void *b);
> + static bool record_key_compare(const void *a, const void *b);
> static unsigned record_key_hash(const void *key);
>
> /**
More information about the mesa-dev
mailing list