[Webkit-unassigned] [Bug 150137] New: IdentifierRepHash has a mismatch between hash() and equal()

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=150137

            Bug ID: 150137
           Summary: IdentifierRepHash has a mismatch between hash() and
                    equal()
    Classification: Unclassified
           Product: WebKit
           Version: WebKit Nightly Build
          Hardware: All
                OS: All
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: JavaScriptCore
          Assignee: webkit-unassigned at lists.webkit.org
          Reporter: fpizlo at apple.com

IdentifierRepHash inherits equal() from PtrHash, so it defines equality as pointer equality.  But it implements hash() by calling the StringImpl's hash method.

This seems either incorrect or inefficient.

The hash() method will return a hash value based on the string's value, not the string's identity.  So, non-identical string might end up having the same value and so the same hash.  Also, that hash takes more effort to compute than the normal PtrHash.

Quite likely, the PtrHash::hash method would be most efficient, since it doesn't require loading from the pointer.  Loads are usually the more expensive things.

Also, this means that we cannot have a HashMap with IdnetifierRepHash that doesn't ref its strings and outlives some of the strings that are in it.  This seems dirty, but it might actually be profitable in some cases, like InferredTypeTable.  It's OK if that table has an entry for a dangling StringImpl*, because all this means is that if some StringImpl gets allocated in the same memory address, we'll already have a hypothesis about the type.  It might be a wrong hypothesis, but that's still sound - at worst we'll come up with a looser type than what we could have done.  This might be a profitable optimization if the reduction in ref/deref traffic produced a big enough speed-up.

Anyway, it seems like IdentifierRepHash is doing either too much work in hash() or too little work in equal(), and we should fix it to be consistent, probably by just replacing it with PtrHash.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20151014/a8ad5ce5/attachment.html>