[Mesa-dev] [PATCH] glsl: replace old hash table with new and faster one

[Ilia Mirkin]

Given that this is a debug-only thing, I doubt perf numbers are that
interesting.

I have no clue what the diff is between the two hash tables, but if
one is allegedly faster than the other, that should be determined, and
we should just mass-migrate...

  -ilia

On Sun, Aug 2, 2015 at 8:05 PM, Chris Forbes <chrisf at ijw.co.nz> wrote:
> Some perf numbers would be nice. How much is this winning?
>
> - Chris
>
> On Mon, Aug 3, 2015 at 11:18 AM, Timothy Arceri <t_arceri at yahoo.com.au> wrote:
>> On Sun, 2015-08-02 at 19:50 +0200, Alejandro Seguí wrote:
>>
>> Maybe just for completeness you could add this to the commit message
>>
>> The util/hash_table was intended to be a fast hash table
>> replacement for the program/hash_table see 35fd61bd99c1 and 72e55bb6888ff.
>>
>>> ---
>>>  src/glsl/ir_print_visitor.cpp | 19 ++++++++++++-------
>>>  1 file changed, 12 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/src/glsl/ir_print_visitor.cpp b/src/glsl/ir_print_visitor.cpp
>>> index 78475f3..641a996 100644
>>> --- a/src/glsl/ir_print_visitor.cpp
>>> +++ b/src/glsl/ir_print_visitor.cpp
>>> @@ -25,7 +25,7 @@
>>>  #include "glsl_types.h"
>>>  #include "glsl_parser_extras.h"
>>>  #include "main/macros.h"
>>> -#include "program/hash_table.h"
>>> +#include "util/hash_table.h"
>>>
>>>  static void print_type(FILE *f, const glsl_type *t);
>>>
>>> @@ -89,14 +89,14 @@ ir_print_visitor::ir_print_visitor(FILE *f)
>>>  {
>>>     indentation = 0;
>>>     printable_names =
>>> -      hash_table_ctor(32, hash_table_pointer_hash,
>>> hash_table_pointer_compare);
>>> +      _mesa_hash_table_create(NULL, _mesa_hash_pointer,
>>> _mesa_key_pointer_equal);
>>>     symbols = _mesa_symbol_table_ctor();
>>>     mem_ctx = ralloc_context(NULL);
>>>  }
>>>
>>>  ir_print_visitor::~ir_print_visitor()
>>>  {
>>> -   hash_table_dtor(printable_names);
>>> +   _mesa_hash_table_destroy(printable_names, NULL);
>>>     _mesa_symbol_table_dtor(symbols);
>>>     ralloc_free(mem_ctx);
>>>  }
>>> @@ -121,9 +121,14 @@ ir_print_visitor::unique_name(ir_variable *var)
>>>     }
>>>
>>>     /* Do we already have a name for this variable? */
>>> -   const char *name = (const char *) hash_table_find(this->printable_names,
>>> var);
>>> -   if (name != NULL)
>>> -      return name;
>>> +   struct hash_entry * entry =
>>> +        _mesa_hash_table_search(this->printable_names, var);
>>> +
>>> +   if (entry != NULL) {
>>> +      return (const char *) entry->data;
>>> +   }
>>> +
>>> +   const char* name = NULL;
>>
>> The above looks a bit funny just floating here maybe move it
>>
>>>
>>>     /* If there's no conflict, just use the original name */
>> Here.
>>>     if (_mesa_symbol_table_find_symbol(this->symbols, -1, var->name) ==
>>> NULL) {
>>> @@ -132,7 +137,7 @@ ir_print_visitor::unique_name(ir_variable *var)
>>>        static unsigned i = 1;
>>>        name = ralloc_asprintf(this->mem_ctx, "%s@%u", var->name, ++i);
>>>     }
>>> -   hash_table_insert(this->printable_names, (void *) name, var);
>>> +   _mesa_hash_table_insert(this->printable_names, var, (void *) name);
>>>     _mesa_symbol_table_add_symbol(this->symbols, -1, name, var);
>>>     return name;
>>>  }
>>
>> With those couple of small changes you can add to the commit message
>> Reviewed-by: Timothy Arceri <t_arceri at yahoo.com.au>
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev