[JSC] Improve the call site of string comparison in some hot path
authorbenjamin@webkit.org <benjamin@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 14 Apr 2014 08:46:27 +0000 (08:46 +0000)
committerbenjamin@webkit.org <benjamin@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 14 Apr 2014 08:46:27 +0000 (08:46 +0000)
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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@167220 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/jit/JITOperations.cpp
Source/JavaScriptCore/runtime/JSCJSValueInlines.h
Source/WTF/ChangeLog
Source/WTF/wtf/text/StringImpl.cpp
Source/WebCore/ChangeLog
Source/WebCore/dom/NodeRareData.h

index a39369c..7f0b52e 100644 (file)
@@ -1,3 +1,22 @@
+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>
 
         Rewrite Function.bind as a builtin
index 52baa8b..8b205f6 100644 (file)
@@ -912,7 +912,7 @@ size_t JIT_OPERATION operationCompareStringEq(ExecState* exec, JSCell* left, JSC
     VM* vm = &exec->vm();
     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));
 #else
index 26b4e15..6a137f8 100644 (file)
@@ -30,6 +30,7 @@
 #include "JSCJSValue.h"
 #include "JSCellInlines.h"
 #include "JSFunction.h"
+#include <wtf/text/StringImpl.h>
 
 namespace JSC {
 
@@ -738,7 +739,7 @@ ALWAYS_INLINE bool JSValue::equalSlowCaseInline(ExecState* exec, JSValue v1, JSV
         bool s1 = v1.isString();
         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()) {
             if (v2.isUndefinedOrNull())
@@ -800,7 +801,7 @@ ALWAYS_INLINE bool JSValue::strictEqualSlowCaseInline(ExecState* exec, JSValue v
     ASSERT(v1.isCell() && v2.isCell());
 
     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;
 }
@@ -835,7 +836,7 @@ inline TriState JSValue::pureStrictEqual(JSValue v1, JSValue v2)
         const StringImpl* v2String = asString(v2)->tryGetValueImpl();
         if (!v1String || !v2String)
             return MixedTriState;
-        return triState(WTF::equal(v1String, v2String));
+        return triState(WTF::equal(*v1String, *v2String));
     }
     
     return triState(v1 == v2);
index 8a4a35f..89a860e 100644 (file)
@@ -1,3 +1,15 @@
+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>
 
         Relax adoption requirements of RefCounted objects that are NeverDestroyed
index 848ccf6..c684710 100644 (file)
@@ -1764,7 +1764,7 @@ PassRef<StringImpl> StringImpl::replace(StringImpl* pattern, StringImpl* replace
     return newImpl;
 }
 
-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();
     unsigned bLength = b.length();
index 471394d..1b471fe 100644 (file)
@@ -1,3 +1,14 @@
+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>
 
         Merge MemoryPressureHandler{Mac,IOS}.mm
index e5b7062..99245d6 100644 (file)
@@ -108,7 +108,7 @@ public:
         {
             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;
     };