ristretto icon indicating copy to clipboard operation
ristretto copied to clipboard

ristretto/z: use hash/memhash for hashing

Open rogpeppe opened this issue 4 years ago • 4 comments

This is cleaner, but unfortunately slower.

Reusing the Hash value in KeyToHash may help there, but wouldn't get back to the original speed, so maybe this isn't a reasonable change.

name             old time/op    new time/op    delta
MemHash-4          12.4ns ± 7%    43.1ns ± 1%  +248.30%  (p=0.008 n=5+5)
MemHashString-4    19.6ns ± 7%    70.9ns ±20%  +262.00%  (p=0.008 n=5+5)

name             old speed      new speed      delta
MemHash-4        5.17GB/s ± 7%  1.48GB/s ± 0%   -71.31%  (p=0.008 n=5+5)
MemHashString-4  6.29GB/s ± 6%  1.75GB/s ±18%   -72.12%  (p=0.008 n=5+5)

name             old alloc/op   new alloc/op   delta
MemHash-4           0.00B          0.00B           ~     (all equal)
MemHashString-4     0.00B          0.00B           ~     (all equal)

name             old allocs/op  new allocs/op  delta
MemHash-4            0.00           0.00           ~     (all equal)
MemHashString-4      0.00           0.00           ~     (all equal)

This change is Reviewable

rogpeppe avatar Jun 05 '20 22:06 rogpeppe

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

CLAassistant avatar Jun 05 '20 22:06 CLAassistant

Thanks for the PR, @rogpeppe . Yeah, the speeds don't look encouraging, though I'm wondering why that's the case. My understanding was that Go 1.14 exposed the same functions.

manishrjain avatar Jun 05 '20 23:06 manishrjain

Yeah, me too. I did it in a rush over about 20 minutes before bed last night so didn't have time to investigate more. I'm interested why it's so much slower - I might take another look today.

rogpeppe avatar Jun 06 '20 06:06 rogpeppe

Just peeking at your code, I don't think you're using maphash the way it's meant to be used. You're supposed to use one maphash struct per goroutine, you're not supposed to create a fresh struct for each hash call. But that's what the code does. This leads to a ton of unneeded allocation. So I'm not surprised this winds up being slower than the original.

nes1983 avatar Jun 10 '20 17:06 nes1983