[Optimize]: Murmur128 is a noncryptographic hash aglorithm#3919
Conversation
Fix&Optimize]: Murmur128 is a noncryptographic hash aglorithmOptimize]: Murmur128 is a noncryptographic hash aglorithm
There was a problem hiding this comment.
Pull Request Overview
This PR implements a noncryptographic version of the Murmur128 hash algorithm and optimizes its performance while updating the associated tests and helper methods.
- Implements Murmur128 as a noncryptographic hash algorithm by deriving from NonCryptographicHashAlgorithm.
- Refactors the hash calculation to use a new Append/ComputeHash pattern with improved tail handling.
- Updates tests and helper extensions to reflect the new API and improved performance.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/Neo.UnitTests/Cryptography/UT_Murmur128.cs | Test adjustments to use the new ComputeHash method and HashSizeInBits. |
| src/Neo/Cryptography/Murmur128.cs | Refactor of the Murmur128 implementation to optimize performance and tail processing. |
| src/Neo/Cryptography/Helper.cs | Update Murmur128 extension methods to use ReadOnlySpan-based implementation. |
| benchmarks/Neo.Benchmarks/Benchmarks.Hash.cs | Introduction of a benchmark for the Murmur128 performance. |
| for (; source.Length >= 16; source = source[16..]) | ||
| { | ||
| Mix(source); |
There was a problem hiding this comment.
[nitpick] Consider refactoring this loop to use an index-based iteration instead of repeatedly slicing the source span. This may help reduce overhead in performance-critical paths.
There was a problem hiding this comment.
@Wi1l-B0t Copilot is right, avoiding coy the array and using index it will be faster
There was a problem hiding this comment.
@Wi1l-B0t Copilot is right, avoiding coy the array and using index it will be faster
The source is a span. Slice is fast.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…roject#3919) * fix: murmur128 is a noncryptographic hash aglorithm * Update src/Neo/Cryptography/Murmur128.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Shargon <shargon@gmail.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Description
Same as the murmur32, murmur128 is a noncryptographic hash aglorithm.
So it shouldn't be implemented as a cryptographic hash (This would be misleading.):
This PR:
A simple benchmark:
Before:
After:
Fixes # (issue)
Type of change
Checklist: