Skip to content
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

feat: non-ASCII support and optimization in murmurhash3 #931

Merged
merged 11 commits into from
Aug 10, 2023

Conversation

dj-stormtrooper
Copy link
Contributor

@dj-stormtrooper dj-stormtrooper commented Jul 30, 2023

I added proper support for non-ASCII symbols for murmurhash3_128_x64. Previous implementation wasn't compatible with implementation in other languages (e.g. Golang, C++) for symbols with charcodes outside of 0 - 0xff range. Also it was limited in terms of uniqueness, e.g. hash('Hello, world, hi') was equal to hash('ňťŬŬůĬĠŷůŲŬŤĬĠŨũ').
Current new implementation are free of these issues

Also I refactored internal functions to achieve minimal minimal memory allocations and reduce GC load, it gained performance boost:

  • Chrome: 27%
  • Webkit: 58%
  • Firefox: 45%

Optimizations:

  • the number of new object creation is minimized
  • all int64 values (int32 tuples) are passed to mutable functions, and the result is written back to the value
  • int64 constants are defined outside the scope if possible

Benchmark (sorry for the minified code in the benchmark, jsbench allows to save suite only if fields are below 2048 symbols, so I tried to make it more compact)

JSBench screenshots image

Webkit

image

Chrome

image

Firefox

Other implementations that I've tried:

Copy link
Member

@Finesse Finesse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please list shortly the optimizations that you applied in the PR description

src/utils/data.ts Outdated Show resolved Hide resolved
src/utils/hashing.test.ts Outdated Show resolved Hide resolved
src/utils/hashing.ts Outdated Show resolved Hide resolved
src/utils/hashing.ts Show resolved Hide resolved
src/utils/hashing.ts Show resolved Hide resolved
@dj-stormtrooper
Copy link
Contributor Author

Please list shortly the optimizations that you applied in the PR description

Done

@dj-stormtrooper dj-stormtrooper merged commit e34c8e3 into master Aug 10, 2023
3 checks passed
@Finesse Finesse deleted the feat/murmur3hash-optimized branch August 10, 2023 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants