Attachment 369554 Details for Bug 197015 – Patch

[patch] Patch

bug-197015-20190510183342.patch (text/plain), 6.65 KB, created by Yacine Bandou on 2019-05-10 09:33:43 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yacine Bandou

Created: 2019-05-10 09:33:43 PDT

Size: 6.65 KB

patch

obsolete

>Subversion Revision: 245126
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index d28ce69f4a1c3ed8500abafd9dfa14ef48e71908..3d8c112884c2f01b8cd1c4b901fde4aa7c9fc38a 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,22 @@
>+2019-05-10  Yacine Bandou  <yacine.bandou@softathome.com>
>+
>+        [JSC] Optimize cases of Array.prototype.join/toString on huge empty arrays which result into one-character repeats
>+        https://bugs.webkit.org/show_bug.cgi?id=197015
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add a fast case in Array.prototype.join when the array is empty and it uses array_storage indexingType and
>+        the separator length is less than 2.
>+
>+        Use the same optimisation that is used in the case of UNDECIDED indexingType, in order allocate the final
>+        string in one step, instead of appending in a loop which consumes additional temporary memory.
>+
>+        * runtime/ArrayPrototype.cpp:
>+        (JSC::canUseFastJoin):
>+        (JSC::fastJoin):
>+        (JSC::arrayProtoFuncToString):
>+        (JSC::arrayProtoFuncJoin):
>+
> 2019-05-08  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         Unreviewed, build fix after r245064
>diff --git a/Source/JavaScriptCore/runtime/ArrayPrototype.cpp b/Source/JavaScriptCore/runtime/ArrayPrototype.cpp
>index 43eec84cc4596e85742116cd301ca931f2dfd7a1..0dd2f32b2321f21501622be9ddf268ef74cd52e9 100644
>--- a/Source/JavaScriptCore/runtime/ArrayPrototype.cpp
>+++ b/Source/JavaScriptCore/runtime/ArrayPrototype.cpp
>@@ -383,7 +383,7 @@ void unshift(ExecState* exec, JSObject* thisObj, unsigned header, unsigned curre
>     }
> }
> 
>-inline bool canUseFastJoin(const JSObject* thisObject)
>+inline bool canUseFastJoin(const JSObject* thisObject, unsigned separatorLength)
> {
>     switch (thisObject->indexingType()) {
>     case ALL_CONTIGUOUS_INDEXING_TYPES:
>@@ -391,6 +391,15 @@ inline bool canUseFastJoin(const JSObject* thisObject)
>     case ALL_DOUBLE_INDEXING_TYPES:
>     case ALL_UNDECIDED_INDEXING_TYPES:
>         return true;
>+    // The fastJoin can be used in the case of array_storage, but it should be empty
>+    // in order to use repeatCharacter.
>+    case ARRAY_WITH_ARRAY_STORAGE_INDEXING_TYPES: {
>+        const ArrayStorage* storage = const_cast<JSObject*>(thisObject)->butterfly()->arrayStorage();
>+        SparseArrayValueMap* map = storage->m_sparseMap.get();
>+        if (!storage->m_numValuesInVector && (!map || !map->size()) && (separatorLength < 2))
>+            return true;
>+        break;
>+    }
>     default:
>         break;
>     }
>@@ -503,9 +512,13 @@ inline JSValue fastJoin(ExecState& state, JSObject* thisObject, StringView separ
>         }
>         RELEASE_AND_RETURN(scope, joiner.join(state));
>     }
>-    case ALL_UNDECIDED_INDEXING_TYPES: {
>+    case ALL_UNDECIDED_INDEXING_TYPES:
>         if (length && holesMustForwardToPrototype(vm, thisObject))
>             goto generalCase;
>+        FALLTHROUGH;
>+    // Use repeatCharacter function in case of array_storage indexingType.
>+    // In this case, is guaranteed that the array is empty and the separator length less than 2, the check is done in canUseFastJoin function.
>+    case ARRAY_WITH_ARRAY_STORAGE_INDEXING_TYPES: {
>         switch (separator.length()) {
>         case 0:
>             RELEASE_AND_RETURN(scope, jsEmptyString(&state));
>@@ -572,7 +585,7 @@ EncodedJSValue JSC_HOST_CALL arrayProtoFuncToString(ExecState* exec)
>     if (JSValue earlyReturnValue = checker.earlyReturnValue())
>         return JSValue::encode(earlyReturnValue);
> 
>-    if (LIKELY(canUseFastJoin(thisArray))) {
>+    if (LIKELY(canUseFastJoin(thisArray, 1))) {
>         const LChar comma = ',';
>         scope.release();
> 
>@@ -733,7 +746,7 @@ EncodedJSValue JSC_HOST_CALL arrayProtoFuncJoin(ExecState* exec)
>     if (separatorValue.isUndefined()) {
>         const LChar comma = ',';
> 
>-        if (UNLIKELY(length > std::numeric_limits<unsigned>::max() || !canUseFastJoin(thisObject))) {
>+        if (UNLIKELY(length > std::numeric_limits<unsigned>::max() || !canUseFastJoin(thisObject, 1))) {
>             uint64_t length64 = static_cast<uint64_t>(length);
>             ASSERT(static_cast<double>(length64) == length);
>             JSString* jsSeparator = jsSingleCharacterString(exec, comma);
>@@ -752,7 +765,7 @@ EncodedJSValue JSC_HOST_CALL arrayProtoFuncJoin(ExecState* exec)
>     JSString* jsSeparator = separatorValue.toString(exec);
>     RETURN_IF_EXCEPTION(scope, encodedJSValue());
> 
>-    if (UNLIKELY(length > std::numeric_limits<unsigned>::max() || !canUseFastJoin(thisObject))) {
>+    if (UNLIKELY(length > std::numeric_limits<unsigned>::max() || !canUseFastJoin(thisObject, jsSeparator->length()))) {
>         uint64_t length64 = static_cast<uint64_t>(length);
>         ASSERT(static_cast<double>(length64) == length);
> 
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index 50c268a423d90a7461ffe98dbf0c7e7930d43194..6cc2e2017922ed9b6a842adb9d57110eaf3279d5 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-05-10  Yacine Bandou  <yacine.bandou@softathome.com>
>+
>+        [JSC] Optimize cases of Array.prototype.join/toString on huge empty arrays which result into one-character repeats
>+        https://bugs.webkit.org/show_bug.cgi?id=197015
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add a test for Array.prototype.join() with a simple or a huge uninitialized empty array.
>+        Without this commit, this test uses a huge amount of CPU memory and freezes the system.
>+
>+        * stress/array-prototype-join-huge-empty-arrays.js: Added.
>+        (shouldPass):
>+        (testJoinArray):
>+
> 2019-05-08  Saam barati  <sbarati@apple.com>
> 
>         AccessGenerationState::emitExplicitExceptionHandler can clobber an in use register
>diff --git a/JSTests/stress/array-prototype-join-huge-empty-arrays.js b/JSTests/stress/array-prototype-join-huge-empty-arrays.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..c5cc06c659c04b3d13b30a599217d112d1440ba8
>--- /dev/null
>+++ b/JSTests/stress/array-prototype-join-huge-empty-arrays.js
>@@ -0,0 +1,33 @@
>+
>+function shouldPass(func) {
>+    var error = null;
>+    try {
>+        func();
>+    } catch (e) {
>+        error = e;
>+    }
>+    if (error)
>+        throw new Error("bad error: " + String(error));
>+}
>+
>+function testJoinArray(array, n) {
>+
>+    var s = array.join('M');
>+    if (s.charAt(n - 2) !== 'M')
>+        throw("Bad result for array.join, expected:\"M\" but got: \"" + s[n - 1] + "\"");
>+}
>+
>+// Simple test
>+shouldPass(function() {
>+    var n = 10;
>+    var array = new Array(n);
>+    testJoinArray(array, n);
>+});
>+
>+// Huge empty array storage test
>+shouldPass(function() {
>+    var n = 130 * 1024 * 1024;
>+    var array = new Array(n);
>+    testJoinArray(array, n);
>+});
>+

Flags:

bandou.yacine: review?
bandou.yacine: commit-queue?

Actions: View | Formatted Diff | Diff

Attachments on bug 197015: 367638 | 367645 | 369554