> I think WebCore::equal already has a patch for SH4 and ARM. I was only > going to replicate that same code in StringHash::equal.
I agree. Regards, S. > Are you > talking about testing for alignment and then doing the 32-bit loop > over the aligned segment? > > On Jul 1, 2009, at 10:31 AM, Simone Fiorentino wrote: > > > Hi Patrick, > > I think WebKit code can't rely on system memcmp() alignment > > functionality > > since its implementation may change. > > I think a better solution should be to write code (inside WebKit) > > that use > > aligned access in memory (as done in WebCore::equal)...for platforms > > that > > needs aligned accesses. > > > > StringHash::equal() needs a patch (as done for WebCore::equal)...at > > least for > > SH4 platform. > > If you prefer I can submit such a patch for SH4 platform. > > > > Tnx. > > > > Best regards, > > S. > > > > > > On Wed July 1 2009, Patrick Hanna wrote: > >> On Jun 30, 2009, at 2:53 PM, Maciej Stachowiak wrote: > >> > >>> > >>> On Jun 30, 2009, at 9:57 AM, Patrick Hanna wrote: > >>> > >>>> I have noticed that rather than using memcmp for comparing strings, > >>>> webkit likes to cast to uint32_t* and compare in a for loop. For > >>>> example, WebCore::equal in AtomicString.cpp and StringHash::equal > >>>> in StringHash.h. Is there any reason not to memcmp? I am assuming > >>>> that most implementations of memcmp will do the word alignment and > >>>> basically do the same thing. > >>> > >>> We found the copy loop to be a little bit faster than memcmp on the > >>> platforms where we tested. memcmp tends not to do as well for short > >>> strings because it doesn't know that the start and end have some > >>> alignment guarantees. Also there is the function call overhead. > >>> > >>>> The reason I ask is because StringHash::equal does not check for > >>>> PLATFORM(ARM) or PLATFORM(SH4) so I was going to submit a patch to > >>>> use memcmp for ARM/SH4 but didn't know if that was frowned upon > >>>> (and WebCore::equal does a loop for ARM/SH4 as well). > >>> > >>> Is there a reason you'd want to make the code use a different > >>> implementation depending on the CPU? (It's not obvious to me from > >>> your message). > >> > >> Sorry, I did not mention the problem. On some arm platforms, > >> unaligned > >> access causes a bus error so we cannot reinterpret_cast from UChar* > >> to > >> uint32_t*. Our memcmp does the alignment check for us and is > >> inlined. > >> When I do the patch, I will do the loop to be consistent with the > >> AtomicString implementation. Thanks for the info. > >> > >>> > >>> Regards, > >>> Maciej > >>> > >> > >> _______________________________________________ > >> webkit-dev mailing list > >> webkit-dev@lists.webkit.org > >> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev > >> > > > > > > _______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
