[JSC] A bit performance improvement for Object.assign by cleaning up code
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 20 Jul 2018 18:28:28 +0000 (18:28 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 20 Jul 2018 18:28:28 +0000 (18:28 +0000)
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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@234058 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/ObjectConstructor.cpp

index 2eab1fe..63f76a2 100644 (file)
@@ -1,3 +1,24 @@
+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>
 
         [GLIB] jsc_context_evaluate_in_object() should receive an instance when a JSCClass is given
index 27471ee..b3a2964 100644 (file)
@@ -304,6 +304,8 @@ EncodedJSValue JSC_HOST_CALL objectConstructorAssign(ExecState* exec)
     // https://bugs.webkit.org/show_bug.cgi?id=185358
     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) {
         JSValue sourceValue = exec->uncheckedArgument(i);
@@ -318,66 +320,59 @@ EncodedJSValue JSC_HOST_CALL objectConstructorAssign(ExecState* exec)
                 RETURN_IF_EXCEPTION(scope, { });
             }
 
-            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;
-                    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));
+            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;
-                    });
-                }
-            }
 
-            if (canDoFastPath) {
+                    properties.append(entry.key);
+                    values.appendWithCrashOnOverflow(source->getDirect(entry.offset));
+
+                    return true;
+                });
+
                 for (size_t i = 0; i < properties.size(); ++i) {
                     // FIXME: We could put properties in a batching manner to accelerate Object.assign more.
                     // https://bugs.webkit.org/show_bug.cgi?id=185358
                     PutPropertySlot putPropertySlot(target, true);
                     target->putOwnDataProperty(vm, properties[i].get(), values.at(i), putPropertySlot);
                 }
-
                 continue;
             }
         }