[JSC] Repeat string created from Array.prototype.join() take too much memory
authorguijemont@igalia.com <guijemont@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 26 Feb 2019 08:50:48 +0000 (08:50 +0000)
committerguijemont@igalia.com <guijemont@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 26 Feb 2019 08:50:48 +0000 (08:50 +0000)
https://bugs.webkit.org/show_bug.cgi?id=193912

Reviewed by Saam Barati.

JSTests:

Added a test and a microbenchmark for corner cases of
Array.prototype.join() with an uninitialized array.

* microbenchmarks/array-prototype-join-uninitialized.js: Added.
* stress/array-prototype-join-uninitialized.js: Added.
(testArray):
(testABC):
(B):
(C):

Source/JavaScriptCore:

Added a fast case in Array.prototype.join when the array is
uninitialized.

* runtime/ArrayPrototype.cpp:
(JSC::canUseFastJoin):
(JSC::fastJoin):
* runtime/JSStringInlines.h:
(JSC::repeatCharacter): moved from StringPrototype.cpp
* runtime/StringPrototype.cpp:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@242081 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/microbenchmarks/array-prototype-join-uninitialized.js [new file with mode: 0644]
JSTests/stress/array-prototype-join-uninitialized.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/ArrayPrototype.cpp
Source/JavaScriptCore/runtime/JSStringInlines.h
Source/JavaScriptCore/runtime/StringPrototype.cpp

index b4a0d7c..937a2bf 100644 (file)
@@ -1,3 +1,20 @@
+2019-02-26  Guillaume Emont  <guijemont@igalia.com>
+
+        [JSC] Repeat string created from Array.prototype.join() take too much memory
+        https://bugs.webkit.org/show_bug.cgi?id=193912
+
+        Reviewed by Saam Barati.
+
+        Added a test and a microbenchmark for corner cases of
+        Array.prototype.join() with an uninitialized array.
+
+        * microbenchmarks/array-prototype-join-uninitialized.js: Added.
+        * stress/array-prototype-join-uninitialized.js: Added.
+        (testArray):
+        (testABC):
+        (B):
+        (C):
+
 2019-02-22  Robin Morisset  <rmorisset@apple.com>
 
         DFGBytecodeParser should not declare that a node won't clobberExit if DFGFixupPhase can later declare it does clobberExit
diff --git a/JSTests/microbenchmarks/array-prototype-join-uninitialized.js b/JSTests/microbenchmarks/array-prototype-join-uninitialized.js
new file mode 100644 (file)
index 0000000..f7ce20d
--- /dev/null
@@ -0,0 +1,5 @@
+//@ runDefault
+var N = 10 * 1024 * 1024
+var s = Array(N).join();
+if (s !== ",".repeat(N - 1))
+    throw("Unexpected result of Array.prototype.join()");
diff --git a/JSTests/stress/array-prototype-join-uninitialized.js b/JSTests/stress/array-prototype-join-uninitialized.js
new file mode 100644 (file)
index 0000000..702d53d
--- /dev/null
@@ -0,0 +1,34 @@
+function testArray(array, expected) {
+    var s = array.join('M');
+    if (s !== expected)
+        throw("Bad result for array " + array + " expected: \"" + expected + "\" but got: \"" + s + "\"");
+}
+
+function testABC(n, resA, resB, resC) {
+    testArray(new Array(n), resA);
+    testArray(new B(n), resB);
+    testArray(new C(n), resC);
+}
+
+class B extends Array { }
+class C extends B { }
+
+
+testABC(0, "", "", "");
+testABC(1, "", "", "");
+testABC(3, "MM", "MM", "MM")
+
+B.prototype[0] = "foo";
+testABC(0, "", "", "");
+testABC(1, "", "foo", "foo");
+testABC(3, "MM", "fooMM", "fooMM");
+
+C.prototype[1] = "bar";
+testABC(0, "", "", "");
+testABC(1, "", "foo", "foo");
+testABC(3, "MM", "fooMM", "fooMbarM");
+
+Array.prototype[1] = "baz";
+testABC(0, "", "", "");
+testABC(1, "", "foo", "foo");
+testABC(3, "MbazM", "fooMbazM", "fooMbarM");
index ed735f0..c9dda48 100644 (file)
@@ -1,3 +1,20 @@
+2019-02-26  Guillaume Emont  <guijemont@igalia.com>
+
+        [JSC] Repeat string created from Array.prototype.join() take too much memory
+        https://bugs.webkit.org/show_bug.cgi?id=193912
+
+        Reviewed by Saam Barati.
+
+        Added a fast case in Array.prototype.join when the array is
+        uninitialized.
+
+        * runtime/ArrayPrototype.cpp:
+        (JSC::canUseFastJoin):
+        (JSC::fastJoin):
+        * runtime/JSStringInlines.h:
+        (JSC::repeatCharacter): moved from StringPrototype.cpp
+        * runtime/StringPrototype.cpp:
+
 2019-02-25  Mark Lam  <mark.lam@apple.com>
 
         Add some randomness into the StructureID.
index c37389a..1a37b3f 100644 (file)
@@ -390,6 +390,7 @@ inline bool canUseFastJoin(const JSObject* thisObject)
     case ALL_CONTIGUOUS_INDEXING_TYPES:
     case ALL_INT32_INDEXING_TYPES:
     case ALL_DOUBLE_INDEXING_TYPES:
+    case ALL_UNDECIDED_INDEXING_TYPES:
         return true;
     default:
         break;
@@ -503,6 +504,21 @@ inline JSValue fastJoin(ExecState& state, JSObject* thisObject, StringView separ
         }
         RELEASE_AND_RETURN(scope, joiner.join(state));
     }
+    case ALL_UNDECIDED_INDEXING_TYPES: {
+        if (length && holesMustForwardToPrototype(vm, thisObject))
+            goto generalCase;
+        switch (separator.length()) {
+        case 0:
+            RELEASE_AND_RETURN(scope, jsEmptyString(&state));
+        case 1: {
+            if (length <= 1)
+                RELEASE_AND_RETURN(scope, jsEmptyString(&state));
+            if (separator.is8Bit())
+                RELEASE_AND_RETURN(scope, repeatCharacter(state, separator.characters8()[0], length - 1));
+            RELEASE_AND_RETURN(scope, repeatCharacter(state, separator.characters16()[0], length - 1));
+        }
+        }
+    }
     }
 
 generalCase:
index 19d4756..a27cf50 100644 (file)
@@ -54,4 +54,22 @@ inline JSValue jsMakeNontrivialString(ExecState* exec, StringType&& string, Stri
     return jsNontrivialString(exec, WTFMove(result));
 }
 
+template <typename CharacterType>
+inline JSString* repeatCharacter(ExecState& exec, CharacterType character, unsigned repeatCount)
+{
+    VM& vm = exec.vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
+    CharacterType* buffer = nullptr;
+    auto impl = StringImpl::tryCreateUninitialized(repeatCount, buffer);
+    if (!impl) {
+        throwOutOfMemoryError(&exec, scope);
+        return nullptr;
+    }
+
+    std::fill_n(buffer, repeatCount, character);
+
+    RELEASE_AND_RETURN(scope, jsString(&exec, WTFMove(impl)));
+}
+
 } // namespace JSC
index 571212a..7000168 100644 (file)
@@ -837,24 +837,6 @@ static inline bool checkObjectCoercible(JSValue thisValue)
     return true;
 }
 
-template <typename CharacterType>
-static inline JSString* repeatCharacter(ExecState& exec, CharacterType character, unsigned repeatCount)
-{
-    VM& vm = exec.vm();
-    auto scope = DECLARE_THROW_SCOPE(vm);
-
-    CharacterType* buffer = nullptr;
-    auto impl = StringImpl::tryCreateUninitialized(repeatCount, buffer);
-    if (!impl) {
-        throwOutOfMemoryError(&exec, scope);
-        return nullptr;
-    }
-
-    std::fill_n(buffer, repeatCount, character);
-
-    RELEASE_AND_RETURN(scope, jsString(&exec, WTFMove(impl)));
-}
-
 EncodedJSValue JSC_HOST_CALL stringProtoFuncRepeatCharacter(ExecState* exec)
 {
     VM& vm = exec->vm();