<div dir="ltr"><div><div>I'm starting to contribute and Timothy Arceri told me about this task. The change was made at that sole file because we talked about no need to migrate all tables at once, but we discuss this and I take note to migrate all hash tables at once if everyone </div>agrees. </div>PS.: I forgot to add Reviewed-by Timothy Arceri at the commit message. If we are ok with migrating in steps, I'll edit it and send the patch again.</div><div class="gmail_extra"> <div class="gmail_quote">2015-08-03 2:40 GMT+02:00 Chris Forbes <<a href="mailto:chrisf@ijw.co.nz" target="_blank">chrisf@ijw.co.nz</a>>: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Oh, fair enough then.<div class="HOEnZb"><div class="h5"> <div class="gmail_quote">On Aug 3, 2015 12:39 PM, "Ilia Mirkin" <<a href="mailto:imirkin@alum.mit.edu" target="_blank">imirkin@alum.mit.edu</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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 <<a href="mailto:chrisf@ijw.co.nz" target="_blank">chrisf@ijw.co.nz</a>> wrote: > Some perf numbers would be nice. How much is this winning? > > - Chris > > On Mon, Aug 3, 2015 at 11:18 AM, Timothy Arceri <<a href="mailto:t_arceri@yahoo.com.au" target="_blank">t_arceri@yahoo.com.au</a>> 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 <<a href="mailto:t_arceri@yahoo.com.au" target="_blank">t_arceri@yahoo.com.au</a>> >> >> _______________________________________________ >> mesa-dev mailing list >> <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> >> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> > _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> > <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div></blockquote></div> </div>