[webkit-reviews] review denied: [Bug 43342] AtomicStringHash does not work with null atomic string : [Attachment 63208] Use the pointer to the implementation as the hash value of AtomicString.

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Benjamin Poulain
<benjamin.poulain at nokia.com>'s request for review:
Bug 43342: AtomicStringHash does not work with null atomic string
https://bugs.webkit.org/show_bug.cgi?id=43342

Attachment 63208: Use the pointer to the implementation as the hash value of
AtomicString.
https://bugs.webkit.org/attachment.cgi?id=63208&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
Making the hash work will null isn’t sufficient. Hash tables still need both an
empty value and a deleted value. A null pointer is the empty value and so won't
work in a hash table even with this hash function.

We used to hash the pointers and switched to using the hash of the string
instead as a measurable speedup. The hash of the string is higher quality than
a hash of the pointer bits.

What you have here is not even a hash of pointer bits, but rather just taking
the low bits. This will be a far-inferior hash function and will almost
certainly result in a measurable slowdown.

The best approach here is to check explicitly for null and not use it with hash
tables. Or to do a more-major redesign.