-
-
Notifications
You must be signed in to change notification settings - Fork 2
Rework HashUtils to support Alder32 and CRC32 and be faster #12
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
base: master
Are you sure you want to change the base?
Conversation
Not gonna require it for approval, but a chart/comparison of this in Mavenizer would be helpful to visualize the gains. |
HashFunction func = functions[x]; | ||
MessageDigest digest = digests[x]; | ||
ret[x] = func.pad(new BigInteger(1, digest.digest()).toString(16)); | ||
ret[x] = functions[x].hash(file); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was actually explicitly written this way because the disc IO is the main concern when hashing files. Doing it this way makes things a lot slower because the entire file needs to be read for each hash function, instead of once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed on Discord, please re-work this PR using static getter methods instead of adding more public API.
https://discord.com/channels/1129059589325852724/1129095235889270844/1419795694625099866
I did this a while back and forgot about it, whoops!
Needs testing and review before merging.