Skip to content

Conversation

@mdumandag
Copy link
Contributor

We were always passing a bytes-like object to the murmur hash function
but, we were copying it from the HEAP_DATA_OVERHEAD(8) offset, which
can be huge. I removed that copy and tuned the offsets according to our
HEAP_DATA_OVERHEAD.

Another parameter of the function was the data size. For valid Data
objects, this is always equal to the len(buffer) - HEAP_DATA_OVERHEAD.
This parameter is removed and this length is calculated inside the
murmur hash function now.

Also, inlined the _fmix function call and removed the unnecessary
method calls on hash_to_index function.

@mdumandag mdumandag added this to the 4.0 milestone Nov 10, 2020
@mdumandag mdumandag self-assigned this Nov 10, 2020
@mdumandag mdumandag force-pushed the murmur-hash branch 2 times, most recently from e1dcadd to 0295ccc Compare November 10, 2020 08:49
We were always passing a bytes-like object to the murmur hash function
but, we were copying it from the HEAP_DATA_OVERHEAD(8) offset, which
can be huge. I removed that copy and tuned the offsets according to our
HEAP_DATA_OVERHEAD.

Another parameter of the function was the data size. For valid Data
objects, this is always equal to the `len(buffer) - HEAP_DATA_OVERHEAD`.
This parameter is removed and this length is calculated inside the
murmur hash function now.

Also, inlined the _fmix function call and removed the unnecessary
method calls on hash_to_index function.
Fix the `__hash__` method of the `Data` class to convert
`bytes` to `bytearray` if it executing on Python2 since we cannot
use murmur hash on `str`(alias of `bytes` on Python2) objects. This
extra if check is not necessary on `hash_code` method since
it is only called on the Data objects to be sent by the client
which will have a bytearray buffer.

Also, extra test cases are added
@mdumandag
Copy link
Contributor Author

Thanks for the review Andrey

@mdumandag mdumandag merged commit 419e133 into hazelcast:master Nov 11, 2020
@mdumandag mdumandag deleted the murmur-hash branch November 11, 2020 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants