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. 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). Thanks, Patrick
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). Regards, Maciej
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
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
I think WebCore::equal already has a patch for SH4 and ARM. I was only going to replicate that same code in StringHash::equal. 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
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
In order to get focus memory for tabs working properly, we need a notion of whether or not the underlying Page is focused. We need this since Web pages should be able to shift around programmatic focus in background tabs and have the new focused frame and focused node be remembered if you switch back to that tab. Our current code was relying only on the window being active plus the frame being focused to decide that it should draw focus rings and blink carets. However this isn't good enough in the case described above. I have just added a new boolean to FocusController that indicates whether or not the Page is focused. The Mac and Windows ports have been patched to update the Page focused state properly (I hope). Other ports will need to call page->focusController()-
setFocused(true) or setFocused(false) now in order for everything to work properly.
dave (hyatt@apple.com)
participants (4)
-
David Hyatt
-
Maciej Stachowiak
-
Patrick Hanna
-
Simone Fiorentino