As part of the changes for https://bugs.webkit.org/show_bug.cgi?id=66161 with the goal of eliminating the use of ::characters() calls, we can coalesce the various equals() methods that exist in the code and have those call sites use the UString implementations.
Created attachment 106003 [details] Proposed patch
Created attachment 106007 [details] Real proposed patch
Comment on attachment 106007 [details] Real proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=106007&action=review > Source/JavaScriptCore/runtime/Identifier.cpp:72 > + return WTF::equal(r, s); It looks like there is an additional level of function call here. What’s the performance impact? > Source/JavaScriptCore/runtime/Identifier.h:140 > + return WTF::equal(r, s, length); It looks like there is an additional level of function call here as well as additional null checks. What’s the performance impact? > Source/JavaScriptCore/wtf/text/AtomicString.cpp:93 > + static inline bool equal(StringImpl* r, const char* s) The inline keyword is redundant inside a class definition. There is no value to adding it. > Source/JavaScriptCore/wtf/text/AtomicString.cpp:95 > + return WTF::equal(r, s); It looks like there is an additional level of function call here as well as additional null checks. What’s the performance impact? > Source/JavaScriptCore/wtf/text/AtomicString.cpp:108 > + return WTF::equal(a.impl(), b); It looks like there is an additional level of function call here. What’s the performance impact? > Source/JavaScriptCore/wtf/text/AtomicString.cpp:-138 > -static inline bool equal(StringImpl* string, const UChar* characters, unsigned length) Looks like the new version of the function is no longer inlined for callers within this source file. What is the performance impact of that? > Source/JavaScriptCore/wtf/text/StringImpl.cpp:1003 > + if (!b) > + return !a; This can be return false.
Comment on attachment 106007 [details] Real proposed patch Per our discussion, since the call sites to equal are all trivial methods we should be able to inline these instead, removing the additional layer of function calling & likely overall reducing the size of the binary. r+ with Darin's comments + perf testing (particularly for JSC, since you're changing the implementation of equal for Identifiers).
Made changes suggested by Darin and Gavin. Measured performance with updated changes: Baseline w/updated changes SunSpider command line 230.0 +/- 0.2% 230.0 +/- 0.2% V8 command line 1639.4 +/- 0.3% 1638.2 +/- 0.2% Dromaeo DOM Core 1494.69 1499.86
Committed r94475: <http://trac.webkit.org/changeset/94475>