[Webkit-unassigned] [Bug 115008] New: Compute "better" hash for WTF::StringImpl::StringImpl(CreateEmptyUnique_T)

[bugzilla-daemon at webkit.org]

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

           Summary: Compute "better" hash for
                    WTF::StringImpl::StringImpl(CreateEmptyUnique_T)
           Product: WebKit
           Version: 528+ (Nightly build)
          Platform: Unspecified
        OS/Version: Unspecified
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: Web Template Framework
        AssignedTo: webkit-unassigned at lists.webkit.org
        ReportedBy: ddkilzer at webkit.org
                CC: darin at apple.com, benjamin at webkit.org


Per comments in Bug 114970, we should compute a "better" hash in WTF::StringImpl::StringImpl(CreateEmptyUnique_T):

(In reply to Bug 114970 comment #2, Darin Adler wrote)
> (From update of attachment 199053 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=199053&action=review
> 
> > Source/WTF/wtf/text/StringImpl.h:316
> > -        unsigned hash = reinterpret_cast<uintptr_t>(this);
> > +        unsigned hash = static_cast<uint32_t>(reinterpret_cast<uintptr_t>(this));
> 
> This is a silly approach to creating the hash. For example, the low bits will always be due to alignment zero. I suggest we use a random number instead.
> 
> Code change is OK, but code is dumb.

(In reply to Bug 114970 comment #6, David Kilzer wrote)
> (In reply to Bug 114970 comment #2, Darin Adler wrote)
> > Code change is OK, but code is dumb.
> 
> Should we use WTF::intHash(uint32_t) on 32-bit platforms and WTF::intHas(uint64_t) on 64-bit platforms instead?

(In reply to Bug 114970 comment #7, Benjamin Poulain wrote)
> (In reply to Bug 114970 comment #6, David Kilzer wrote)
> > Should we use WTF::intHash(uint32_t) on 32-bit platforms and WTF::intHas(uint64_t) on 64-bit platforms instead?
> 
> The only property needed for that hash is it should never equals the hash of emptyString().
> 
> We could basically use a constant here. Any number !m hash(empty()) has the same theoretical chances of conflicts.
> 
> I have been trying to kill this constructor every now and then :)

(In reply to Bug 114970 comment #8, Darin Adler wrote)
> (In reply to Bug 114970 comment #7, Benjamin Poulain wrote)
> > The only property needed for that hash is it should never equals the hash of emptyString().
> 
> Maybe (hash(emptyString()) ^ 0x100) would work?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.