Attachment 370745 Details for Bug 198271 – Patch for landing

[patch] Patch for landing

bug-198271-20190528174747.patch (text/plain), 4.27 KB, created by Tadeu Zagallo on 2019-05-28 08:47:49 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Tadeu Zagallo

Created: 2019-05-28 08:47:49 PDT

Size: 4.27 KB

patch

obsolete

>Subversion Revision: 245789
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index df5adc4cc725fa461b2ac8974f772558f11a5455..0dac348b05118e006d1d9bbeedd228542f342cee 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 Saam Barati.
>+
>+        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..fbd6d7369b952cd977cc5223a1db8ab9836f96ac 100644
>--- a/Source/JavaScriptCore/jit/JITOperations.cpp
>+++ b/Source/JavaScriptCore/jit/JITOperations.cpp
>@@ -653,6 +653,10 @@ static void putByVal(CallFrame* callFrame, JSValue baseValue, JSValue subscript,
>         scope.release();
>         baseValue.putByIndex(callFrame, i, value, callFrame->codeBlock()->isStrictMode());
>         return;
>+    } else if (subscript.isInt32()) {
>+        byValInfo->tookSlowPath = true;
>+        if (baseValue.isObject())
>+            byValInfo->arrayProfile->setOutOfBounds();
>     }
> 
>     auto property = subscript.toPropertyKey(callFrame);
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index 96d083c93a85dfc81c589526c3a439dbd8bd881a..c57fd6ba411d5a09f1f518a5307dd1db2eedb210 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 Saam Barati.
>+
>+        * 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