Closed Bug 1377947 Opened 3 years ago Closed 3 years ago

Make MFBT hashing functions return 64-bit hashes on 64-bit platforms

Categories

(Core :: MFBT, enhancement)

enhancement
Not set

Tracking

()

RESOLVED WONTFIX

People

(Reporter: ehsan, Assigned: ehsan)

References

Details

A huge use case of these hash functions is to generate PLDHashNumbers.  With bug 1377333, right now in many cases we end up generating a 32-bit hash key and expand it to a 64-bit key by zero padding which is pretty bad in terms of the hash distribution.  We should similarly switch these functions to return size_t instead of uint32_t.

One big aspect that I am not going to change in this bug to reduce the amount of work involved is SpiderMonkey's size for hash keys.  I will file a follow-up bug for that.  Doing that probably requires some SpiderMonkey specific knowledge.
See Also: → 1377949
Depends on: 1378015
No longer depends on: 1378015
Depends on: 1378042
Depends on: 1378044
Depends on: 1378045
Depends on: 1378046
Turns out we won't need to do this because bug 1377333 was wontfixed.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
FWIW, this is something I've pondered doing, and/or thought we probably should do at some point.  Aside from laziness/immediate unimportance, the other thing that's sort of stopped me is the places that depend upon/expect the 32-bitness of hashes, sometimes implicitly.  Perhaps that can be worked through with helper functions to massage a hash of one size into a hash of another, maybe.  Hasn't been worth the trouble without a pressing use case to figure out the ergonomics, tho.  But I'm totally fine reopening this when a good need arises.
...and while I'm thinking about it, it'd also be a stupid simple alternative to just double up the hash functions and have Hash32/Hash64 of everything so the choice is explicit.  That actually may well be the best option, especially if 32-bit hashes are necessary/desirable in places for reasons of size.  Still not gonna do it without the pressing need.
You need to log in before you can comment on or make changes to this bug.