<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Nov 25, 2014 at 3:40 PM, Eric Anholt <span dir="ltr"><<a href="mailto:eric@anholt.net" target="_blank">eric@anholt.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> writes:<br>
<br>
> Previously, the hash_table API required the user to do all of the hashing<br>
> of keys as it passed them in.  Since the hashing function is intrinsicly<br>
> tied to the comparison function, it makes sense for the hash table to know<br>
> about it.  Also, it makes for a somewhat clumsy API as the user is<br>
> constantly calling hashing functions many of which have long names.  This<br>
> is especially bad when the standard call looks something like<br>
><br>
> _mesa_hash_table_insert(ht, _mesa_pointer_hash(key), key, data);<br>
><br>
> In the above case, there is no reason why the hash table shouldn't do the<br>
> hashing for you.  We leave the option for you to do your own hashing if<br>
> it's more efficient, but it's no longer needed.  Also, if you do do your<br>
> own hashing, the hash table will assert that your hash matches what it<br>
> expects out of the hashing function.  This should make it harder to mess up<br>
> your hashing.<br>
<br>
</span>I went to go port this change to the upstream repo, and found a commit<br>
laying around from cworth that did the same thing.  He called the<br>
renamed functions "_pre_hashed", and included sets as well.  I think I<br>
prefer that name slightly?  What do you think?  I've pushed his change<br>
to the upstream (git+ssh://<a href="http://people.freedesktop.org/~anholt/hash_table" target="_blank">people.freedesktop.org/~anholt/hash_table</a>).<br></blockquote><div><br></div><div>That looks fine to me.  It looks mostly like what I did.  The one thing I think my patch has over Carl's is that I split the insert_pre_hashed function into an internal and an external version.  If you call the external version, it asserts that the hash value from the internal hashing function matches the hash value you gave it.  That way you'll notice if you ever mix up hashing functions.  If you'd like, I can make that a patch on top of what you have in your branch there.  Or you can do it and just push it.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
It's a really good idea, though, and I'm sorry I hadn't pushed the<br>
change and propagated it to Mesa already. :(<br></blockquote><div><br></div><div>That's ok.  Mesa doesn't use it that many places.  We don't really feel the pain until NIR which uses it about twice as much as the rest of mesa.<br></div><div>--Jason<br></div></div><br></div></div>