Changeset 266215 in webkit for trunk/Source/JavaScriptCore

Timestamp:

Aug 26, 2020, 6:55:08 PM (5 years ago)

Author:

Alexey Shvayka

Message:

Merge putLength() into setLength()
​https://bugs.webkit.org/show_bug.cgi?id=211279

Reviewed by Darin Adler and Saam Barati.

JSTests:

• microbenchmarks/array-shift-unshift-empty.js: Added.
• stress/array-setLength-on-proxy-error.js: Added.

Source/JavaScriptCore:

This patch:

1. Replaces all putLength() call sites with setLength(), saving two JSValue instantiations in arrayProtoFuncPop() and two in arrayProtoFuncShift().

2. Merges putLength() into setLength(), removing superfluous put() call for JSArray. Also, performs put() in strict mode to preserve the original error messages, like ones in ProxyObject::performPut().

3. Inlines performPop(), which avoided an extra index check and Identifier creation, as it was on the slow path anyway (note JSArray::pop() call).

This change advances provided setLength()-heavy microbenchmark by ~40%,
while existing Array tests are neutral.

• runtime/ArrayPrototype.cpp:

(JSC::setLength):
(JSC::arrayProtoFuncPop):
(JSC::arrayProtoFuncPush):
(JSC::arrayProtoFuncShift):
(JSC::arrayProtoFuncUnShift):
(JSC::putLength): Deleted.

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• runtime/ArrayPrototype.cpp (modified) (6 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2020-08-26  Alexey Shvayka  <[email protected]>
+
+        Merge putLength() into setLength()
+        https://bugs.webkit.org/show_bug.cgi?id=211279
+
+        Reviewed by Darin Adler and Saam Barati.
+
+        This patch:
+
+        1. Replaces all putLength() call sites with setLength(), saving two JSValue
+           instantiations in arrayProtoFuncPop() and two in arrayProtoFuncShift().
+
+        2. Merges putLength() into setLength(), removing superfluous put() call for
+           JSArray. Also, performs put() in strict mode to preserve the original
+           error messages, like ones in ProxyObject::performPut().
+
+        3. Inlines performPop(), which avoided an extra index check and Identifier
+           creation, as it was on the slow path anyway (note JSArray::pop() call).
+
+        This change advances provided setLength()-heavy microbenchmark by ~40%,
+        while existing Array tests are neutral.
+
+        * runtime/ArrayPrototype.cpp:
+        (JSC::setLength):
+        (JSC::arrayProtoFuncPop):
+        (JSC::arrayProtoFuncPush):
+        (JSC::arrayProtoFuncShift):
+        (JSC::arrayProtoFuncUnShift):
+        (JSC::putLength): Deleted.
+
 2020-08-26  Saam Barati  <[email protected]>
 

trunk/Source/JavaScriptCore/runtime/ArrayPrototype.cpp

@@ -161 +161 @@
 }
 
-static ALWAYS_INLINE void putLength(JSGlobalObject* globalObject, VM& vm, JSObject* obj, JSValue value)
-{
-    auto scope = DECLARE_THROW_SCOPE(vm);
-    PutPropertySlot slot(obj);
-    bool success = obj->methodTable(vm)->put(obj, globalObject, vm.propertyNames->length, value, slot);
-    RETURN_IF_EXCEPTION(scope, void());
-    if (UNLIKELY(!success))
-        throwTypeError(globalObject, scope, ReadonlyPropertyWriteError);
-}
-
 static ALWAYS_INLINE void setLength(JSGlobalObject* globalObject, VM& vm, JSObject* obj, uint64_t value)
 {
-    auto scope = DECLARE_THROW_SCOPE(vm);
     static constexpr bool throwException = true;
-    if (isJSArray(obj)) {
+    if (LIKELY(isJSArray(obj))) {
         ASSERT(static_cast<uint32_t>(value) == value);
         jsCast<JSArray*>(obj)->setLength(globalObject, static_cast<uint32_t>(value), throwException);
-        RETURN_IF_EXCEPTION(scope, void());
-    }
-    scope.release();
-    putLength(globalObject, vm, obj, jsNumber(value));
+    } else {
+        PutPropertySlot slot(obj, throwException);
+        obj->methodTable(vm)->put(obj, globalObject, vm.propertyNames->length, jsNumber(value), slot);
+    }
 }
 
@@ -877 +866 @@
     if (length == 0) {
         scope.release();
-        putLength(globalObject, vm, thisObj, jsNumber(length));
+        setLength(globalObject, vm, thisObj, length);
         return JSValue::encode(jsUndefined());
     }
 
-    auto performPop = [&] (auto index, JSValue indexValue) {
-        JSValue result = thisObj->get(globalObject, index);
-        RETURN_IF_EXCEPTION(scope, encodedJSValue());
-        bool success = thisObj->deleteProperty(globalObject, index);
-        RETURN_IF_EXCEPTION(scope, encodedJSValue());
-        if (UNLIKELY(!success)) {
-            throwTypeError(globalObject, scope, UnableToDeletePropertyError);
-            return encodedJSValue();
-        }
-        scope.release();
-        putLength(globalObject, vm, thisObj, indexValue);
-        return JSValue::encode(result);
-    };
-
     static_assert(MAX_ARRAY_INDEX + 1 > MAX_ARRAY_INDEX);
-    if (LIKELY(length <= MAX_ARRAY_INDEX + 1))
-        return performPop(static_cast<uint32_t>(length - 1), jsNumber(static_cast<uint32_t>(length - 1)));
-    return performPop(Identifier::from(vm, length - 1), jsNumber(length - 1));
+    uint64_t index = length - 1;
+    JSValue result = thisObj->get(globalObject, index);
+    RETURN_IF_EXCEPTION(scope, { });
+    bool success = thisObj->deleteProperty(globalObject, index);
+    RETURN_IF_EXCEPTION(scope, { });
+    if (UNLIKELY(!success)) {
+        throwTypeError(globalObject, scope, UnableToDeletePropertyError);
+        return { };
+    }
+
+    scope.release();
+    setLength(globalObject, vm, thisObj, index);
+    return JSValue::encode(result);
 }
 
@@ -936 +921 @@
     }
     
-    JSValue newLength(static_cast<int64_t>(length) + static_cast<int64_t>(callFrame->argumentCount()));
+    uint64_t newLength = length + argCount;
     scope.release();
-    putLength(globalObject, vm, thisObj, newLength);
-    return JSValue::encode(newLength);
+    setLength(globalObject, vm, thisObj, newLength);
+    return JSValue::encode(jsNumber(newLength));
 }
 
@@ -1058 +1043 @@
     if (length == 0) {
         scope.release();
-        putLength(globalObject, vm, thisObj, jsNumber(length));
+        setLength(globalObject, vm, thisObj, length);
         return JSValue::encode(jsUndefined());
     }
@@ -1067 +1052 @@
     RETURN_IF_EXCEPTION(scope, encodedJSValue());
     scope.release();
-    putLength(globalObject, vm, thisObj, jsNumber(length - 1));
+    setLength(globalObject, vm, thisObj, length - 1);
     return JSValue::encode(result);
 }
@@ -1264 +1249 @@
         RETURN_IF_EXCEPTION(scope, encodedJSValue());
     }
-    JSValue result = jsNumber(length + nrArgs);
+    uint64_t newLength = length + nrArgs;
     scope.release();
-    putLength(globalObject, vm, thisObj, result);
-    return JSValue::encode(result);
+    setLength(globalObject, vm, thisObj, newLength);
+    return JSValue::encode(jsNumber(newLength));
 }