search

sundy-li / simple_hll

A simple HyperLogLog implementation in rust
https://crates.io/crates/simple_hll
Apache License 2.0
3 stars 2 forks source link

ahash? #2

Open jianshu93 opened 3 weeks ago

jianshu93 commented 3 weeks ago

Hi Simple HLL team,

Is there a reason why ahash was used (I know ahash is fast and CoS attack resistant) but not xxh3 (rust version of xxhash) or wyhash-rs.

Thanks,

Jianshu

sundy-li commented 3 weeks ago

Hi, I just knew that ahash is fast so I used it.

I don't think there will be much more difference to switch to another hash crate. But fixed seed must be used to keep the result stable.

jianshu93 commented 3 weeks ago

Hi @sundy-li,

Yes the seed must be fixed:

use xxh3; /// Fixed seed for xxHash const SEED: u64 = 0x1A2B3C4D5E6F7856_u64; pub fn add_object(&mut self, obj: &T) { let hash = xxh3::hash64_with_seed(obj, SEED); self.add_hash(hash); }

In the case for speed, xxh3 in Rust is neon/sse or AVX2 supported and should be much faster according to this benchmark here (although it was in C++): https://xxhash.com/doc/v0.8.2/index.html

Not sure whehter it is worthwhile to add it.

Thanks,

Jianshu

sundy-li commented 3 weeks ago

We can support it with feature gate I think.

But the default hash must be ahash to keep compatible wth before.

Can you submit a pr to support this ?