search

cloudflare / pingora

A library for building fast, reliable and evolvable network services.
Apache License 2.0
21.03k stars 1.16k forks source link

Slight performance boost in `pingora-limit::Estimator` #16

Closed atamakahere-git closed 4 months ago

atamakahere-git commented 6 months ago

I was tinkering with the code, to my surprise I made Estimator run faster just by using iterators instead of for-loops

The overall diff is about 60% faster on single thread

Pingora Estimator single thread 2.334566806s total, 23ns avg per operation

vs

Pingora Estimator single thread 868.937015ms total, 8ns avg per operation

There is 2ns gain per thread, from 25ns -> 23ns (8%).

I have attached the complete bench diff for my machine: https://www.diffchecker.com/9EnuDEot/

I may be missing/ignoring something here, cause this is too good to be true.

Though this is a documented and tested behavior.

I have also added a test for reset method, which might be redundant.

atamakahere-git commented 6 months ago

Hey @fee1-dead , I applied your suggested changes, and it seems like .min() is loosing performance compared to .fold(). Can you run the benchmark and tell if that's the same case on other machines too?

fee1-dead commented 6 months ago

I don't have much time to run benchmarks myself, but I can trust the results. fold doesn't look that bad stylistically when compared to min.

gumpt commented 4 months ago

This has been merged to the main branch. Thank you for contributing!