[ovs-dev] [PATCHv5 07/12] hash: Add 128-bit murmurhash.

[Joe Stringer]

On 18 September 2014 11:03, Ben Pfaff <blp at nicira.com> wrote:

> On Mon, Sep 15, 2014 at 02:25:13PM +1200, Joe Stringer wrote:
> > Add the 128-bit murmurhash by Austin Appleby, for 32-bit systems from:
> > http://code.google.com/p/smhasher/source/browse/trunk/MurmurHash3.cpp
> >
> > Signed-off-by: Joe Stringer <joestringer at nicira.com>
>
> Why does the loop in hash_words128 count up from a negative number to
> zero?
>
> Please put spaces after commas and around binary infix operators,
> e.g. in these cases:
>     k1  = hash_rot(k1,15);
>     hash.h[0] = hash_rot(hash.h[0],19);
>     hash.h[0] = hash.h[0]*5+0x561ccd1b;
> and similar.
>

Style changes from the original to OVS that I didn't pick up on. I can
change these.


>
> It looks like there is a version of 128-bit murmurhash that uses 64-bit
> operations.  Did you consider it?  Do you have any idea whether there is
> a performance difference?
>

I didn't try that. I did try the 64-bit CityHash (64-bit operations) which
is based on MurmurHash and is meant to have higher performance, and I
couldn't determine much change. That said, I was testing revalidation as a
whole and not the individual hash functions.


>
> Did you look at the GCC-generated code to see that it was sane?
>

No. Do you have particular things that you look for? Are you concerned
about the ovs_u128 structure and whether access into/out of that is
sensible?


> Did you do any testing of the hash function output?  We have tests for
> our main hash function.
>

I can look into this.