kmaragon
commented
2 years ago This all looks super. I'm embarrassed to say that I didn't even remember writing the Argon2Memory class so it wasn't even on my radar once .NET introduced Span to migrate. I'm very glad someone thought of it. My only remaining request is that you please bump the minor version on the packages:
1.1.0 for Blake2 1.3.0 for Argon2 (with reference update to the above)
And then I shall merge.
Great work and huge thanks!
Insomniak47
Sorry about the delay on this I poked around a bit and had to swap syntax because net4.x doesn't nicely support the range operator that I was using extensively.
This improves the perf of the lib generally by removing the Argon2Memory class and using System.Memory and spans in its place. This prevents allocations in the hot loop and makes things work a little bit nicer. Also blitting was simplified and made into an extension on Memory which works out a bit nicer.
I've added a benchmark suite and a comparison harness to ensure that things are still working:
The results of the benchmark are below (Note the before and after). As iterations increase the memory savings becomes larger and larger.
I also fixed a bug where the blake initializer was never called and always assigned to the default. I believe that's #42 You'll notice that there are a few formatting changes that were just victims of my "format current file" muscle memory.
Let me know if there's any changes you'd like made or any concerns and I'll try to get to them pretty soon. I'll get to some cleanup this weekend as well
closes #48 closes #42
Before:
Low end of mem/iter:
High end:
After:
Low end of memory/iter:
high end:
I've attached the dataset as MD below:
Diff.md