Attachment 350179 Details for Bug 189780 – Patch

[patch] Patch

bug-189780-20180920172626.patch (text/plain), 15.46 KB, created by Yusuke Suzuki on 2018-09-20 01:26:27 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2018-09-20 01:26:27 PDT

Size: 15.46 KB

patch

obsolete

>Subversion Revision: 236240
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 163db4b44a49eabb4698f887a1a5d803c792d398..8ef62d6e0df5176080ae944f84faa5389fa812cb 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,16 @@
>+2018-09-20  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
>+
>+        [JSC] Optimize Array#lastIndexOf
>+        https://bugs.webkit.org/show_bug.cgi?id=189780
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Optimize Array#lastIndexOf as the same to Array#indexOf. We add a fast path
>+        for JSArray with contiguous storage.
>+
>+        * runtime/ArrayPrototype.cpp:
>+        (JSC::arrayProtoFuncLastIndexOf):
>+
> 2018-09-19  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
> 
>         [JSC] Optimize Array#indexOf in C++ runtime
>diff --git a/Source/JavaScriptCore/runtime/ArrayPrototype.cpp b/Source/JavaScriptCore/runtime/ArrayPrototype.cpp
>index f714c1f5e9d0fcffe3d215e7d75abfbef9945495..0f0abf63d6d74cce821eb188e2d9bc990bbf25d5 100644
>--- a/Source/JavaScriptCore/runtime/ArrayPrototype.cpp
>+++ b/Source/JavaScriptCore/runtime/ArrayPrototype.cpp
>@@ -1247,11 +1247,11 @@ EncodedJSValue JSC_HOST_CALL arrayProtoFuncLastIndexOf(ExecState* exec)
>     auto scope = DECLARE_THROW_SCOPE(vm);
> 
>     // 15.4.4.15
>-    JSObject* thisObj = exec->thisValue().toThis(exec, StrictMode).toObject(exec);
>-    EXCEPTION_ASSERT(!!scope.exception() == !thisObj);
>-    if (UNLIKELY(!thisObj))
>-        return encodedJSValue();
>-    unsigned length = toLength(exec, thisObj);
>+    JSObject* thisObject = exec->thisValue().toThis(exec, StrictMode).toObject(exec);
>+    EXCEPTION_ASSERT(!!scope.exception() == !thisObject);
>+    if (UNLIKELY(!thisObject))
>+        return { };
>+    unsigned length = toLength(exec, thisObject);
>     if (UNLIKELY(scope.exception()) || !length)
>         return JSValue::encode(jsNumber(-1));
> 
>@@ -1259,7 +1259,7 @@ EncodedJSValue JSC_HOST_CALL arrayProtoFuncLastIndexOf(ExecState* exec)
>     if (exec->argumentCount() >= 2) {
>         JSValue fromValue = exec->uncheckedArgument(1);
>         double fromDouble = fromValue.toInteger(exec);
>-        RETURN_IF_EXCEPTION(scope, encodedJSValue());
>+        RETURN_IF_EXCEPTION(scope, { });
>         if (fromDouble < 0) {
>             fromDouble += length;
>             if (fromDouble < 0)
>@@ -1270,14 +1270,78 @@ EncodedJSValue JSC_HOST_CALL arrayProtoFuncLastIndexOf(ExecState* exec)
>     }
> 
>     JSValue searchElement = exec->argument(0);
>+
>+    if (isJSArray(thisObject)) {
>+        JSArray* array = asArray(thisObject);
>+        if (array->canDoFastIndexedAccess(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();
>+                do {
>+                    ASSERT(index < length);
>+                    // Array#lastIndexOf uses `===` semantics (not HashMap isEqual semantics).
>+                    // And the hole never matches against Int32 value.
>+                    if (searchInt32 == data[index].get())
>+                        return JSValue::encode(jsNumber(index));
>+                } while (index--);
>+                return JSValue::encode(jsNumber(-1));
>+            }
>+            case ALL_CONTIGUOUS_INDEXING_TYPES: {
>+                auto& butterfly = *array->butterfly();
>+                auto data = butterfly.contiguous().data();
>+                do {
>+                    ASSERT(index < length);
>+                    JSValue value = data[index].get();
>+                    if (!value)
>+                        continue;
>+                    bool isEqual = JSValue::strictEqual(exec, searchElement, value);
>+                    RETURN_IF_EXCEPTION(scope, { });
>+                    if (isEqual)
>+                        return JSValue::encode(jsNumber(index));
>+                } while (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();
>+                do {
>+                    ASSERT(index < length);
>+                    // Array#lastIndexOf uses `===` semantics (not HashMap isEqual semantics).
>+                    // And the hole never matches since it is NaN.
>+                    if (data[index] == searchNumber)
>+                        return JSValue::encode(jsNumber(index));
>+                } while (index--);
>+                return JSValue::encode(jsNumber(-1));
>+            }
>+            default:
>+                break;
>+            }
>+        }
>+    }
>+
>     do {
>-        RELEASE_ASSERT(index < length);
>-        JSValue e = getProperty(exec, thisObj, index);
>-        RETURN_IF_EXCEPTION(scope, encodedJSValue());
>+        ASSERT(index < length);
>+        JSValue e = getProperty(exec, thisObject, index);
>+        RETURN_IF_EXCEPTION(scope, { });
>         if (!e)
>             continue;
>         bool isEqual = JSValue::strictEqual(exec, searchElement, e);
>-        RETURN_IF_EXCEPTION(scope, encodedJSValue());
>+        RETURN_IF_EXCEPTION(scope, { });
>         if (isEqual)
>             return JSValue::encode(jsNumber(index));
>     } while (index--);
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index 6d13f017d73e49b13158851c92a7e0c623ac8855..992f5258a48285f740c1a33b8d86ffcfe5f4ccf5 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,34 @@
>+2018-09-20  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
>+
>+        [JSC] Optimize Array#lastIndexOf
>+        https://bugs.webkit.org/show_bug.cgi?id=189780
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * stress/array-lastindexof-array-prototype-trap.js: Added.
>+        (shouldBe):
>+        (AncestorArray.prototype.get 2):
>+        (AncestorArray):
>+        * stress/array-lastindexof-have-a-bad-time-c-runtime.js: Added.
>+        (shouldBe):
>+        * stress/array-lastindexof-hole-nan.js: Added.
>+        (shouldBe):
>+        (throw.new.Error):
>+        * stress/array-lastindexof-infinity.js: Added.
>+        (shouldBe):
>+        (throw.new.Error):
>+        * stress/array-lastindexof-negative-zero.js: Added.
>+        (shouldBe):
>+        (throw.new.Error):
>+        * stress/array-lastindexof-own-getter.js: Added.
>+        (shouldBe):
>+        (throw.new.Error.get array):
>+        (get array):
>+        * stress/array-lastindexof-prototype-trap.js: Added.
>+        (shouldBe):
>+        (DerivedArray.prototype.get 2):
>+        (DerivedArray):
>+
> 2018-09-19  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
> 
>         [JSC] Optimize Array#indexOf in C++ runtime
>diff --git a/JSTests/stress/array-lastindexof-array-prototype-trap.js b/JSTests/stress/array-lastindexof-array-prototype-trap.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..ee7bc7be911103dbc5585bf4b91f735f2224b08e
>--- /dev/null
>+++ b/JSTests/stress/array-lastindexof-array-prototype-trap.js
>@@ -0,0 +1,45 @@
>+function shouldBe(actual, expected) {
>+    if (actual !== expected)
>+        throw new Error('bad value: ' + actual);
>+}
>+
>+class AncestorArray extends Object {
>+    get 2() {
>+        this.called = true;
>+        return 42;
>+    }
>+}
>+
>+Array.prototype.__proto__ = AncestorArray.prototype;
>+
>+{
>+    let array = [];
>+    array.length = 42;
>+    shouldBe(array.lastIndexOf(42), 2);
>+    shouldBe(array.called, true);
>+}
>+{
>+    let array = [20, 20];
>+    array.length = 42;
>+    shouldBe(array.lastIndexOf(42), 2);
>+    shouldBe(array.called, true);
>+}
>+{
>+    let array = ["Hello"];
>+    array.length = 42;
>+    shouldBe(array.lastIndexOf(42), 2);
>+    shouldBe(array.called, true);
>+}
>+{
>+    let array = [42.195];
>+    array.length = 42;
>+    shouldBe(array.lastIndexOf(42), 2);
>+    shouldBe(array.called, true);
>+}
>+{
>+    let array = ["Hello"];
>+    array.length = 42;
>+    ensureArrayStorage(array);
>+    shouldBe(array.lastIndexOf(42), 2);
>+    shouldBe(array.called, true);
>+}
>diff --git a/JSTests/stress/array-lastindexof-have-a-bad-time-c-runtime.js b/JSTests/stress/array-lastindexof-have-a-bad-time-c-runtime.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..81bab225a5e80acf6624b1a09d688c692a9f2550
>--- /dev/null
>+++ b/JSTests/stress/array-lastindexof-have-a-bad-time-c-runtime.js
>@@ -0,0 +1,43 @@
>+function shouldBe(actual, expected) {
>+    if (actual !== expected)
>+        throw new Error('bad value: ' + actual);
>+}
>+
>+Object.defineProperty(Array.prototype, 2, {
>+    get() {
>+        this.called = true;
>+        return 42;
>+    }
>+});
>+
>+{
>+    let array = [];
>+    array.length = 42;
>+    shouldBe(array.lastIndexOf(42), 2);
>+    shouldBe(array.called, true);
>+}
>+{
>+    let array = [20, 20];
>+    array.length = 42;
>+    shouldBe(array.lastIndexOf(42), 2);
>+    shouldBe(array.called, true);
>+}
>+{
>+    let array = ["Hello"];
>+    array.length = 42;
>+    shouldBe(array.lastIndexOf(42), 2);
>+    shouldBe(array.called, true);
>+}
>+{
>+    let array = [42.195];
>+    array.length = 42;
>+    shouldBe(array.lastIndexOf(42), 2);
>+    shouldBe(array.called, true);
>+}
>+{
>+    let array = ["Hello"];
>+    array.length = 42;
>+    ensureArrayStorage(array);
>+    shouldBe(array.lastIndexOf(42), 2);
>+    shouldBe(array.called, true);
>+}
>diff --git a/JSTests/stress/array-lastindexof-hole-nan.js b/JSTests/stress/array-lastindexof-hole-nan.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..862ed837559c3ad7fbb907f110e66ca6e029e970
>--- /dev/null
>+++ b/JSTests/stress/array-lastindexof-hole-nan.js
>@@ -0,0 +1,19 @@
>+function shouldBe(actual, expected) {
>+    if (actual !== expected)
>+        throw new Error('bad value: ' + actual);
>+}
>+
>+
>+{
>+    let array = [42, , , 0];
>+    shouldBe(array.lastIndexOf(Number.NaN), -1);
>+    shouldBe(array.lastIndexOf(0), 3);
>+}
>+{
>+    let array = [42.195, , , 0];
>+    shouldBe(array.lastIndexOf(Number.NaN), -1);
>+}
>+{
>+    let array = [42.195, Number.NaN, , 0];
>+    shouldBe(array.lastIndexOf(Number.NaN), -1);
>+}
>diff --git a/JSTests/stress/array-lastindexof-infinity.js b/JSTests/stress/array-lastindexof-infinity.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..3f0a4b616d72bd6106e6617ae95f702a410e33f6
>--- /dev/null
>+++ b/JSTests/stress/array-lastindexof-infinity.js
>@@ -0,0 +1,21 @@
>+function shouldBe(actual, expected) {
>+    if (actual !== expected)
>+        throw new Error('bad value: ' + actual);
>+}
>+
>+{
>+    let array = [42.195, -0.0, -Infinity];
>+    shouldBe(array.lastIndexOf(Infinity), -1);
>+    shouldBe(array.lastIndexOf(-Infinity), 2);
>+    shouldBe(array.lastIndexOf(42), -1);
>+}
>+{
>+    let array = [1, 2, 3, 0];
>+    shouldBe(array.lastIndexOf(Infinity), -1);
>+    shouldBe(array.lastIndexOf(-Infinity), -1);
>+}
>+{
>+    let array = ["String", 42.5, Infinity, 33];
>+    shouldBe(array.lastIndexOf(Infinity), 2);
>+    shouldBe(array.lastIndexOf(-Infinity), -1);
>+}
>diff --git a/JSTests/stress/array-lastindexof-negative-zero.js b/JSTests/stress/array-lastindexof-negative-zero.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..42bf2ad5d2d1fa470a7e1b72c03ca1e5c850db13
>--- /dev/null
>+++ b/JSTests/stress/array-lastindexof-negative-zero.js
>@@ -0,0 +1,25 @@
>+function shouldBe(actual, expected) {
>+    if (actual !== expected)
>+        throw new Error('bad value: ' + actual);
>+}
>+
>+{
>+    let array = [42.195, -0.0];
>+    shouldBe(array.lastIndexOf(0), 1);
>+    shouldBe(array.lastIndexOf(-0), 1);
>+}
>+{
>+    let array = [42.195, 0, -0.0];
>+    shouldBe(array.lastIndexOf(0), 2);
>+    shouldBe(array.lastIndexOf(-0), 2);
>+}
>+{
>+    let array = [42, 0];
>+    shouldBe(array.lastIndexOf(0), 1);
>+    shouldBe(array.lastIndexOf(-0), 1);
>+}
>+{
>+    let array = [42, 0, 44];
>+    shouldBe(array.lastIndexOf(0), 1);
>+    shouldBe(array.lastIndexOf(-0), 1);
>+}
>diff --git a/JSTests/stress/array-lastindexof-own-getter.js b/JSTests/stress/array-lastindexof-own-getter.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..adab0917d4b40c26448e29f7ea58a1203dbcecb1
>--- /dev/null
>+++ b/JSTests/stress/array-lastindexof-own-getter.js
>@@ -0,0 +1,66 @@
>+function shouldBe(actual, expected) {
>+    if (actual !== expected)
>+        throw new Error('bad value: ' + actual);
>+}
>+
>+{
>+    let array = [];
>+    Object.defineProperty(array, 2, {
>+        get() {
>+            this.called = true;
>+            return 42;
>+        }
>+    });
>+    array.length = 42;
>+    shouldBe(array.lastIndexOf(42), 2);
>+    shouldBe(array.called, true);
>+}
>+{
>+    let array = [20, 20];
>+    Object.defineProperty(array, 2, {
>+        get() {
>+            this.called = true;
>+            return 42;
>+        }
>+    });
>+    array.length = 42;
>+    shouldBe(array.lastIndexOf(42), 2);
>+    shouldBe(array.called, true);
>+}
>+{
>+    let array = ["Hello"];
>+    Object.defineProperty(array, 2, {
>+        get() {
>+            this.called = true;
>+            return 42;
>+        }
>+    });
>+    array.length = 42;
>+    shouldBe(array.lastIndexOf(42), 2);
>+    shouldBe(array.called, true);
>+}
>+{
>+    let array = [42.195];
>+    Object.defineProperty(array, 2, {
>+        get() {
>+            this.called = true;
>+            return 42;
>+        }
>+    });
>+    array.length = 42;
>+    shouldBe(array.lastIndexOf(42), 2);
>+    shouldBe(array.called, true);
>+}
>+{
>+    let array = ["Hello"];
>+    Object.defineProperty(array, 2, {
>+        get() {
>+            this.called = true;
>+            return 42;
>+        }
>+    });
>+    array.length = 42;
>+    ensureArrayStorage(array);
>+    shouldBe(array.lastIndexOf(42), 2);
>+    shouldBe(array.called, true);
>+}
>diff --git a/JSTests/stress/array-lastindexof-prototype-trap.js b/JSTests/stress/array-lastindexof-prototype-trap.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..ee5edcb04fdd99163a98f2a92c1fd9390ed999da
>--- /dev/null
>+++ b/JSTests/stress/array-lastindexof-prototype-trap.js
>@@ -0,0 +1,48 @@
>+function shouldBe(actual, expected) {
>+    if (actual !== expected)
>+        throw new Error('bad value: ' + actual);
>+}
>+
>+class DerivedArray extends Array {
>+    get 2() {
>+        this.called = true;
>+        return 42;
>+    }
>+}
>+
>+{
>+    let array = [];
>+    array.__proto__ = DerivedArray.prototype;
>+    array.length = 42;
>+    shouldBe(array.lastIndexOf(42), 2);
>+    shouldBe(array.called, true);
>+}
>+{
>+    let array = [20, 20];
>+    array.__proto__ = DerivedArray.prototype;
>+    array.length = 42;
>+    shouldBe(array.lastIndexOf(42), 2);
>+    shouldBe(array.called, true);
>+}
>+{
>+    let array = ["Hello"];
>+    array.__proto__ = DerivedArray.prototype;
>+    array.length = 42;
>+    shouldBe(array.lastIndexOf(42), 2);
>+    shouldBe(array.called, true);
>+}
>+{
>+    let array = [42.195];
>+    array.__proto__ = DerivedArray.prototype;
>+    array.length = 42;
>+    shouldBe(array.lastIndexOf(42), 2);
>+    shouldBe(array.called, true);
>+}
>+{
>+    let array = ["Hello"];
>+    array.__proto__ = DerivedArray.prototype;
>+    array.length = 42;
>+    ensureArrayStorage(array);
>+    shouldBe(array.lastIndexOf(42), 2);
>+    shouldBe(array.called, true);
>+}

Flags:

saam: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 189780: 350179