Closed ongardie-ebay closed 5 years ago
I haven't forgotten about this, just busy. I'll try to get to it this week.
Hey @cespare, just a quick reminder that this PR is waiting for your review. I know it's a busy time of year, so no big deal if you can't fit this in yet.
Yeah, it's on my list. Sorry, just been busy.
@ongardie-ebay hey, I finally got to this. Sorry for the long delay.
I made some changes on the digest branch and merged that. I changed the base of this PR to a temporary branch just to keep it in the same state for now.
To reduce back-and-forth I made followup changes myself on a different branch. I made a v2
branch from master and rebased your changes on that. Then I opened #16 against the v2
branch. Please review that PR.
The changes here will be merged as part of the v2
branch merge so we can close this PR unless there's anything that's easier to discuss here rather than on #16.
Cool. We probably could have sequenced our efforts more simply, but this works. I benchmarked this PR (old=900e0d4) vs the rebase in v2 (new=f728abc) and confirmed that v2 is strictly better:
name old time/op new time/op delta
tags:!purego
Hashes/xxhash,direct,bytes,n=100_B-4 15.4ns ± 1% 15.3ns ± 0% -0.82% (p=0.000 n=6+7)
Hashes/xxhash,direct,string,n=100_B-4 17.6ns ± 1% 17.5ns ± 1% ~ (p=0.142 n=7+7)
Hashes/xxhash,digest,bytes,n=100_B-4 28.1ns ±13% 27.4ns ± 1% ~ (p=0.854 n=6+7)
Hashes/xxhash,digest,string,n=100_B-4 30.2ns ± 1% 30.4ns ± 5% ~ (p=0.384 n=7+7)
tags:purego
Hashes/xxhash,direct,bytes,n=100_B-4 16.4ns ± 1% 16.4ns ± 0% ~ (p=0.449 n=7+7)
Hashes/xxhash,direct,string,n=100_B-4 18.5ns ± 1% 19.2ns ± 8% ~ (p=0.092 n=7+7)
Hashes/xxhash,digest,bytes,n=100_B-4 36.8ns ± 1% 29.4ns ±14% -20.18% (p=0.001 n=6+7)
Hashes/xxhash,digest,string,n=100_B-4 39.1ns ± 1% 33.3ns ± 7% -14.86% (p=0.001 n=7+7)
name old speed new speed delta
tags:!purego
Hashes/xxhash,direct,bytes,n=100_B-4 6.51GB/s ± 1% 6.55GB/s ± 1% +0.66% (p=0.035 n=6+7)
Hashes/xxhash,direct,string,n=100_B-4 5.68GB/s ± 1% 5.70GB/s ± 1% ~ (p=0.073 n=7+7)
Hashes/xxhash,digest,bytes,n=100_B-4 3.57GB/s ±12% 3.65GB/s ± 1% ~ (p=1.000 n=6+7)
Hashes/xxhash,digest,string,n=100_B-4 3.32GB/s ± 1% 3.29GB/s ± 5% ~ (p=0.535 n=7+7)
tags:purego
Hashes/xxhash,direct,bytes,n=100_B-4 6.08GB/s ± 1% 6.11GB/s ± 0% ~ (p=0.318 n=7+7)
Hashes/xxhash,direct,string,n=100_B-4 5.40GB/s ± 1% 5.22GB/s ± 8% ~ (p=0.053 n=7+7)
Hashes/xxhash,digest,bytes,n=100_B-4 2.72GB/s ± 1% 3.42GB/s ±13% +25.86% (p=0.001 n=6+7)
Hashes/xxhash,digest,string,n=100_B-4 2.56GB/s ± 1% 3.01GB/s ± 7% +17.58% (p=0.001 n=7+7)
name old alloc/op new alloc/op delta
tags:!purego
Hashes/xxhash,direct,bytes,n=100_B-4 0.00B 0.00B ~ (all equal)
Hashes/xxhash,direct,string,n=100_B-4 0.00B 0.00B ~ (all equal)
Hashes/xxhash,digest,bytes,n=100_B-4 0.00B 0.00B ~ (all equal)
Hashes/xxhash,digest,string,n=100_B-4 0.00B 0.00B ~ (all equal)
tags:purego
Hashes/xxhash,direct,bytes,n=100_B-4 0.00B 0.00B ~ (all equal)
Hashes/xxhash,direct,string,n=100_B-4 0.00B 0.00B ~ (all equal)
Hashes/xxhash,digest,bytes,n=100_B-4 0.00B 0.00B ~ (all equal)
Hashes/xxhash,digest,string,n=100_B-4 0.00B 0.00B ~ (all equal)
name old allocs/op new allocs/op delta
tags:!purego
Hashes/xxhash,direct,bytes,n=100_B-4 0.00 0.00 ~ (all equal)
Hashes/xxhash,direct,string,n=100_B-4 0.00 0.00 ~ (all equal)
Hashes/xxhash,digest,bytes,n=100_B-4 0.00 0.00 ~ (all equal)
Hashes/xxhash,digest,string,n=100_B-4 0.00 0.00 ~ (all equal)
tags:purego
Hashes/xxhash,direct,bytes,n=100_B-4 0.00 0.00 ~ (all equal)
Hashes/xxhash,direct,string,n=100_B-4 0.00 0.00 ~ (all equal)
Hashes/xxhash,digest,bytes,n=100_B-4 0.00 0.00 ~ (all equal)
Hashes/xxhash,digest,string,n=100_B-4 0.00 0.00 ~ (all equal)
This resolves #13. You may want to review the commits individually.
Note that merging this requires a major version bump (to 2.0.0).