Attachment 349546 Details for Bug 189507 – Patch

[patch] Patch

bug-189507-20180913000514.patch (text/plain), 11.21 KB, created by Yusuke Suzuki on 2018-09-12 08:05:15 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2018-09-12 08:05:15 PDT

Size: 11.21 KB

patch

obsolete

>Subversion Revision: 235934
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index e60abee4189f1442afc52c598099305a20175ed1..a0c0f3dc5a6d604206a8df7d206853db4253f5f9 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1 +1,24 @@
>+2018-09-12  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
>+
>+        [JSC] Optimize Array#indexOf in C++ runtime
>+        https://bugs.webkit.org/show_bug.cgi?id=189507
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * runtime/ArrayPrototype.cpp:
>+        (JSC::arrayProtoFuncIndexOf):
>+        * runtime/JSArray.h:
>+        * runtime/JSArrayInlines.h:
>+        (JSC::JSArray::canFastIndexedAccess):
>+        (JSC::toLength):
>+        * runtime/JSCJSValueInlines.h:
>+        (JSC::JSValue::JSValue):
>+        * runtime/JSGlobalObject.h:
>+        * runtime/JSGlobalObjectInlines.h:
>+        (JSC::JSGlobalObject::isArrayPrototypeIndexedAccessFastAndNonObservable):
>+        (JSC::JSGlobalObject::isArrayPrototypeIteratorProtocolFastAndNonObservable):
>+        * runtime/MathCommon.h:
>+        (JSC::canBeStrictInt32):
>+        (JSC::canBeInt32):
>+
> == Rolled over to ChangeLog-2018-09-11 ==
>diff --git a/Source/JavaScriptCore/runtime/ArrayPrototype.cpp b/Source/JavaScriptCore/runtime/ArrayPrototype.cpp
>index c4fa8b6b6bbbc0b8578020425c4f63a5a42b6d06..138cae55eb9d21fc8f93e430eb6d5adaa7d726b4 100644
>--- a/Source/JavaScriptCore/runtime/ArrayPrototype.cpp
>+++ b/Source/JavaScriptCore/runtime/ArrayPrototype.cpp
>@@ -1156,18 +1156,76 @@ EncodedJSValue JSC_HOST_CALL arrayProtoFuncIndexOf(ExecState* exec)
>     auto scope = DECLARE_THROW_SCOPE(vm);
> 
>     // 15.4.4.14
>-    JSObject* thisObj = exec->thisValue().toThis(exec, StrictMode).toObject(exec);
>-    EXCEPTION_ASSERT(!!scope.exception() == !thisObj);
>-    if (UNLIKELY(!thisObj))
>+    JSObject* thisObject = exec->thisValue().toThis(exec, StrictMode).toObject(exec);
>+    EXCEPTION_ASSERT(!!scope.exception() == !thisObject);
>+    if (UNLIKELY(!thisObject))
>         return encodedJSValue();
>-    unsigned length = toLength(exec, thisObj);
>+    unsigned length = toLength(exec, thisObject);
>     RETURN_IF_EXCEPTION(scope, encodedJSValue());
> 
>     unsigned index = argumentClampedIndexFromStartOrEnd(exec, 1, length);
>     RETURN_IF_EXCEPTION(scope, encodedJSValue());
>     JSValue searchElement = exec->argument(0);
>+
>+    if (isJSArray(thisObject)) {
>+        JSArray* array = asArray(thisObject);
>+        if (array->canFastIndexedAccess(vm)) {
>+            switch (array->indexingType()) {
>+            case ALL_INT32_INDEXING_TYPES: {
>+                if (!searchElement.isNumber())
>+                    return JSValue::encode(jsNumber(-1));
>+                JSValue searchInt32;
>+                if (searchElement.isInt32())
>+                    searchInt32 = searchElement;
>+                else {
>+                    double searchNumber = searchElement.asNumber();
>+                    if (!canBeInt32(searchNumber))
>+                        return JSValue::encode(jsNumber(-1));
>+                    searchInt32 = jsNumber(static_cast<int32_t>(searchNumber));
>+                }
>+                auto& butterfly = *array->butterfly();
>+                auto data = butterfly.contiguous().data();
>+                for (; index < length; ++index) {
>+                    if (searchInt32 == data[index].get())
>+                        return JSValue::encode(jsNumber(index));
>+                }
>+                return JSValue::encode(jsNumber(-1));
>+            }
>+            case ALL_CONTIGUOUS_INDEXING_TYPES: {
>+                auto& butterfly = *array->butterfly();
>+                auto data = butterfly.contiguous().data();
>+                for (; index < length; ++index) {
>+                    JSValue value = data[index].get();
>+                    if (!value)
>+                        continue;
>+                    bool isEqual = JSValue::strictEqual(exec, searchElement, value);
>+                    RETURN_IF_EXCEPTION(scope, encodedJSValue());
>+                    if (isEqual)
>+                        return JSValue::encode(jsNumber(index));
>+                }
>+                return JSValue::encode(jsNumber(-1));
>+            }
>+            case ALL_DOUBLE_INDEXING_TYPES: {
>+                if (!searchElement.isNumber())
>+                    return JSValue::encode(jsNumber(-1));
>+                double searchNumber = searchElement.asNumber();
>+                auto& butterfly = *array->butterfly();
>+                auto data = butterfly.contiguousDouble().data();
>+                for (; index < length; ++index) {
>+                    double value = data[index];
>+                    if (value == searchNumber)
>+                        return JSValue::encode(jsNumber(index));
>+                }
>+                return JSValue::encode(jsNumber(-1));
>+            }
>+            default:
>+                break;
>+            }
>+        }
>+    }
>+
>     for (; index < length; ++index) {
>-        JSValue e = getProperty(exec, thisObj, index);
>+        JSValue e = getProperty(exec, thisObject, index);
>         RETURN_IF_EXCEPTION(scope, encodedJSValue());
>         if (!e)
>             continue;
>diff --git a/Source/JavaScriptCore/runtime/JSArray.h b/Source/JavaScriptCore/runtime/JSArray.h
>index a6ee5d3d1c2743d318ec143466ca6985cb4f726e..948fef836fba1924ca0f318d958add948bdf535b 100644
>--- a/Source/JavaScriptCore/runtime/JSArray.h
>+++ b/Source/JavaScriptCore/runtime/JSArray.h
>@@ -101,6 +101,7 @@ class JSArray : public JSNonFinalObject {
>     JSArray* fastSlice(ExecState&, unsigned startIndex, unsigned count);
> 
>     bool canFastCopy(VM&, JSArray* otherArray);
>+    bool canFastIndexedAccess(VM&);
>     // This function returns NonArray if the indexing types are not compatable for copying.
>     IndexingType mergeIndexingTypeForCopying(IndexingType other);
>     bool appendMemcpy(ExecState*, VM&, unsigned startIndex, JSArray* otherArray);
>diff --git a/Source/JavaScriptCore/runtime/JSArrayInlines.h b/Source/JavaScriptCore/runtime/JSArrayInlines.h
>index 368dbd6c43f587c02622f44edd0578a9d57f7c39..9f457cf09a199eae2905aeb3eef649bfbc5b8f0a 100644
>--- a/Source/JavaScriptCore/runtime/JSArrayInlines.h
>+++ b/Source/JavaScriptCore/runtime/JSArrayInlines.h
>@@ -70,11 +70,31 @@ inline bool JSArray::canFastCopy(VM& vm, JSArray* otherArray)
>     return true;
> }
> 
>+inline bool JSArray::canFastIndexedAccess(VM& vm)
>+{
>+    JSGlobalObject* globalObject = this->globalObject();
>+    if (!globalObject->isArrayPrototypeIndexedAccessFastAndNonObservable())
>+        return false;
>+
>+    Structure* structure = this->structure(vm);
>+    // This is the fast case. Many arrays will be an original array.
>+    if (globalObject->isOriginalArrayStructure(structure))
>+        return true;
>+
>+    if (structure->mayInterceptIndexedAccesses())
>+        return false;
>+
>+    if (getPrototypeDirect(vm) != globalObject->arrayPrototype())
>+        return false;
>+
>+    return true;
>+}
>+
> ALWAYS_INLINE double toLength(ExecState* exec, JSObject* obj)
> {
>     VM& vm = exec->vm();
>     auto scope = DECLARE_THROW_SCOPE(vm);
>-    if (isJSArray(obj))
>+    if (LIKELY(isJSArray(obj)))
>         return jsCast<JSArray*>(obj)->length();
> 
>     JSValue lengthValue = obj->get(exec, vm.propertyNames->length);
>diff --git a/Source/JavaScriptCore/runtime/JSCJSValueInlines.h b/Source/JavaScriptCore/runtime/JSCJSValueInlines.h
>index 5aa41e4374adde520b2f6e40671c9a21e4886f29..a948844cd12741393b9eef99e1c6190ed7020f46 100644
>--- a/Source/JavaScriptCore/runtime/JSCJSValueInlines.h
>+++ b/Source/JavaScriptCore/runtime/JSCJSValueInlines.h
>@@ -167,13 +167,11 @@ inline JSValue::JSValue(unsigned long long i)
> 
> inline JSValue::JSValue(double d)
> {
>-    // Note: while this behavior is undefined for NaN and inf, the subsequent statement will catch these cases.
>-    const int32_t asInt32 = static_cast<int32_t>(d);
>-    if (asInt32 != d || (!asInt32 && std::signbit(d))) { // true for -0.0
>-        *this = JSValue(EncodeAsDouble, d);
>+    if (canBeStrictInt32(d)) {
>+        *this = JSValue(static_cast<int32_t>(d));
>         return;
>     }
>-    *this = JSValue(static_cast<int32_t>(d));
>+    *this = JSValue(EncodeAsDouble, d);
> }
> 
> inline EncodedJSValue JSValue::encode(JSValue value)
>diff --git a/Source/JavaScriptCore/runtime/JSGlobalObject.h b/Source/JavaScriptCore/runtime/JSGlobalObject.h
>index b92606d79404823fcee8cf89e65371baee1a00aa..56887afd836e94d958897e5fbd1d9714b9238c31 100644
>--- a/Source/JavaScriptCore/runtime/JSGlobalObject.h
>+++ b/Source/JavaScriptCore/runtime/JSGlobalObject.h
>@@ -476,6 +476,7 @@ class JSGlobalObject : public JSSegmentedVariableObject {
>     PoisonedUniquePtr<ObjectPropertyChangeAdaptiveWatchpoint<InlineWatchpointSet>> m_setPrototypeAddWatchpoint;
>     PoisonedUniquePtr<ObjectPropertyChangeAdaptiveWatchpoint<InlineWatchpointSet>> m_numberPrototypeToStringWatchpoint;
> 
>+    bool isArrayPrototypeIndexedAccessFastAndNonObservable();
>     bool isArrayPrototypeIteratorProtocolFastAndNonObservable();
>     bool isMapPrototypeIteratorProtocolFastAndNonObservable();
>     bool isSetPrototypeIteratorProtocolFastAndNonObservable();
>diff --git a/Source/JavaScriptCore/runtime/JSGlobalObjectInlines.h b/Source/JavaScriptCore/runtime/JSGlobalObjectInlines.h
>index 6f4ebc077e32e28436177af8ec9fc8143f2190be..5a0b4f5b704d87a0b5910f96cb0faadba748f655 100644
>--- a/Source/JavaScriptCore/runtime/JSGlobalObjectInlines.h
>+++ b/Source/JavaScriptCore/runtime/JSGlobalObjectInlines.h
>@@ -52,6 +52,12 @@ ALWAYS_INLINE bool JSGlobalObject::stringPrototypeChainIsSane()
>         && objectPrototypeIsSane();
> }
> 
>+ALWAYS_INLINE bool JSGlobalObject::isArrayPrototypeIndexedAccessFastAndNonObservable()
>+{
>+    // We're fast if we don't have any indexed properties on the prototype.
>+
>+    return !isHavingABadTime() && arrayPrototypeChainIsSane();
>+}
> 
> ALWAYS_INLINE bool JSGlobalObject::isArrayPrototypeIteratorProtocolFastAndNonObservable()
> {
>@@ -63,7 +69,7 @@ ALWAYS_INLINE bool JSGlobalObject::isArrayPrototypeIteratorProtocolFastAndNonObs
>     // carefully set up watchpoints to have correct ordering while JS code is
>     // executing concurrently.
> 
>-    return arrayIteratorProtocolWatchpoint().isStillValid() && !isHavingABadTime() && arrayPrototypeChainIsSane();
>+    return arrayIteratorProtocolWatchpoint().isStillValid() && isArrayPrototypeIndexedAccessFastAndNonObservable();
> }
> 
> // We're non-observable if the iteration protocol hasn't changed.
>diff --git a/Source/JavaScriptCore/runtime/MathCommon.h b/Source/JavaScriptCore/runtime/MathCommon.h
>index e9194612142a537446e66d986c64e806439f2059..9ffa55c53bc3ecd38c66dd9d97e3f3de2bc98321 100644
>--- a/Source/JavaScriptCore/runtime/MathCommon.h
>+++ b/Source/JavaScriptCore/runtime/MathCommon.h
>@@ -178,6 +178,19 @@ inline std::optional<double> safeReciprocalForDivByConst(double constant)
>     return reciprocal;
> }
> 
>+ALWAYS_INLINE bool canBeStrictInt32(double value)
>+{
>+    // Note: while this behavior is undefined for NaN and inf, the subsequent statement will catch these cases.
>+    const int32_t asInt32 = static_cast<int32_t>(value);
>+    return !(asInt32 != value || (!asInt32 && std::signbit(value))); // true for -0.0
>+}
>+
>+ALWAYS_INLINE bool canBeInt32(double value)
>+{
>+    // Note: while this behavior is undefined for NaN and inf, the subsequent statement will catch these cases.
>+    return static_cast<int32_t>(value) == value;
>+}
>+
> extern "C" {
> double JIT_OPERATION jsRound(double value) REFERENCED_FROM_ASM WTF_INTERNAL;
>

Actions: View | Formatted Diff | Diff

Attachments on bug 189507: 349410 | 349421 | 349423 | 349425 | 349437 | 349465 | 349546 | 349550 | 349553 | 349581 | 350087 | 350165