On Sun, 2015-08-02 at 20:39 -0400, Ilia Mirkin wrote: > 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...
The main difference is program/hash_table.h does not resize itself. The number of buckets needs to be specified at creation time so if this is set too low we will start to hit performace problems [1]. However for most uses of the tables there will likely be no measurable difference, this will more likely just be a clean-up exercise rather than a performance win. As far as I can tell all new code is already encouraged to use the new hash (as nir does) and it just that no-one has gotten around to replacing the old hash [2]. [1] http://lists.freedesktop.org/archives/mesa-dev/2012-October/029074.html [2] http://lists.freedesktop.org/archives/mesa-dev/2013-December/050524.html > > -ilia > > On Sun, Aug 2, 2015 at 8:05 PM, Chris Forbes <chr...@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_arc...@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_arc...@yahoo.com.au> > > > > > > _______________________________________________ > > > mesa-dev mailing list > > > mesa-dev@lists.freedesktop.org > > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > > _______________________________________________ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
