Attachment 361577 Details for Bug 194466 – Patch

[patch] Patch

bug-194466-20190208183306.patch (text/plain), 4.92 KB, created by Yusuke Suzuki on 2019-02-08 18:33:06 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2019-02-08 18:33:06 PST

Size: 4.92 KB

patch

obsolete

>Subversion Revision: 241201
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 66e4837d6ad2b79afe7bcb1795ade1e788b4495b..ca858d8963452e235b8bff8723d716dd161be1fa 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,22 @@
>+2019-02-08  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        [JSC] String.fromCharCode's slow path always generates 16bit string
>+        https://bugs.webkit.org/show_bug.cgi?id=194466
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        String.fromCharCode(a1) has a fast path and the most frequently used. And String.fromCharCode(a1, a2, ...)
>+        goes to the slow path. However, in the slow path, we always create 16bit string. 16bit string takes 2x memory,
>+        and even worse, taints ropes 16bit if 16bit string is included in the given rope. We find that acorn-wtb
>+        creates very large strings multiple times with String.fromCharCode, and String.fromCharCode always produces
>+        16bit string. However, only few strings are actually 16bit strings. This patch attempts to make 8bit string
>+        as much as possible.
>+
>+        It improves non JIT acorn-wtb's peak and current memory footprint by 6% and 3% respectively.
>+
>+        * runtime/StringConstructor.cpp:
>+        (JSC::stringFromCharCode):
>+
> 2019-02-08  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         [JSC] SourceProviderCacheItem should be small
>diff --git a/Source/JavaScriptCore/runtime/StringConstructor.cpp b/Source/JavaScriptCore/runtime/StringConstructor.cpp
>index bd489375e2c6f431039960695017e531264aa702..6606bb9e465b9f0f75994987320600e0a9bf3ec9 100644
>--- a/Source/JavaScriptCore/runtime/StringConstructor.cpp
>+++ b/Source/JavaScriptCore/runtime/StringConstructor.cpp
>@@ -83,13 +83,26 @@ static EncodedJSValue JSC_HOST_CALL stringFromCharCode(ExecState* exec)
>         return JSValue::encode(jsSingleCharacterString(exec, code));
>     }
> 
>-    UChar* buf;
>-    auto impl = StringImpl::createUninitialized(length, buf);
>+    LChar* buf8Bit;
>+    auto impl8Bit = StringImpl::createUninitialized(length, buf8Bit);
>     for (unsigned i = 0; i < length; ++i) {
>-        buf[i] = static_cast<UChar>(exec->uncheckedArgument(i).toUInt32(exec));
>+        UChar character = static_cast<UChar>(exec->uncheckedArgument(i).toUInt32(exec));
>         RETURN_IF_EXCEPTION(scope, encodedJSValue());
>+        if (UNLIKELY(character & 0xFF00)) {
>+            UChar* buf16Bit;
>+            auto impl16Bit = StringImpl::createUninitialized(length, buf16Bit);
>+            StringImpl::copyCharacters(buf16Bit, buf8Bit, i);
>+            buf16Bit[i] = character;
>+            ++i;
>+            for (; i < length; ++i) {
>+                buf16Bit[i] = static_cast<UChar>(exec->uncheckedArgument(i).toUInt32(exec));
>+                RETURN_IF_EXCEPTION(scope, encodedJSValue());
>+            }
>+            RELEASE_AND_RETURN(scope, JSValue::encode(jsString(exec, WTFMove(impl16Bit))));
>+        }
>+        buf8Bit[i] = static_cast<LChar>(character);
>     }
>-    RELEASE_AND_RETURN(scope, JSValue::encode(jsString(exec, WTFMove(impl))));
>+    RELEASE_AND_RETURN(scope, JSValue::encode(jsString(exec, WTFMove(impl8Bit))));
> }
> 
> JSString* JSC_HOST_CALL stringFromCharCode(ExecState* exec, int32_t arg)
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index d69169f5ca574699219ff7ba5b0cbd3379c0775d..cd989fc8256c1fbec05df74e91d12268d98b36a4 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-02-08  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        [JSC] String.fromCharCode's slow path always generates 16bit string
>+        https://bugs.webkit.org/show_bug.cgi?id=194466
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * stress/string-from-char-code-slow-path.js: Added.
>+        (shouldBe):
>+        (testWithLength):
>+
> 2019-02-06  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         [JSC] InitializeEntrypointArguments should produce SpecCellCheck if FlushFormat is FlushedCell
>diff --git a/JSTests/stress/string-from-char-code-slow-path.js b/JSTests/stress/string-from-char-code-slow-path.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..9b6d97fa79fc78743251dedc0016234c0c7030f1
>--- /dev/null
>+++ b/JSTests/stress/string-from-char-code-slow-path.js
>@@ -0,0 +1,26 @@
>+function shouldBe(actual, expected) {
>+    if (actual !== expected)
>+        throw new Error('bad value: ' + actual);
>+}
>+
>+function testWithLength(length, index) {
>+    shouldBe(length >= 1, true);
>+    var array = [];
>+    for (var i = 0; i < length; ++i)
>+        array[i] = i & 0xff;
>+    array[index] = 0xffef;
>+    var string = String.fromCharCode.apply(String, array);
>+    shouldBe(string.length, length);
>+    for (var i = 0; i < length; ++i) {
>+        if (index === i)
>+            shouldBe(string[i], String.fromCharCode(0xffef));
>+        else
>+            shouldBe(string[i], String.fromCharCode(i & 0xff));
>+    }
>+}
>+
>+testWithLength(1e4, 1e4 - 1);
>+testWithLength(1e4, 1e3);
>+testWithLength(1, 0);
>+testWithLength(2, 1);
>+testWithLength(2, 0);

Flags:

keith_miller: review+
ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 194466: 361577 | 361591