WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
NEW
115008
Compute "better" hash for WTF::StringImpl::StringImpl(CreateEmptyUnique_T)
https://bugs.webkit.org/show_bug.cgi?id=115008
Summary
Compute "better" hash for WTF::StringImpl::StringImpl(CreateEmptyUnique_T)
David Kilzer (:ddkilzer)
Reported
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?
Attachments
Add attachment
proposed patch, testcase, etc.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug