Attachment 370689 Details for Bug 198271 – Patch

[patch] Patch

bug-198271-20190527140525.patch (text/plain), 5.33 KB, created by Tadeu Zagallo on 2019-05-27 05:05:26 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Tadeu Zagallo

Created: 2019-05-27 05:05:26 PDT

Size: 5.33 KB

patch

obsolete

>Subversion Revision: 245789
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index df5adc4cc725fa461b2ac8974f772558f11a5455..63ac945ddcc36d267e050ec20b128b8c089a816b 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,18 @@
>+2019-05-27  Tadeu Zagallo  <tzagallo@apple.com>
>+
>+        JITOperations putByVal should mark negative array indices as out-of-bounds
>+        https://bugs.webkit.org/show_bug.cgi?id=198271
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Similar to what was done to getByVal in r245769, we should also mark put_by_val as out-of-bounds
>+        when we exit from DFG for putting to a negative index. This avoids the same scenario where we keep
>+        recompiling a CodeBlock with DFG and exiting at the same bytecode.
>+
>+        This is a 3.7x improvement in the microbenchmark being added: put-by-val-negative-array-index.js.
>+
>+        * jit/JITOperations.cpp:
>+
> 2019-05-25  Tadeu Zagallo  <tzagallo@apple.com>
> 
>         JITOperations getByVal should mark negative array indices as out-of-bounds
>diff --git a/Source/JavaScriptCore/jit/JITOperations.cpp b/Source/JavaScriptCore/jit/JITOperations.cpp
>index b5c545b8c0cbf95b25e1c5b676e6214a574410a7..ac97022b1f3b3f2fa608b4f035729efbaaa8c1d5 100644
>--- a/Source/JavaScriptCore/jit/JITOperations.cpp
>+++ b/Source/JavaScriptCore/jit/JITOperations.cpp
>@@ -631,12 +631,12 @@ static void putByVal(CallFrame* callFrame, JSValue baseValue, JSValue subscript,
> {
>     VM& vm = callFrame->vm();
>     auto scope = DECLARE_THROW_SCOPE(vm);
>-    if (LIKELY(subscript.isUInt32())) {
>+    if (LIKELY(subscript.isInt32())) {
>         byValInfo->tookSlowPath = true;
>-        uint32_t i = subscript.asUInt32();
>+        int32_t i = subscript.asInt32();
>         if (baseValue.isObject()) {
>             JSObject* object = asObject(baseValue);
>-            if (object->canSetIndexQuickly(i)) {
>+            if (i >= 0 && object->canSetIndexQuickly(i)) {
>                 object->setIndexQuickly(vm, i, value);
>                 return;
>             }
>@@ -645,16 +645,24 @@ static void putByVal(CallFrame* callFrame, JSValue baseValue, JSValue subscript,
>             // out-of-bounds.
>             // https://bugs.webkit.org/show_bug.cgi?id=149886
>             byValInfo->arrayProfile->setOutOfBounds();
>+
>+            if (i < 0)
>+                goto putProperty;
>+
>             scope.release();
>             object->methodTable(vm)->putByIndex(object, callFrame, i, value, callFrame->codeBlock()->isStrictMode());
>             return;
>         }
> 
>+        if (i < 0)
>+            goto putProperty;
>+
>         scope.release();
>         baseValue.putByIndex(callFrame, i, value, callFrame->codeBlock()->isStrictMode());
>         return;
>     }
> 
>+putProperty:
>     auto property = subscript.toPropertyKey(callFrame);
>     // Don't put to an object if toString threw an exception.
>     RETURN_IF_EXCEPTION(scope, void());
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index 96d083c93a85dfc81c589526c3a439dbd8bd881a..40a7d329df6af1d34ee7866de1c2ecfb48c87c43 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,18 @@
>+2019-05-27  Tadeu Zagallo  <tzagallo@apple.com>
>+
>+        JITOperations putByVal should mark negative array indices as out-of-bounds
>+        https://bugs.webkit.org/show_bug.cgi?id=198271
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * microbenchmarks/get-by-val-negative-array-index.js:
>+        (foo):
>+        Update the getByVal microbenchmark added in r245769. This now shows that r245769
>+        is 4.2x faster than the previous commit.
>+
>+        * microbenchmarks/put-by-val-negative-array-index.js: Added.
>+        (foo):
>+
> 2019-05-25  Tadeu Zagallo  <tzagallo@apple.com>
> 
>         JITOperations getByVal should mark negative array indices as out-of-bounds
>diff --git a/JSTests/microbenchmarks/get-by-val-negative-array-index.js b/JSTests/microbenchmarks/get-by-val-negative-array-index.js
>index 1a86915f59c47e8793a3ddac87a1ae6ea11819fe..1d67c2c2573b8ebf4fa4c95e0e96f8a94b916f8f 100644
>--- a/JSTests/microbenchmarks/get-by-val-negative-array-index.js
>+++ b/JSTests/microbenchmarks/get-by-val-negative-array-index.js
>@@ -1,11 +1,19 @@
> function foo(arr, index) {
>+    for (let i = 0; i < 1e2; i++) {
>+        let x = {};
>+        x.x = arr;
>+    }
>+
>     return arr[index];
> }
> noInline(foo);
> 
>-const arr = new Array(1000).fill({});
>-for (let i = 0; i < 1e7; i++) {
>+const arr = new Array(10).fill({});
>+for (let i = 0; i < 1e6; i++) {
>+    foo(arr, i % arr.length);
>+}
>+for (let i = 0; i < 1e6; i++) {
>     foo(arr, i % arr.length);
>-    if (!(i % 1e3))
>+    if (!(i % arr.length))
>         foo(arr, -1);
> }
>diff --git a/JSTests/microbenchmarks/put-by-val-negative-array-index.js b/JSTests/microbenchmarks/put-by-val-negative-array-index.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..5f8746e56f9e70a18c9557d3da4a3810d367fd1f
>--- /dev/null
>+++ b/JSTests/microbenchmarks/put-by-val-negative-array-index.js
>@@ -0,0 +1,20 @@
>+function foo(arr, index) {
>+    arr[index] = index;
>+
>+    for (let j = 0; j < 1e2; j++) {
>+        let x = {};
>+        x.x = arr;
>+    }
>+}
>+noInline(foo);
>+
>+const arr = new Array(10).fill({});
>+let result = 0;
>+for (let i = 0; i < 1e6; i++) {
>+    result += foo(arr, i % arr.length);
>+}
>+for (let i = 0; i < 1e6; i++) {
>+    result += foo(arr, i % arr.length);
>+    if (!(i % arr.length))
>+        result += foo(arr, -1);
>+}

Actions: View | Formatted Diff | Diff

Attachments on bug 198271: 370689 | 370745