rtfeldman / elm-css

Typed CSS in Elm.
https://package.elm-lang.org/packages/rtfeldman/elm-css/latest
BSD 3-Clause "New" or "Revised" License
1.23k stars 196 forks source link

Hash Collisions: FNV vs Murmur3 #551

Open rtfeldman opened 2 years ago

rtfeldman commented 2 years ago

Given https://mobile.twitter.com/matheusdev23/status/1459272654544904204 I'm thinking we should probably stick with Murmur3 for now. Hash collision bugs would be extremely painful to detect and debug in user space, and I don't really think it's worth the extra performance unless we're super confident they won't happen. 😅

Thoughts @robinheghan?

robinheghan commented 2 years ago

FNV1a will lead to more collisions when compared to Murmur3. But from what I've seen, not much more. There are more links in the link matheusdev23 mentions, which verifies this.

This also means that it's likely that hash collisions have happened in some project in the real world today.

The main problem, really, is that we have no collision handling.

Since Bekk, my employer, let's me spend one day every week working on stuff for the benefit of the larger elm community, perhaps we could do a slack meeting, @rtfeldman, and explain some of the code in elm-css to me. With a better understanding of the current code base, I might be able to make the code faster and avoid collisions at the same time. Reach out to me on slack if you think this is a good idea :)