Increase performance for fitness_score_cardinality and genes_cardinality#10
Increase performance for fitness_score_cardinality and genes_cardinality#10tomtomwombat wants to merge 1 commit intobasvanwesting:mainfrom
Conversation
…cardinality and genes_cardinality
|
Thanks, I see you are the author of hyperloglockless. While the claimed performance increase is relevant percentage-wise, I'm not sure the cardinality estimation is a non-trivial performance bottleneck in practice. You did not provide evidence for that claim (especially with respect to the Fitness calculations, which are the bottleneck in real-world usage). Also, our use case centers on very low cardinality counts: typically between 100 and 1000. So that is very limited use case, we need to keep that in mind. It will have special performance characteristics for these low levels. Furthermore cardinality-estimator has a benchmark with conflicting conclusions regarding your implementation. I will consider the switch. If you can provide evidence of the non-trivial performance bottleneck in real-world usage, that would make a difference. Regards, Bas |
|
Thanks for the quick response
Which benchmark and conflicting conclusion are you referring to? They don't include
That's useful context. What do you mean by "It will have special performance characteristics for these low levels."? Also, I'm curious what you prioritize in cardinality estimation in
I don't use |
Cardinality estimation in calls to
fitness_score_cardinalityandgenes_cardinalityis a non-trivial performance bottleneck.This PR replaces the
cardinality-estimatorwithhyperloglocklessto optimize those calls. I chosefoldhashbecause it's fast (especially for small inputs) and because cardinality estimation doesn't need to be deterministic (correct me if I'm wrong on this! We can use a different hasher otherwise).HyperLogLog::new(12);uses the same memory asCardinalityEstimator::<u64>::new(): 2^12 bytes.The accuracy of estimation for small cardinalities is unchanged while being improved for cardinalities larger than 10^7 since
cardinality-estimatorno longer provides accurate estimation then (though such large cardinalities may be outside your use-case). You can find more performance and accuracy comparisons here.In addition,
hyperloglocklessuses considerably less dependencies thancardinality-estimator.The below benchmarks show before and after change (other benchmarks are not affected):
These benchmarks were run with