serenity icon indicating copy to clipboard operation
serenity copied to clipboard

Everywhere: Port remaining integer hash users to SipHash (and invent a cool multi-element hash function in the process!)

Open kleinesfilmroellchen opened this issue 2 years ago • 9 comments

Everything necessary to actually remove the old integer hash functions entirely, including the HashFunctions header.

The new multi_hash function hashes any number of elements of any type by first hashing non-trivial (non-integer or float) types with their Trait hash, then hashing them together in a u64 array. This reduces the overhead to one hash function call as compared to O(log(n))-like tree hashes* when the number of elements hashed is known at compile time, which is quite often the case. Of course, it can also just be used as a drop-in replacement for pair_int_hash, but a lot of code gets cleaner by using multi_hash properly. :^)

(*)Each hash function call is not constant-time of course, so the runtime analysis is not accurate. However, since most hashing work is performed at finalization, running the hash function less often is always faster by at least a factor 2 (for SipHash-4-8).


AK+Everywhere: Use a SipHash-based pointer hash

AK+Everywhere: Use a trivial bitwise hash where applicable

This simplifies a significant portion of users with many input values, all of them were just hashing an entire larger trivial struct anyways.

AK: Add hash traits to BigEndian&LittleEndian

AK+Everywhere: Use a multi-element hash to replace pair_hash

This function allows hashing any number of heterogeneous elements, which is the most common application of pair hash. The use of this method instead of many pair hash calls reduces the number of hash function calls significantly and makes the code more readable. It also propagates SipHash usage to most remaining users.

AK: Use direct byte hash for Span

This is faster since we don't need to run finalization rounds every time.

Everywhere: Port remaining users of old hash functions and remove them

This is a mixture of different changes that could each be their own commit, but are small enough to all fit together with the removal.

kleinesfilmroellchen avatar Oct 01 '23 15:10 kleinesfilmroellchen

Does not compile according to CI

gmta avatar Oct 01 '23 21:10 gmta

Does not compile according to CI

For some reason GNU wants to constexpr some stuff in LibWebView, which most likely doesn't matter.

kleinesfilmroellchen avatar Oct 02 '23 11:10 kleinesfilmroellchen

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!

stale[bot] avatar Oct 24 '23 04:10 stale[bot]

This pull request has been closed because it has not had recent activity. Feel free to re-open if you wish to still contribute these changes. Thank you for your contributions!

stale[bot] avatar Nov 01 '23 02:11 stale[bot]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!

stale[bot] avatar Nov 23 '23 22:11 stale[bot]

This pull request has been closed because it has not had recent activity. Feel free to re-open if you wish to still contribute these changes. Thank you for your contributions!

stale[bot] avatar Dec 01 '23 11:12 stale[bot]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!

stale[bot] avatar Jan 26 '24 06:01 stale[bot]

I would rather we change direction on this and only use SipHash in places where the security considerations are applicable. Meaning, anywhere that using a "bad hash" could result in a TCP port attack, or other quantifiable scenario where letting untrusted user input control collisions is no bueno.

It is quite sad to see SipHash showing up in almost every browser profile. Perhaps that's actually a separate issue?

ADKaster avatar Feb 01 '24 10:02 ADKaster

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!

stale[bot] avatar Feb 24 '24 23:02 stale[bot]

This pull request has been closed because it has not had recent activity. Feel free to re-open if you wish to still contribute these changes. Thank you for your contributions!

stale[bot] avatar Mar 03 '24 09:03 stale[bot]