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

[Kenneth Graunke]

On Tuesday, July 14, 2015 04:26:50 PM Timothy Arceri wrote:
> 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.

Yup, just an idea - what you had was fine.  Thanks!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150714/77f2be07/attachment.sig>