Changeset 271343 in webkit for trunk/Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp

Timestamp:

Jan 8, 2021, 8:31:12 PM (4 years ago)

Author:

Alexey Shvayka

Message:

Implement @copyDataProperties in C++ to optimize object rest / spread
​https://bugs.webkit.org/show_bug.cgi?id=193618

Reviewed by Yusuke Suzuki.

JSTests:

• microbenchmarks/object-rest-destructuring.js: Added.
• microbenchmarks/object-spread.js: Added.
• stress/object-rest-deconstruct.js:
• stress/object-spread.js:

Source/JavaScriptCore:

Since @copyDataProperties is inherently polymorphic, implementing it in JS is not beneficial.
This patch:

1. Merges almost identical @copyDataProperties variants and moves them to C++, avoiding allocations of JSArray instances and Identifier wrappers.
2. Skips non-observable Get calls, leveraging slot.isTaintedByOpaqueObject().
3. Performs DefineOwnProperty via putDirectMayBeIndex(), since the spec guarantees property creation to be successful [1]: target is an newly created object that is not yet accessible to userland code. It's impossible for target to be non-extensible nor have a non-configurable property.
4. Introduces a fast path similar to Object.assign, but: a) with no checks on target, because it's guaranteed to be an extensible JSFinalObject; b) with less checks on source, since we are performing putDirect() and don't care about

read-only properties nor proto.

Altogether, these changes result in 3.1x speed-up for object rest / spread.
Also, this patch removes unnecessary target return and @isObject check.

[1]: ​https://tc39.es/ecma262/#sec-copydataproperties (step 6.c.ii.2, note the "!" prefix)

• builtins/BuiltinNames.h:
• builtins/GlobalOperations.js:

(globalPrivate.speciesConstructor):
(globalPrivate.copyDataProperties): Deleted.
(globalPrivate.copyDataPropertiesNoExclusions): Deleted.

• bytecode/BytecodeIntrinsicRegistry.h:
• bytecode/LinkTimeConstant.h:
• bytecompiler/NodesCodegen.cpp:

(JSC::ObjectPatternNode::bindValue const):
(JSC::ObjectSpreadExpressionNode::emitBytecode):
(JSC::BytecodeIntrinsicNode::emit_intrinsic_defineEnumerableWritableConfigurableDataProperty): Deleted.

• runtime/JSGlobalObject.cpp:

(JSC::JSGlobalObject::init):

• runtime/JSGlobalObjectFunctions.cpp:

(JSC::canPerformFastPropertyEnumerationForCopyDataProperties):
(JSC::JSC_DEFINE_HOST_FUNCTION):

• runtime/JSGlobalObjectFunctions.h:

File:

• trunk/Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp

@@ -840 +840 @@
 }
 
