[webkit-dev] Using memcmp

[Simone Fiorentino]

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 at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>