Changeset 167220 in webkit

Timestamp:

Apr 14, 2014 1:46:27 AM (10 years ago)

Author:

benjamin@webkit.org

Message:

[JSC] Improve the call site of string comparison in some hot path
​https://bugs.webkit.org/show_bug.cgi?id=131605

Reviewed by Darin Adler.

Source/JavaScriptCore:

When resolved, the String of a JSString is never null. It can be empty but not null.
The null value is reserved for ropes but those would be resolved when getting the value.

Consequently, we should use the equal() operation that do not handle null values.
Using the StringImpl directly is already common in StringPrototype but it was not used here for some reason.

• jit/JITOperations.cpp:
• runtime/JSCJSValueInlines.h:

(JSC::JSValue::equalSlowCaseInline):
(JSC::JSValue::strictEqualSlowCaseInline):
(JSC::JSValue::pureStrictEqual):

Source/WebCore:

• dom/NodeRareData.h:

(WebCore::NodeListsNodeData::NodeListCacheMapEntryHash::equal):
We should use the right comparison operation depending on the Hash Traits.

Source/WTF:

• wtf/text/StringImpl.cpp:

(WTF::stringImplContentEqual):
Inline that function to reduce the call overhead for JSC.
This is only inlined twice, it is not catastrophic for our binary.

Location:

trunk/Source

Files:

• JavaScriptCore/ChangeLog (modified) (1 diff)
• JavaScriptCore/jit/JITOperations.cpp (modified) (1 diff)
• JavaScriptCore/runtime/JSCJSValueInlines.h (modified) (4 diffs)
• WTF/ChangeLog (modified) (1 diff)
• WTF/wtf/text/StringImpl.cpp (modified) (1 diff)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/dom/NodeRareData.h (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2014-04-14  Benjamin Poulain  <benjamin@webkit.org>
+
+        [JSC] Improve the call site of string comparison in some hot path
+        https://bugs.webkit.org/show_bug.cgi?id=131605
+
+        Reviewed by Darin Adler.
+
+        When resolved, the String of a JSString is never null. It can be empty but not null.
+        The null value is reserved for ropes but those would be resolved when getting the value.
+
+        Consequently, we should use the equal() operation that do not handle null values.
+        Using the StringImpl directly is already common in StringPrototype but it was not used here for some reason.
+
+        * jit/JITOperations.cpp:
+        * runtime/JSCJSValueInlines.h:
+        (JSC::JSValue::equalSlowCaseInline):
+        (JSC::JSValue::strictEqualSlowCaseInline):
+        (JSC::JSValue::pureStrictEqual):
+
 2014-04-08  Oliver Hunt  <oliver@apple.com>
 

trunk/Source/JavaScriptCore/jit/JITOperations.cpp

@@ -913 +913 @@
     NativeCallFrameTracer tracer(vm, exec);
 
-    bool result = asString(left)->value(exec) == asString(right)->value(exec);
+    bool result = WTF::equal(*asString(left)->value(exec).impl(), *asString(right)->value(exec).impl());
 #if USE(JSVALUE64)
     return JSValue::encode(jsBoolean(result));

trunk/Source/JavaScriptCore/runtime/JSCJSValueInlines.h

@@ -31 +31 @@
 #include "JSCellInlines.h"
 #include "JSFunction.h"
+#include <wtf/text/StringImpl.h>
 
 namespace JSC {
@@ -739 +740 @@
         bool s2 = v2.isString();
         if (s1 && s2)
-            return asString(v1)->value(exec) == asString(v2)->value(exec);
+            return WTF::equal(*asString(v1)->value(exec).impl(), *asString(v2)->value(exec).impl());
 
         if (v1.isUndefinedOrNull()) {
@@ -801 +802 @@
 
     if (v1.asCell()->isString() && v2.asCell()->isString())
-        return asString(v1)->value(exec) == asString(v2)->value(exec);
+        return WTF::equal(*asString(v1)->value(exec).impl(), *asString(v2)->value(exec).impl());
 
     return v1 == v2;
@@ -836 +837 @@
         if (!v1String || !v2String)
             return MixedTriState;
-        return triState(WTF::equal(v1String, v2String));
+        return triState(WTF::equal(*v1String, *v2String));
     }
     

trunk/Source/WTF/ChangeLog

@@ +1 @@
+2014-04-14  Benjamin Poulain  <benjamin@webkit.org>
+
+        [JSC] Improve the call site of string comparison in some hot path
+        https://bugs.webkit.org/show_bug.cgi?id=131605
+
+        Reviewed by Darin Adler.
+
+        * wtf/text/StringImpl.cpp:
+        (WTF::stringImplContentEqual):
+        Inline that function to reduce the call overhead for JSC.
+        This is only inlined twice, it is not catastrophic for our binary.
+
 2014-04-13  Andy Estes  <aestes@apple.com>
 

trunk/Source/WTF/wtf/text/StringImpl.cpp

@@ -1765 +1765 @@
 }
 
-static inline bool stringImplContentEqual(const StringImpl& a, const StringImpl& b)
+static ALWAYS_INLINE bool stringImplContentEqual(const StringImpl& a, const StringImpl& b)
 {
     unsigned aLength = a.length();

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2014-04-14  Benjamin Poulain  <benjamin@webkit.org>
+
+        [JSC] Improve the call site of string comparison in some hot path
+        https://bugs.webkit.org/show_bug.cgi?id=131605
+
+        Reviewed by Darin Adler.
+
+        * dom/NodeRareData.h:
+        (WebCore::NodeListsNodeData::NodeListCacheMapEntryHash::equal):
+        We should use the right comparison operation depending on the Hash Traits.
+
 2014-04-14  Andreas Kling  <akling@apple.com>
 

trunk/Source/WebCore/dom/NodeRareData.h

@@ -109 +109 @@
             return DefaultHash<StringType>::Hash::hash(entry.second) + entry.first;
         }
-        static bool equal(const std::pair<unsigned char, StringType>& a, const std::pair<unsigned char, StringType>& b) { return a == b; }
+        static bool equal(const std::pair<unsigned char, StringType>& a, const std::pair<unsigned char, StringType>& b) { return a.first == b.first && DefaultHash<StringType>::Hash::equal(a.second, b.second); }
         static const bool safeToCompareToEmptyOrDeleted = DefaultHash<StringType>::Hash::safeToCompareToEmptyOrDeleted;
     };