-JSC_DEFINE_HOST_FUNCTION(globalFuncOwnKeys, (JSGlobalObject* globalObject, CallFrame* callFrame))
-{
-    VM& vm = globalObject->vm();
-    auto scope = DECLARE_THROW_SCOPE(vm);
-    JSObject* object = callFrame->argument(0).toObject(globalObject);
-    RETURN_IF_EXCEPTION(scope, encodedJSValue());
-    RELEASE_AND_RETURN(scope, JSValue::encode(ownPropertyKeys(globalObject, object, PropertyNameMode::StringsAndSymbols, DontEnumPropertiesMode::Include, WTF::nullopt)));
+static bool canPerformFastPropertyEnumerationForCopyDataProperties(Structure* structure)
+{
+    if (structure->typeInfo().overridesGetOwnPropertySlot())
+        return false;
+    if (structure->typeInfo().overridesAnyFormOfGetOwnPropertyNames())
+        return false;
+    // FIXME: Indexed properties can be handled.
+    // https://bugs.webkit.org/show_bug.cgi?id=185358
+    if (hasIndexedProperties(structure->indexingType()))
+        return false;
+    if (structure->hasGetterSetterProperties())
+        return false;
+    if (structure->hasCustomGetterSetterProperties())
+        return false;
+    if (structure->isUncacheableDictionary())
+        return false;
+    return true;
+};
+
+// https://tc39.es/ecma262/#sec-copydataproperties
+JSC_DEFINE_HOST_FUNCTION(globalFuncCopyDataProperties, (JSGlobalObject* globalObject, CallFrame* callFrame))
+{
+    VM& vm = globalObject->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
+    JSFinalObject* target = jsCast<JSFinalObject*>(callFrame->uncheckedArgument(0));
+    ASSERT(target->isStructureExtensible(vm));
+
+    JSValue sourceValue = callFrame->uncheckedArgument(1);
+    if (sourceValue.isUndefinedOrNull())
+        return JSValue::encode(jsUndefined());
+
+    JSObject* source = sourceValue.toObject(globalObject);
+    scope.assertNoException();
+
+    JSSet* excludedSet = nullptr;
+    if (callFrame->argumentCount() > 2)
+        excludedSet = jsCast<JSSet*>(callFrame->uncheckedArgument(2));
+
+    auto isPropertyNameExcluded = [&] (JSGlobalObject* globalObject, PropertyName propertyName) -> bool {
+        ASSERT(!propertyName.isPrivateName());
+        if (!excludedSet)
+            return false;
+
+        JSValue propertyNameValue = identifierToJSValue(vm, Identifier::fromUid(vm, propertyName.uid()));
+        RETURN_IF_EXCEPTION(scope, false);
+        return excludedSet->has(globalObject, propertyNameValue);
+    };
+
+    if (!source->staticPropertiesReified(vm)) {
+        source->reifyAllStaticProperties(globalObject);
+        RETURN_IF_EXCEPTION(scope, { });
+    }
+
+    if (canPerformFastPropertyEnumerationForCopyDataProperties(source->structure(vm))) {
+        Vector<RefPtr<UniquedStringImpl>, 8> properties;
+        MarkedArgumentBuffer values;
+
+        // FIXME: It doesn't seem like we should have to do this in two phases, but
+        // we're running into crashes where it appears that source is transitioning
+        // under us, and even ends up in a state where it has a null butterfly. My
+        // leading hypothesis here is that we fire some value replacement watchpoint
+        // that ends up transitioning the structure underneath us.
+        // https://bugs.webkit.org/show_bug.cgi?id=187837
+
+        source->structure(vm)->forEachProperty(vm, [&] (const PropertyMapEntry& entry) -> bool {
+            PropertyName propertyName(entry.key);
+            if (propertyName.isPrivateName())
+                return true;
+
+            bool excluded = isPropertyNameExcluded(globalObject, propertyName);
+            RETURN_IF_EXCEPTION(scope, false);
+            if (excluded)
+                return true;
+            if (entry.attributes & PropertyAttribute::DontEnum)
+                return true;
+
+            properties.append(entry.key);
+            values.appendWithCrashOnOverflow(source->getDirect(entry.offset));
+            return true;
+        });
+
+        RETURN_IF_EXCEPTION(scope, { });
+
+        for (size_t i = 0; i < properties.size(); ++i) {
+            // FIXME: We could put properties in a batching manner to accelerate CopyDataProperties more.
+            // https://bugs.webkit.org/show_bug.cgi?id=185358
+            target->putDirect(vm, properties[i].get(), values.at(i));
+        }
+    } else {
+        PropertyNameArray propertyNames(vm, PropertyNameMode::StringsAndSymbols, PrivateSymbolMode::Exclude);
+        source->methodTable(vm)->getOwnPropertyNames(source, globalObject, propertyNames, DontEnumPropertiesMode::Include);
+        RETURN_IF_EXCEPTION(scope, { });
+
+        for (const auto& propertyName : propertyNames) {
+            bool excluded = isPropertyNameExcluded(globalObject, propertyName);
+            RETURN_IF_EXCEPTION(scope, false);
+            if (excluded)
+                continue;
+
+            PropertySlot slot(source, PropertySlot::InternalMethodType::GetOwnProperty);
+            bool hasProperty = source->methodTable(vm)->getOwnPropertySlot(source, globalObject, propertyName, slot);
+            RETURN_IF_EXCEPTION(scope, { });
+            if (!hasProperty)
+                continue;
+            if (slot.attributes() & PropertyAttribute::DontEnum)
+                continue;
+
+            JSValue value;
+            if (LIKELY(!slot.isTaintedByOpaqueObject()))
+                value = slot.getValue(globalObject, propertyName);
+            else
+                value = source->get(globalObject, propertyName);
+            RETURN_IF_EXCEPTION(scope, { });
+
+            target->putDirectMayBeIndex(globalObject, propertyName, value);
+        }
+    }
+
+    return JSValue::encode(jsUndefined());
 }