Attachment 342879 Details for Bug 186721 – Patch

[patch] Patch

bug-186721-20180617002237.patch (text/plain), 6.61 KB, created by Yusuke Suzuki on 2018-06-16 08:22:37 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2018-06-16 08:22:37 PDT

Size: 6.61 KB

patch

obsolete

>Subversion Revision: 232902
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index da5b38eee1bb5c58520859cf11524c61e114b2f3..df09f24790de338326f9dc31ca21a615c886b87c 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,32 @@
>+2018-06-16  Yusuke Suzuki  <utatane.tea@gmail.com>
>+
>+        [DFG] Reduce OSRExit for Kraken/crypto-aes due to CoW array
>+        https://bugs.webkit.org/show_bug.cgi?id=186721
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        We still have several other OSRExits, but this patch reduces that.
>+
>+        1. While ArraySlice code accepts CoW arrays, it always emits CheckStructure without CoW Array structures.
>+        So DFG emits ArraySlice onto CoW arrays, and always performs OSRExits.
>+
>+        2. The CoW patch removed ArrayAllocationProfile updates. This makes allocated JSImmutableButterfly
>+        non-appropriate.
>+
>+        These changes a bit fix Kraken/crypto-aes regression.
>+
>+                                      baseline                  patched
>+
>+        stanford-crypto-aes        63.718+-2.312      ^      56.140+-0.966         ^ definitely 1.1350x faster
>+
>+
>+        * dfg/DFGByteCodeParser.cpp:
>+        (JSC::DFG::ByteCodeParser::handleIntrinsicCall):
>+        * ftl/FTLOperations.cpp:
>+        (JSC::FTL::operationMaterializeObjectInOSR):
>+        * runtime/CommonSlowPaths.cpp:
>+        (JSC::SLOW_PATH_DECL):
>+
> 2018-06-15  Yusuke Suzuki  <utatane.tea@gmail.com>
> 
>         [DFG][FTL] Spread onto PhantomNewArrayBuffer assumes JSFixedArray, but JSImmutableButterfly is returned
>diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>index 2541c17474f8236ff0dd6373348e204256e7660c..a2f9237124939e33995894ce213579b9c04c8f38 100644
>--- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>@@ -2303,6 +2303,9 @@ bool ByteCodeParser::handleIntrinsicCall(Node* callee, int resultOperand, Intrin
>                 structureSet.add(globalObject->originalArrayStructureForIndexingType(ArrayWithInt32));
>                 structureSet.add(globalObject->originalArrayStructureForIndexingType(ArrayWithContiguous));
>                 structureSet.add(globalObject->originalArrayStructureForIndexingType(ArrayWithDouble));
>+                structureSet.add(globalObject->originalArrayStructureForIndexingType(CopyOnWriteArrayWithInt32));
>+                structureSet.add(globalObject->originalArrayStructureForIndexingType(CopyOnWriteArrayWithContiguous));
>+                structureSet.add(globalObject->originalArrayStructureForIndexingType(CopyOnWriteArrayWithDouble));
>                 addToGraph(CheckStructure, OpInfo(m_graph.addStructureSet(structureSet)), array);
> 
>                 addVarArgChild(array);
>diff --git a/Source/JavaScriptCore/ftl/FTLOperations.cpp b/Source/JavaScriptCore/ftl/FTLOperations.cpp
>index b016e188aded9a9d5282b5f50c9f2eede3ba39c7..8fbbfb244f0907db15d7a43e1535791c4c50ea50 100644
>--- a/Source/JavaScriptCore/ftl/FTLOperations.cpp
>+++ b/Source/JavaScriptCore/ftl/FTLOperations.cpp
>@@ -474,9 +474,12 @@ extern "C" JSCell* JIT_OPERATION operationMaterializeObjectInOSR(
>         CodeBlock* codeBlock = baselineCodeBlockForOriginAndBaselineCodeBlock(materialization->origin(), exec->codeBlock());
>         Instruction* currentInstruction = &codeBlock->instructions()[materialization->origin().bytecodeIndex];
>         RELEASE_ASSERT(Interpreter::getOpcodeID(currentInstruction[0].u.opcode) == op_new_array_buffer);
>-        Structure* structure = exec->lexicalGlobalObject()->arrayStructureForIndexingTypeDuringAllocation(currentInstruction[3].u.arrayAllocationProfile->selectIndexingType());
>+        ArrayAllocationProfile* profile = currentInstruction[3].u.arrayAllocationProfile;
>+        Structure* structure = exec->lexicalGlobalObject()->arrayStructureForIndexingTypeDuringAllocation(profile->selectIndexingType());
>         ASSERT(!structure->outOfLineCapacity());
>-        return JSArray::createWithButterfly(vm, nullptr, structure, array->toButterfly());
>+        JSArray* result = JSArray::createWithButterfly(vm, nullptr, structure, array->toButterfly());
>+        ArrayAllocationProfile::updateLastAllocationFor(profile, result);
>+        return result;
>     }
> 
>     case PhantomNewArrayWithSpread: {
>diff --git a/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp b/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp
>index 1dafc63caff78ebafa37f06a2b5d6748dde3fee3..dabd9c037af1d395d4405b6f83b1b2c0ead4798f 100644
>--- a/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp
>+++ b/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp
>@@ -1163,6 +1163,7 @@ SLOW_PATH_DECL(slow_path_new_array_buffer)
> 
>     JSArray* result = CommonSlowPaths::allocateNewArrayBuffer(vm, structure, immutableButterfly);
>     ASSERT(isCopyOnWrite(result->indexingMode()) || exec->lexicalGlobalObject()->isHavingABadTime());
>+    ArrayAllocationProfile::updateLastAllocationFor(profile, result);
>     RETURN(result);
> }
> 
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index dd389dbdd2e994b42514f0a923ddb1154241a5cb..2c408bae41d0c30690904b33a314c48cb625592e 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,16 @@
>+2018-06-16  Yusuke Suzuki  <utatane.tea@gmail.com>
>+
>+        [DFG] Reduce OSRExit for Kraken/crypto-aes due to CoW array
>+        https://bugs.webkit.org/show_bug.cgi?id=186721
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * stress/array-slice-cow.js: Added.
>+        (shouldBe):
>+        (testInt32):
>+        (testDouble):
>+        (testContiguous):
>+
> 2018-06-15  Yusuke Suzuki  <utatane.tea@gmail.com>
> 
>         [DFG][FTL] Spread onto PhantomNewArrayBuffer assumes JSFixedArray, but JSImmutableButterfly is returned
>diff --git a/JSTests/stress/array-slice-cow.js b/JSTests/stress/array-slice-cow.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..57f1ccf78e9047821f5b65ed8735dc8aedde31b7
>--- /dev/null
>+++ b/JSTests/stress/array-slice-cow.js
>@@ -0,0 +1,31 @@
>+function shouldBe(actual, expected) {
>+    if (actual !== expected)
>+        throw new Error('bad value: ' + actual);
>+}
>+
>+function testInt32()
>+{
>+    var array = [0, 1, 2, 3];
>+    return array.slice(1);
>+}
>+noInline(testInt32);
>+
>+function testDouble()
>+{
>+    var array = [0.1, 1.1, 2.1, 3.1];
>+    return array.slice(1);
>+}
>+noInline(testDouble);
>+
>+function testContiguous()
>+{
>+    var array = [true, false, true, false];
>+    return array.slice(1);
>+}
>+noInline(testContiguous);
>+
>+for (var i = 0; i < 1e4; ++i) {
>+    shouldBe(JSON.stringify(testInt32()), `[1,2,3]`);
>+    shouldBe(JSON.stringify(testDouble()), `[1.1,2.1,3.1]`);
>+    shouldBe(JSON.stringify(testContiguous()), `[false,true,false]`);
>+}

Actions: View | Formatted Diff | Diff

Attachments on bug 186721: 342879 | 342880