[FFmpeg-devel] [PATCH] Add 128 bit murmur3 hash function.

[Reimar Döffinger]

On 10.05.2013, at 04:14, Michael Niedermayer <michaelni at gmx.at> wrote:
>> +{
>> +    uint64_t k = get_k1(src);
>> +    k ^= c->h1;
>> +    k = ROT(k, 27);
>> +    k += c->h2;
>> +    k *= 5;
>> +    k += 0x52dce729;
>> +    c->h1 = k;
>> +
>> +    k = get_k2(src);
>> +    k ^= c->h2;
>> +    k = ROT(k, 31);
>> +    k += c->h1;
>> +    k *= 5;
>> +    k += 0x38495ab5;
>> +    c->h2 = k;
>> +}
> 
> interleaving the operations a bit might improve performance

How? The generated code looked fairly good to me, the biggest issue seems to be that gcc is too stupid to eliminate the context loads/stores inside the loop.
Doing that manually would mean using e.g. a macro instead of a function, which seems a bit ugly.
My quick test indicated it runs at about 2.5 GB/s on 64 bit (possibly memory bandwidth limited) and 1 GB/s on 32 bit, so I felt like it might not be worth optimizing.

>> +    uint8_t hash_result[8] = {0};
> 
> array too small

Embarassing.
Btw. the missing free is fixed locally.
I currently can't get valgrind to work on my PC, otherwise I'd have found these.
If someone can quickly run valgrind on the murmur3-test binary after fixing these that's welcome, otherwise I guess the FATE machine will do it once it's in.