KeccakHash minor opt - #10
Conversation
Benchmark below + it required one more pair of eyes Method | AllZeros | Mean | Error | StdDev | Scaled | MeadowHashSpan | False | 620.4 ns | 1.379 ns | 1.152 ns | 1.00 | MeadowHashBytes | False | 615.3 ns | 1.902 ns | 1.779 ns | 1.00 | New | False | 479.3 ns | 1.464 ns | 1.223 ns | 1.00 | HashLib | False | 1,228.6 ns | 1.927 ns | 1.609 ns | 1.00 | MeadowHashSpan | True | 617.6 ns | 6.739 ns | 6.304 ns | 1.00 | MeadowHashBytes | True | 614.5 ns | 7.798 ns | 6.912 ns | 1.00 | New | True | 474.5 ns | 1.596 ns | 1.333 ns | 1.00 | HashLib | True | 1,307.2 ns | 27.010 ns | 52.680 ns | 1.00 |
|
Thanks for the improvement! Looks like some tests are broken. Can you fix those? |
|
Yes, I had some changes in other places that I did not want to touch in your code and did not remove them properly from this commit - I hope I will get a moment soon to fix the PR. I was wondering about the stackallocs too at the beginning but they have been behaving well so far. Please note that we only use the code for Keccak256 so the stackallocs are not so big in the end. I imagine it may be more of a problem with bigger hash sizes. What was the nature of problems that you experienced? Which runtimes were affected? We were running it at all three platforms without issues. |
Benchmark below + it required one more pair of eyes
Method | AllZeros | Mean | Error | StdDev | Scaled |
MeadowHashSpan | False | 620.4 ns | 1.379 ns | 1.152 ns | 1.00 |
MeadowHashBytes | False | 615.3 ns | 1.902 ns | 1.779 ns | 1.00 |
New | False | 479.3 ns | 1.464 ns | 1.223 ns | 1.00 |
HashLib | False | 1,228.6 ns | 1.927 ns | 1.609 ns | 1.00 |
MeadowHashSpan | True | 617.6 ns | 6.739 ns | 6.304 ns | 1.00 |
MeadowHashBytes | True | 614.5 ns | 7.798 ns | 6.912 ns | 1.00 |
New | True | 474.5 ns | 1.596 ns | 1.333 ns | 1.00 |
HashLib | True | 1,307.2 ns | 27.010 ns | 52.680 ns | 1.00 |