Changeset 234058 in webkit

Timestamp:

Jul 20, 2018 11:28:28 AM (6 years ago)

Author:

Yusuke Suzuki

Message:

[JSC] A bit performance improvement for Object.assign by cleaning up code
​https://bugs.webkit.org/show_bug.cgi?id=187852

Reviewed by Saam Barati.

We clean up Object.assign code a bit.

1. Vector and MarkedArgumentBuffer are extracted out from the loop since repeatedly creating MarkedArgumentBuffer is costly.
2. canDoFastPath is not necessary. Restructuring the code to clean up things.

It improves the performance a bit.

baseline patched

object-assign.es6 237.7719+-5.5175 231.2856+-4.6907 might be 1.0280x faster

• runtime/ObjectConstructor.cpp:

(JSC::objectConstructorAssign):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• runtime/ObjectConstructor.cpp (modified) (3 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2018-07-20  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] A bit performance improvement for Object.assign by cleaning up code
+        https://bugs.webkit.org/show_bug.cgi?id=187852
+
+        Reviewed by Saam Barati.
+
+        We clean up Object.assign code a bit.
+
+        1. Vector and MarkedArgumentBuffer are extracted out from the loop since repeatedly creating MarkedArgumentBuffer is costly.
+        2. canDoFastPath is not necessary. Restructuring the code to clean up things.
+
+        It improves the performance a bit.
+
+                                    baseline                  patched
+
+        object-assign.es6      237.7719+-5.5175          231.2856+-4.6907          might be 1.0280x faster
+
+        * runtime/ObjectConstructor.cpp:
+        (JSC::objectConstructorAssign):
+
 2018-07-19  Carlos Garcia Campos  <cgarcia@igalia.com>
 

trunk/Source/JavaScriptCore/runtime/ObjectConstructor.cpp

@@ -305 +305 @@
     bool targetCanPerformFastPut = jsDynamicCast<JSFinalObject*>(vm, target) && target->canPerformFastPutInlineExcludingProto(vm);
 
+    Vector<RefPtr<UniquedStringImpl>, 8> properties;
+    MarkedArgumentBuffer values;
     unsigned argsCount = exec->argumentCount();
     for (unsigned i = 1; i < argsCount; ++i) {
@@ -319 +321 @@
             }
 
-            bool canDoFastPath;
-            Vector<RefPtr<UniquedStringImpl>, 8> properties;
-            MarkedArgumentBuffer values;
-            {
-                auto canPerformFastPropertyEnumerationForObjectAssign = [] (Structure* structure) {
-                    if (structure->typeInfo().overridesGetOwnPropertySlot())
-                        return false;
-                    if (structure->typeInfo().overridesGetPropertyNames())
-                        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->isUncacheableDictionary())
-                        return false;
-                    // Cannot perform fast [[Put]] to |target| if the property names of the |source| contain "__proto__".
-                    if (structure->hasUnderscoreProtoPropertyExcludingOriginalProto())
-                        return false;
+            auto canPerformFastPropertyEnumerationForObjectAssign = [] (Structure* structure) {
+                if (structure->typeInfo().overridesGetOwnPropertySlot())
+                    return false;
+                if (structure->typeInfo().overridesGetPropertyNames())
+                    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->isUncacheableDictionary())
+                    return false;
+                // Cannot perform fast [[Put]] to |target| if the property names of the |source| contain "__proto__".
+                if (structure->hasUnderscoreProtoPropertyExcludingOriginalProto())
+                    return false;
+                return true;
+            };
+
+            if (canPerformFastPropertyEnumerationForObjectAssign(source->structure(vm))) {
+                // |source| Structure does not have any getters. And target can perform fast put.
+                // So enumerating properties and putting properties are non observable.
+
+                // 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
+
+                // Do not clear since Vector::clear shrinks the backing store.
+                properties.resize(0);
+                values.clear();
+                source->structure(vm)->forEachProperty(vm, [&] (const PropertyMapEntry& entry) -> bool {
+                    if (entry.attributes & PropertyAttribute::DontEnum)
+                        return true;
+
+                    PropertyName propertyName(entry.key);
+                    if (propertyName.isPrivateName())
+                        return true;
+
+                    properties.append(entry.key);
+                    values.appendWithCrashOnOverflow(source->getDirect(entry.offset));
+
                     return true;
-                };
-
-                Structure* structure = source->structure(vm);
-                canDoFastPath = canPerformFastPropertyEnumerationForObjectAssign(structure);
-                if (canDoFastPath) {
-                    // |source| Structure does not have any getters. And target can perform fast put.
-                    // So enumerating properties and putting properties are non observable.
-
-                    // 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
-
-                    structure->forEachProperty(vm, [&] (const PropertyMapEntry& entry) -> bool {
-                        if (entry.attributes & PropertyAttribute::DontEnum)
-                            return true;
-
-                        PropertyName propertyName(entry.key);
-                        if (propertyName.isPrivateName())
-                            return true;
-                        
-                        properties.append(entry.key);
-                        values.appendWithCrashOnOverflow(source->getDirect(entry.offset));
-
-                        return true;
-                    });
-                }
-            }
-
-            if (canDoFastPath) {
+                });
+
                 for (size_t i = 0; i < properties.size(); ++i) {
                     // FIXME: We could put properties in a batching manner to accelerate Object.assign more.
@@ -378 +374 @@
                     target->putOwnDataProperty(vm, properties[i].get(), values.at(i), putPropertySlot);
                 }
-
                 continue;
             }