Attachment 361655 Details for Bug 194488 – ROLLOUT of r241230

[patch] ROLLOUT of r241230

bug-194488-20190210200713.patch (text/plain), 10.29 KB, created by WebKit Commit Bot on 2019-02-10 20:07:14 PST

(hide)

Description:

Filename:

MIME Type:

Creator: WebKit Commit Bot

Created: 2019-02-10 20:07:14 PST

Size: 10.29 KB

patch

obsolete

>Subversion Revision: 241254
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 3dd3f856bcccc41b31e10604a651a9312b859373..f926af0a16ac919b1cc839b4ae159a2d598c7618 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,18 @@
>+2019-02-10  Commit Queue  <commit-queue@webkit.org>
>+
>+        Unreviewed, rolling out r241230.
>+        https://bugs.webkit.org/show_bug.cgi?id=194488
>+
>+        "It regressed JetStream2 by ~6%" (Requested by saamyjoon on
>+        #webkit).
>+
>+        Reverted changeset:
>+
>+        "We should only make rope strings when concatenating strings
>+        long enough."
>+        https://bugs.webkit.org/show_bug.cgi?id=194465
>+        https://trac.webkit.org/changeset/241230
>+
> 2019-02-10  Saam barati  <sbarati@apple.com>
> 
>         BBQ-Air: Emit better code for switch
>diff --git a/Source/JavaScriptCore/dfg/DFGOperations.cpp b/Source/JavaScriptCore/dfg/DFGOperations.cpp
>index 6a578266eb4a128327e53cdaacb7d861577c49ef..984fc0b961e500954d6df0c4a9d3a677749eaaff 100644
>--- a/Source/JavaScriptCore/dfg/DFGOperations.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGOperations.cpp
>@@ -503,7 +503,12 @@ EncodedJSValue JIT_OPERATION operationValueAddNotNumber(ExecState* exec, Encoded
>     JSValue op1 = JSValue::decode(encodedOp1);
>     JSValue op2 = JSValue::decode(encodedOp2);
>     
>-    return JSValue::encode(jsAddNonNumber(exec, op1, op2));
>+    ASSERT(!op1.isNumber() || !op2.isNumber());
>+    
>+    if (op1.isString() && !op2.isObject())
>+        return JSValue::encode(jsString(exec, asString(op1), op2.toString(exec)));
>+
>+    return JSValue::encode(jsAddSlowCase(exec, op1, op2));
> }
> 
> EncodedJSValue JIT_OPERATION operationValueDiv(ExecState* exec, EncodedJSValue encodedOp1, EncodedJSValue encodedOp2)
>diff --git a/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp b/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp
>index 5871c930bbc8e06636110ea4c6769b202340751d..bbbadab67fdf6fcad4cac0664cdc8e60bf5e4b2e 100644
>--- a/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp
>+++ b/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp
>@@ -517,11 +517,19 @@ SLOW_PATH_DECL(slow_path_add)
>     auto bytecode = pc->as<OpAdd>();
>     JSValue v1 = GET_C(bytecode.m_lhs).jsValue();
>     JSValue v2 = GET_C(bytecode.m_rhs).jsValue();
>+    JSValue result;
> 
>     ArithProfile& arithProfile = *exec->codeBlock()->arithProfileForPC(pc);
>     arithProfile.observeLHSAndRHS(v1, v2);
> 
>-    JSValue result = jsAdd(exec, v1, v2);
>+    if (v1.isString() && !v2.isObject()) {
>+        JSString* v2String = v2.toString(exec);
>+        if (LIKELY(!throwScope.exception()))
>+            result = jsString(exec, asString(v1), v2String);
>+    } else if (v1.isNumber() && v2.isNumber())
>+        result = jsNumber(v1.asNumber() + v2.asNumber());
>+    else
>+        result = jsAddSlowCase(exec, v1, v2);
> 
>     RETURN_WITH_PROFILING(result, {
>         updateArithProfileForBinaryArithOp(exec, pc, result, v1, v2);
>diff --git a/Source/JavaScriptCore/runtime/JSString.h b/Source/JavaScriptCore/runtime/JSString.h
>index 932b9f2f5caf84c046c49d47f7fe945cfd591ad8..00afd41432204d0042ba7a95390641d17bddcc92 100644
>--- a/Source/JavaScriptCore/runtime/JSString.h
>+++ b/Source/JavaScriptCore/runtime/JSString.h
>@@ -486,8 +486,6 @@ private:
> 
> 
>     friend JSString* jsString(ExecState*, JSString*, JSString*);
>-    friend JSString* jsString(ExecState*, const String&, JSString*);
>-    friend JSString* jsString(ExecState*, JSString*, const String&);
>     friend JSString* jsString(ExecState*, JSString*, JSString*, JSString*);
>     friend JSString* jsString(ExecState*, const String&, const String&, const String&);
> };
>diff --git a/Source/JavaScriptCore/runtime/Operations.cpp b/Source/JavaScriptCore/runtime/Operations.cpp
>index a5310b346d89945bea295a1623d70a5707fcb93a..836f953c3a87c57d5c622fbf4c70509050751beb 100644
>--- a/Source/JavaScriptCore/runtime/Operations.cpp
>+++ b/Source/JavaScriptCore/runtime/Operations.cpp
>@@ -52,23 +52,13 @@ NEVER_INLINE JSValue jsAddSlowCase(CallFrame* callFrame, JSValue v1, JSValue v2)
>     RETURN_IF_EXCEPTION(scope, { });
> 
>     if (p1.isString()) {
>-        if (p2.isCell()) {
>-            JSString* p2String = p2.toString(callFrame);
>-            RETURN_IF_EXCEPTION(scope, { });
>-            RELEASE_AND_RETURN(scope, jsString(callFrame, asString(p1), p2String));
>-        }
>-        String p2String = p2.toWTFString(callFrame);
>+        JSString* p2String = p2.toString(callFrame);
>         RETURN_IF_EXCEPTION(scope, { });
>         RELEASE_AND_RETURN(scope, jsString(callFrame, asString(p1), p2String));
>     }
> 
>     if (p2.isString()) {
>-        if (p2.isCell()) {
>-            JSString* p1String = p1.toString(callFrame);
>-            RETURN_IF_EXCEPTION(scope, { });
>-            RELEASE_AND_RETURN(scope, jsString(callFrame, p1String, asString(p2)));
>-        }
>-        String p1String = p1.toWTFString(callFrame);
>+        JSString* p1String = p1.toString(callFrame);
>         RETURN_IF_EXCEPTION(scope, { });
>         RELEASE_AND_RETURN(scope, jsString(callFrame, p1String, asString(p2)));
>     }
>diff --git a/Source/JavaScriptCore/runtime/Operations.h b/Source/JavaScriptCore/runtime/Operations.h
>index a34eea244564cfe26b9fbc1cdfd1fbcf6707f765..0ea98291bb68873bb3bab10461861e9158a87bd4 100644
>--- a/Source/JavaScriptCore/runtime/Operations.h
>+++ b/Source/JavaScriptCore/runtime/Operations.h
>@@ -37,50 +37,6 @@ JSValue jsTypeStringForValue(VM&, JSGlobalObject*, JSValue);
> bool jsIsObjectTypeOrNull(CallFrame*, JSValue);
> size_t normalizePrototypeChain(CallFrame*, JSCell*, bool& sawPolyProto);
> 
>-// This is a lower bound on the extra memory we expect to write if we were to allocate a rope instead of copying.
>-// Note, it doesn't differentiate between 8 and 16-bit strings because 16-bit strings are relatively rare.
>-constexpr unsigned minRopeStringLength = sizeof(JSRopeString);
>-
>-ALWAYS_INLINE JSString* jsString(ExecState* exec, const String& u1, JSString* s2)
>-{
>-    VM& vm = exec->vm();
>-    auto scope = DECLARE_THROW_SCOPE(vm);
>-
>-    unsigned length1 = u1.length();
>-    if (!length1)
>-        return s2;
>-    unsigned length2 = s2->length();
>-    if (!length2)
>-        return jsString(&vm, u1);
>-
>-    if (length1 + length2 >= minRopeStringLength)
>-        return jsString(exec, jsString(&vm, u1), s2);
>-
>-    const String& u2 = s2->value(exec);
>-    RETURN_IF_EXCEPTION(scope, { });
>-    return JSString::create(vm, makeString(u1, u2).releaseImpl().releaseNonNull());
>-}
>-
>-ALWAYS_INLINE JSString* jsString(ExecState* exec, JSString* s1, const String& u2)
>-{
>-    VM& vm = exec->vm();
>-    auto scope = DECLARE_THROW_SCOPE(vm);
>-
>-    unsigned length1 = s1->length();
>-    if (!length1)
>-        return jsString(&vm, u2);
>-    unsigned length2 = u2.length();
>-    if (!length2)
>-        return s1;
>-
>-    if (length1 + length2 >= minRopeStringLength)
>-        return jsString(exec, s1, jsString(&vm, u2));
>-
>-    const String& u1 = s1->value(exec);
>-    RETURN_IF_EXCEPTION(scope, { });
>-    return JSString::create(vm, makeString(u1, u2).releaseImpl().releaseNonNull());
>-}
>-
> ALWAYS_INLINE JSString* jsString(ExecState* exec, JSString* s1, JSString* s2)
> {
>     VM& vm = exec->vm();
>@@ -98,14 +54,7 @@ ALWAYS_INLINE JSString* jsString(ExecState* exec, JSString* s1, JSString* s2)
>         return nullptr;
>     }
> 
>-    if (length1 + length2 >= minRopeStringLength)
>-        return JSRopeString::create(vm, s1, s2);
>-
>-    const String& u1 = s1->value(exec);
>-    RETURN_IF_EXCEPTION(scope, { });
>-    const String& u2 = s2->value(exec);
>-    RETURN_IF_EXCEPTION(scope, { });
>-    return JSString::create(vm, makeString(u1, u2).releaseImpl().releaseNonNull());
>+    return JSRopeString::create(vm, s1, s2);
> }
> 
> ALWAYS_INLINE JSString* jsString(ExecState* exec, JSString* s1, JSString* s2, JSString* s3)
>@@ -130,17 +79,7 @@ ALWAYS_INLINE JSString* jsString(ExecState* exec, JSString* s1, JSString* s2, JS
>         throwOutOfMemoryError(exec, scope);
>         return nullptr;
>     }
>-
>-    if (length1 + length2 + length3 >= minRopeStringLength)
>-        return JSRopeString::create(vm, s1, s2, s3);
>-
>-    const String& u1 = s1->value(exec);
>-    RETURN_IF_EXCEPTION(scope, { });
>-    const String& u2 = s2->value(exec);
>-    RETURN_IF_EXCEPTION(scope, { });
>-    const String& u3 = s3->value(exec);
>-    RETURN_IF_EXCEPTION(scope, { });
>-    return JSString::create(vm, makeString(u1, u2, u3).releaseImpl().releaseNonNull());
>+    return JSRopeString::create(vm, s1, s2, s3);
> }
> 
> ALWAYS_INLINE JSString* jsString(ExecState* exec, const String& u1, const String& u2, const String& u3)
>@@ -170,10 +109,7 @@ ALWAYS_INLINE JSString* jsString(ExecState* exec, const String& u1, const String
>         return nullptr;
>     }
> 
>-    if (length1 + length2 + length3 >= minRopeStringLength)
>-        return JSRopeString::create(*vm, jsString(vm, u1), jsString(vm, u2), jsString(vm, u3));
>-
>-    return JSString::create(*vm, makeString(u1, u2, u3).releaseImpl().releaseNonNull());
>+    return JSRopeString::create(*vm, jsString(vm, u1), jsString(vm, u2), jsString(vm, u3));
> }
> 
> ALWAYS_INLINE JSValue jsStringFromRegisterArray(ExecState* exec, Register* strings, unsigned count)
>@@ -389,31 +325,16 @@ ALWAYS_INLINE bool jsLessEq(CallFrame* callFrame, JSValue v1, JSValue v2)
> //    13962   Add case: 5 3
> //    4000    Add case: 3 5
> 
>-
>-ALWAYS_INLINE JSValue jsAddNonNumber(CallFrame* callFrame, JSValue v1, JSValue v2)
>-{
>-    VM& vm = callFrame->vm();
>-    auto scope = DECLARE_THROW_SCOPE(vm);
>-    ASSERT(!v1.isNumber() || !v2.isNumber());
>-
>-    if (v1.isString() && !v2.isObject()) {
>-        if (v2.isString())
>-            RELEASE_AND_RETURN(scope, jsString(callFrame, asString(v1), asString(v2)));
>-        String s2 = v2.toWTFString(callFrame);
>-        RETURN_IF_EXCEPTION(scope, { });
>-        RELEASE_AND_RETURN(scope, jsString(callFrame, asString(v1), s2));
>-    }
>-
>-    // All other cases are pretty uncommon
>-    RELEASE_AND_RETURN(scope, jsAddSlowCase(callFrame, v1, v2));
>-}
>-
> ALWAYS_INLINE JSValue jsAdd(CallFrame* callFrame, JSValue v1, JSValue v2)
> {
>     if (v1.isNumber() && v2.isNumber())
>         return jsNumber(v1.asNumber() + v2.asNumber());
>         
>-    return jsAddNonNumber(callFrame, v1, v2);
>+    if (v1.isString() && !v2.isObject())
>+        return jsString(callFrame, asString(v1), v2.toString(callFrame));
>+
>+    // All other cases are pretty uncommon
>+    return jsAddSlowCase(callFrame, v1, v2);
> }
> 
> ALWAYS_INLINE JSValue jsSub(ExecState* exec, JSValue v1, JSValue v2)

Actions: View | Formatted Diff | Diff

Attachments on bug 194488: 361655