Bug 115008 - Compute "better" hash for WTF::StringImpl::StringImpl(CreateEmptyUnique_T)
Summary: Compute "better" hash for WTF::StringImpl::StringImpl(CreateEmptyUnique_T)
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-22 19:41 PDT by David Kilzer (:ddkilzer)
Modified: 2013-04-22 19:41 PDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2013-04-22 19:41:28 PDT
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?