Page MenuHomeFreeBSD

Murmur3 implementation by des@ to replace Jenkins hash
ClosedPublic

Authored by gnn on Jul 21 2014, 8:56 PM.

Details

Reviewers
emaste
des
Summary

Previous version of Murmur3 patch had several issues. Dag-Egling graciously stepped forward with his own implmentation.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

gnn updated this revision to Diff 792.
gnn retitled this revision from to Murmur3 implementation by des@ to replace Jenkins hash.
gnn updated this object.
gnn edited the test plan for this revision. (Show Details)
gnn added reviewers: des, emaste, rpaulo.

Typo in the flowtable call of murmur3

You replaced the original diff with a new diff relative to the old one. This makes it impossible to comment on the complete diff.

You need to click Diff1 to see the original diff.

In D461#6, @gnn wrote:

You need to click Diff1 to see the original diff.

That's not very useful. I don't want to see the original diff, I want to see the complete diff.

gnn edited edge metadata.

Try to get a single diff in place using the arc update command.

sys/libkern/murmur3_32.c
27–30

<sys/types.h> should always come first, and <sys/stdint.h> is redundant.

84

<string.h> isn't needed.

sys/sys/hash.h
123–130

The comment should note that this is intended for aligned data in multiples of four bytes, and / or we should have separate aligned and unaligned versions of the code.

sys/libkern/murmur3_32.c
37

Should we have a different name for this function, due to the multiple-of-4 issue? (I.e., other implementations called murmur3_32 handle the 0-3 byte tail.)

Abandoned in favor of 920.

Adding the 'D' will make a link automatically: D920

The comment should note that this is intended for aligned data in multiples of four bytes, and / or we should have separate aligned and unaligned versions of the code.

I strongly believe we should provide the same api (alignment and size constraints) as other murmur3_32 implementations if we make that the function's name. If we're happy with the alignment and multiple-of-four requirement (I am) we should update the name somehow to reflect that.

Just add _aligned to the function name and a KASSERT() at the top.

Upated with a name change. Also, leave Jenkins in place for the other
consumer of that hash.

emaste edited edge metadata.
emaste added inline comments.
sys/sys/hash.h
128

drop the extra whitespace

This revision is now accepted and ready to land.Oct 9 2014, 8:09 PM
des edited edge metadata.

Good to commit.