Skip to content

Conversation

@tkstanczak
Copy link

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 |

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 |
@zone117x
Copy link
Member

zone117x commented Dec 1, 2018

Thanks for the improvement! Looks like some tests are broken. Can you fix those?
Also can you verify that those large stackallocs don't cause issues on any runtimes? I've tried those for this hash code previously but ran into issues.

@tkstanczak
Copy link
Author

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.

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.

2 participants