115008 2013-04-22 19:41:28 -0700 Compute "better" hash for WTF::StringImpl::StringImpl(CreateEmptyUnique_T) 2013-04-22 19:41:28 -0700 1 1 1 Unclassified WebKit Web Template Framework 528+ (Nightly build) Unspecified Unspecified NEW P2 Normal --- 1 ddkilzer webkit-unassigned benjamin darin oldest_to_newest 879564 0 ddkilzer 2013-04-22 19:41:28 -0700 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?