glium icon indicating copy to clipboard operation
glium copied to clipboard

Suggestion: Improve HashMap lookup performance

Open Boscop opened this issue 4 years ago • 3 comments

Currently, glium uses fnv for HashMaps, but lookup could be made faster with FxHash.

https://github.com/cbreeden/fxhash#benchmarks

Generally fxhash is faster than fnv on u32, u64, or any byte sequence with length >= 5. However, keep in mind that hashing speed is not the only characteristic worth considering. That being said, Rustc had an observable increase in speed when switching from fnv backed hashmaps to fx based hashmaps.

Many things are looked up at each frame/draw call, e.g attributes / uniform strings, and they usually have length >= 5, so they would benefit from this.

I know it's only a small improvement, but it's basically for free so it makes sense to do it :) And the improvement is better with increasing byte length, e.g. at len 23 the lookup time is already halved with fxhash.

Boscop avatar Dec 15 '20 21:12 Boscop

Interesting proposal! Would be interesting to have some benchmarks comparing fnv with fxhash.

est31 avatar Dec 15 '20 21:12 est31

Actually I just found out that the std hash is now aHash:

Since Rust 1.36, this is now the HashMap implementation for the Rust standard library.

which is faster than both fnv and fxhash: https://github.com/tkaitchuck/aHash#comparison-with-other-hashers There is more detail here: https://github.com/tkaitchuck/aHash/blob/master/compare/readme.md#speed

So it probably makes sense to switch back to std :)

Boscop avatar Dec 15 '20 22:12 Boscop

So it probably makes sense to switch back to std :)

Good point! Ideally there would be benchmarks comparing the three strategies and then we can choose one. Admittedly though it's hard to benchmark it as it's likely not very hot code. Still worth a try. I'm taking PRs that switch back to std even without benchmarks, although I'd prefer to have some.

est31 avatar Dec 15 '20 23:12 est31