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 <ito...@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); > > /** _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev