[Mesa-dev] [PATCH] glsl: free interface_types and replace old hash_table uses

[Timothy Arceri]

On Mon, 2015-07-13 at 22:19 -0700, Kenneth Graunke wrote:
> On Monday, July 13, 2015 11:21:05 AM Iago Toral wrote:
> > On Sat, 2015-07-11 at 10:13 +1000, Timothy Arceri wrote:
> > > @@ -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).
> 
> Or...why not continue to have a local variable t, and just set t =
> entry->data?  Then these could all stay the same, and there would be
> less casting.

I did have it like that but in my opinion it just looked messy. 2 extra lines
vs 3 extra casts all of which are in asserts(), it felt like I was making the
surrounding code worse for the sake of the asserts.

I've pushed this already so I guess it doesn't matter now anyway.