New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix hash function and hash table size in Snappy #7038

Closed
wants to merge 1 commit into
base: 4.1
from

Conversation

Projects
None yet
3 participants
@fenik17
Contributor

fenik17 commented Jul 30, 2017

Motivation:

  1. Hash function in the Snappy encoding is wrong probably: used + instead of *. See the reference implementation [1].
  2. Size of the hash table is calculated, but not applied.

Modifications:

  1. Fix hash function: replace addition by multiplication.
  2. Allocate hash table with calculated size.
  3. Use an Integer.numberOfLeadingZeros trick for calculate log2.
  4. Release buffers in tests.

Result:

  1. Better compression. In the test encodeAndDecodeLongTextUsesCopy now compressed size is 175 instead of 180 before this change.
  2. No extra allocations for hash table.
  3. A bit faster the calc of shift (less expensive math operations).

[1] https://github.com/google/snappy/blob/513df5fb5a2d51146f409141f9eb8736935cc486/snappy.cc#L67

Fix hash function and hash table size in Snappy
Motivation:

1. Hash function in the Snappy encoding is wrong probably: used '+' instead of '*'. See the reference implementation [1].
2. Size of the hash table is calculated, but not applied.

Modifications:

1. Fix hash function: replace addition by multiplication.
2. Allocate hash table with calculated size.
3. Use an `Integer.numberOfLeadingZeros` trick for calculate log2.
4. Release buffers in tests.

Result:

1. Better compression. In the test `encodeAndDecodeLongTextUsesCopy` now compressed size is 175 instead of 180 before this change.
2. No redundant allocations for hash table.
3. A bit faster the calc of shift (less an expensive math operations).

[1] https://github.com/google/snappy/blob/513df5fb5a2d51146f409141f9eb8736935cc486/snappy.cc#L67

@normanmaurer normanmaurer requested review from lw346 and Scottmitch Jul 31, 2017

@lw346

lw346 approved these changes Jul 31, 2017

@normanmaurer

This comment has been minimized.

Show comment
Hide comment
@normanmaurer

normanmaurer Aug 1, 2017

Member

Cherry-picked into 4.1 (6ab9c17) and 4.0 (287e147)

@fenik17 thanks!

Member

normanmaurer commented Aug 1, 2017

Cherry-picked into 4.1 (6ab9c17) and 4.0 (287e147)

@fenik17 thanks!

@normanmaurer normanmaurer self-assigned this Aug 1, 2017

@normanmaurer normanmaurer added this to the 4.0.50.Final milestone Aug 1, 2017

@normanmaurer normanmaurer added the defect label Aug 1, 2017

@fenik17 fenik17 deleted the fenik17:snappy_fixes branch Aug 1, 2017

@fenik17

This comment has been minimized.

Show comment
Hide comment
@fenik17

fenik17 Aug 1, 2017

Contributor

@normanmaurer is not pushed into 4.1?

Contributor

fenik17 commented Aug 1, 2017

@normanmaurer is not pushed into 4.1?

@normanmaurer

This comment has been minimized.

Show comment
Hide comment
@normanmaurer

normanmaurer Aug 1, 2017

Member

@fenik17 ups... fixed :)

Member

normanmaurer commented Aug 1, 2017

@fenik17 ups... fixed :)

@fenik17

This comment has been minimized.

Show comment
Hide comment
@fenik17

fenik17 Aug 1, 2017

Contributor

Thanks :)

Contributor

fenik17 commented Aug 1, 2017

Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment