[JSC] Object.assign for final objects should be faster
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 May 2018 03:38:10 +0000 (03:38 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 May 2018 03:38:10 +0000 (03:38 +0000)
https://bugs.webkit.org/show_bug.cgi?id=185348

Reviewed by Saam Barati.

JSTests:

* stress/object-assign-fast-path.js: Added.
(shouldBe):
(checkProperty):

Source/JavaScriptCore:

Object.assign is so heavily used to clone an object. For example, speedometer react-redux can be significantly
improved if Object.assign becomes fast. It is worth adding a complex fast path to accelerate the major use cases.

If enumerating properties of source objects and putting properties to target object are non observable,
we can avoid hash table looking up of source object properties. We can enumerate object property entries,
and put them to target object. This patch adds this fast path to Object.assign implementation.

When enumerating properties, we need to ensure that the given |source| object does not include "__proto__"
property since we cannot perform fast [[Put]] for the |target| object. We add a new flag
"HasUnderscoreProtoPropertyExcludingOriginalProto" to Structure to track this state.

This improves object-assign.es6 by 1.85x.

                                baseline                  patched

    object-assign.es6      368.6132+-8.3508     ^    198.8775+-4.9042        ^ definitely 1.8535x faster

And Speedometer2.0 React-Redux-TodoMVC's total time is improved from 490ms to 431ms.

* runtime/JSObject.h:
* runtime/JSObjectInlines.h:
(JSC::JSObject::canPerformFastPutInlineExcludingProto):
(JSC::JSObject::canPerformFastPutInline):
* runtime/ObjectConstructor.cpp:
(JSC::objectConstructorAssign):
* runtime/Structure.cpp:
(JSC::Structure::Structure):
* runtime/Structure.h:
* runtime/StructureInlines.h:
(JSC::Structure::forEachProperty):
(JSC::Structure::add):

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

JSTests/ChangeLog
JSTests/stress/object-assign-fast-path.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSObject.h
Source/JavaScriptCore/runtime/JSObjectInlines.h
Source/JavaScriptCore/runtime/ObjectConstructor.cpp
Source/JavaScriptCore/runtime/Structure.cpp
Source/JavaScriptCore/runtime/Structure.h
Source/JavaScriptCore/runtime/StructureInlines.h

index 3311588..587ccd3 100644 (file)
@@ -1,3 +1,14 @@
+2018-05-09  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Object.assign for final objects should be faster
+        https://bugs.webkit.org/show_bug.cgi?id=185348
+
+        Reviewed by Saam Barati.
+
+        * stress/object-assign-fast-path.js: Added.
+        (shouldBe):
+        (checkProperty):
+
 2018-05-10  Leo Balter  <leonardo.balter@gmail.com>
 
         Update Test262 tests through the new import script - 20180509
diff --git a/JSTests/stress/object-assign-fast-path.js b/JSTests/stress/object-assign-fast-path.js
new file mode 100644 (file)
index 0000000..5c6d547
--- /dev/null
@@ -0,0 +1,161 @@
+var createBuiltin = $vm.createBuiltin;
+
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error(`bad value: ${String(actual)}`);
+}
+
+function shouldThrow(func, errorMessage) {
+    var errorThrown = false;
+    var error = null;
+    try {
+        func();
+    } catch (e) {
+        errorThrown = true;
+        error = e;
+    }
+    if (!errorThrown)
+        throw new Error('not thrown');
+    if (String(error) !== errorMessage)
+        throw new Error(`bad error: ${String(error)}`);
+}
+
+function checkProperty(object, name, value, attributes = { writable: true, enumerable: true, configurable: true })
+{
+    var desc = Object.getOwnPropertyDescriptor(object, name);
+    shouldBe(!!desc, true);
+    shouldBe(desc.writable, attributes.writable);
+    shouldBe(desc.enumerable, attributes.enumerable);
+    shouldBe(desc.configurable, attributes.configurable);
+    shouldBe(desc.value, value);
+}
+
+{
+    let result = Object.assign({}, RegExp);
+    shouldBe(JSON.stringify(Object.getOwnPropertyNames(result).sort()), `["$1","$2","$3","$4","$5","$6","$7","$8","$9","input","lastMatch","lastParen","leftContext","multiline","rightContext"]`);
+}
+{
+    function Hello() { }
+    let result = Object.assign(Hello, {
+        ok: 42
+    });
+
+    shouldBe(JSON.stringify(Object.getOwnPropertyNames(result).sort()), `["arguments","caller","length","name","ok","prototype"]`);
+    checkProperty(result, "ok", 42);
+}
+{
+    let result = Object.assign({ ok: 42 }, { 0: 0, 1: 1 });
+    shouldBe(JSON.stringify(Object.getOwnPropertyNames(result).sort()), `["0","1","ok"]`);
+    checkProperty(result, "ok", 42);
+    checkProperty(result, "0", 0);
+    checkProperty(result, "1", 1);
+}
+{
+    let object = { 0: 0, 1: 1 };
+    ensureArrayStorage(object);
+    let result = Object.assign({ ok: 42 }, object);
+    shouldBe(JSON.stringify(Object.getOwnPropertyNames(result).sort()), `["0","1","ok"]`);
+    checkProperty(result, "ok", 42);
+    checkProperty(result, "0", 0);
+    checkProperty(result, "1", 1);
+}
+{
+    let called = false;
+    let result = Object.assign({}, {
+        get hello() {
+            called = true;
+            return 42;
+        }
+    });
+    shouldBe(JSON.stringify(Object.getOwnPropertyNames(result).sort()), `["hello"]`);
+    shouldBe(called, true);
+    checkProperty(result, "hello", 42);
+}
+{
+    let object = {};
+    Object.defineProperty(object, "__proto__", {
+        value: 42,
+        enumerable: true,
+        writable: true,
+        configurable: true
+    });
+    checkProperty(object, "__proto__", 42);
+    shouldBe(JSON.stringify(Object.getOwnPropertyNames(object).sort()), `["__proto__"]`);
+    let result = Object.assign({}, object);
+    shouldBe(JSON.stringify(Object.getOwnPropertyNames(result).sort()), `[]`);
+    shouldBe(Object.getOwnPropertyDescriptor(result, "__proto__"), undefined);
+    shouldBe(result.__proto__, Object.prototype);
+}
+{
+    let object = {};
+    Object.defineProperty(object, "hello", {
+        value: 42,
+        writable: false,
+        enumerable: true,
+        configurable: false
+    });
+    checkProperty(object, "hello", 42, { writable: false, enumerable: true, configurable: false });
+    shouldBe(JSON.stringify(Object.getOwnPropertyNames(object).sort()), `["hello"]`);
+    shouldThrow(() => {
+        Object.assign(object, { hello: 50 });
+    }, `TypeError: Attempted to assign to readonly property.`);
+}
+{
+    let counter = 0;
+    let helloCalled = null;
+    let okCalled = null;
+    let source = {};
+    source.hello = 42;
+    source.ok = 52;
+    checkProperty(source, "hello", 42);
+    checkProperty(source, "ok", 52);
+    shouldBe(JSON.stringify(Object.getOwnPropertyNames(source)), `["hello","ok"]`);
+
+    let result = Object.assign({
+        set hello(value) {
+            this.__hello = value;
+            helloCalled = counter++;
+        },
+        set ok(value) {
+            this.__ok = value;
+            okCalled = counter++;
+        }
+    }, source);
+    checkProperty(result, "__hello", 42);
+    checkProperty(result, "__ok", 52);
+    shouldBe(JSON.stringify(Object.getOwnPropertyNames(result).sort()), `["__hello","__ok","hello","ok"]`);
+    shouldBe(helloCalled, 0);
+    shouldBe(okCalled, 1);
+}
+{
+    let builtin = createBuiltin(`(function (obj) {
+        return @getByIdDirectPrivate(obj, "generatorState");
+    })`);
+    function* hello() { }
+    let generator = hello();
+    shouldBe(typeof builtin(generator), "number");
+    let result = Object.assign({}, generator);
+    shouldBe(typeof builtin(result), "undefined");
+}
+{
+    let object = {};
+    let setterCalledWithValue = null;
+    let result = Object.assign(object, {
+        get hello() {
+            Object.defineProperty(object, "added", {
+                get() {
+                    return 42;
+                },
+                set(value) {
+                    setterCalledWithValue = value;
+                }
+            });
+            return 0;
+        }
+    }, {
+        added: "world"
+    });
+    shouldBe(result.added, 42);
+    shouldBe(result.hello, 0);
+    shouldBe(setterCalledWithValue, "world");
+}
index 270d952..3b091ed 100644 (file)
@@ -1,3 +1,42 @@
+2018-05-09  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Object.assign for final objects should be faster
+        https://bugs.webkit.org/show_bug.cgi?id=185348
+
+        Reviewed by Saam Barati.
+
+        Object.assign is so heavily used to clone an object. For example, speedometer react-redux can be significantly
+        improved if Object.assign becomes fast. It is worth adding a complex fast path to accelerate the major use cases.
+
+        If enumerating properties of source objects and putting properties to target object are non observable,
+        we can avoid hash table looking up of source object properties. We can enumerate object property entries,
+        and put them to target object. This patch adds this fast path to Object.assign implementation.
+
+        When enumerating properties, we need to ensure that the given |source| object does not include "__proto__"
+        property since we cannot perform fast [[Put]] for the |target| object. We add a new flag
+        "HasUnderscoreProtoPropertyExcludingOriginalProto" to Structure to track this state.
+
+        This improves object-assign.es6 by 1.85x.
+
+                                        baseline                  patched
+
+            object-assign.es6      368.6132+-8.3508     ^    198.8775+-4.9042        ^ definitely 1.8535x faster
+
+        And Speedometer2.0 React-Redux-TodoMVC's total time is improved from 490ms to 431ms.
+
+        * runtime/JSObject.h:
+        * runtime/JSObjectInlines.h:
+        (JSC::JSObject::canPerformFastPutInlineExcludingProto):
+        (JSC::JSObject::canPerformFastPutInline):
+        * runtime/ObjectConstructor.cpp:
+        (JSC::objectConstructorAssign):
+        * runtime/Structure.cpp:
+        (JSC::Structure::Structure):
+        * runtime/Structure.h:
+        * runtime/StructureInlines.h:
+        (JSC::Structure::forEachProperty):
+        (JSC::Structure::add):
+
 2018-05-10  Filip Pizlo  <fpizlo@apple.com>
 
         DFG CFA should pick the right time to inject OSR entry data
index c2eb8e6..e4e9b32 100644 (file)
@@ -872,6 +872,9 @@ public:
 
     JS_EXPORT_PRIVATE JSValue getMethod(ExecState*, CallData&, CallType&, const Identifier&, const String& errorMessage);
 
+    bool canPerformFastPutInline(VM&, PropertyName);
+    bool canPerformFastPutInlineExcludingProto(VM&);
+
     DECLARE_EXPORT_INFO;
 
 protected:
@@ -1017,7 +1020,6 @@ private:
         
     template<PutMode>
     bool putDirectInternal(VM&, PropertyName, JSValue, unsigned attr, PutPropertySlot&);
-    bool canPerformFastPutInline(VM&, PropertyName);
 
     JS_EXPORT_PRIVATE NEVER_INLINE bool putInlineSlow(ExecState*, PropertyName, JSValue, PutPropertySlot&);
 
index b3f2786..9fc266c 100644 (file)
@@ -60,11 +60,8 @@ void createListFromArrayLike(ExecState* exec, JSValue arrayLikeValue, RuntimeTyp
     }
 }
 
-ALWAYS_INLINE bool JSObject::canPerformFastPutInline(VM& vm, PropertyName propertyName)
+ALWAYS_INLINE bool JSObject::canPerformFastPutInlineExcludingProto(VM& vm)
 {
-    if (UNLIKELY(propertyName == vm.propertyNames->underscoreProto))
-        return false;
-
     // Check if there are any setters or getters in the prototype chain
     JSValue prototype;
     JSObject* obj = this;
@@ -83,6 +80,13 @@ ALWAYS_INLINE bool JSObject::canPerformFastPutInline(VM& vm, PropertyName proper
     ASSERT_NOT_REACHED();
 }
 
+ALWAYS_INLINE bool JSObject::canPerformFastPutInline(VM& vm, PropertyName propertyName)
+{
+    if (UNLIKELY(propertyName == vm.propertyNames->underscoreProto))
+        return false;
+    return canPerformFastPutInlineExcludingProto(vm);
+}
+
 template<typename CallbackWhenNoException>
 ALWAYS_INLINE typename std::result_of<CallbackWhenNoException(bool, PropertySlot&)>::type JSObject::getPropertySlot(ExecState* exec, PropertyName propertyName, CallbackWhenNoException callback) const
 {
index e18c765..99210e4 100644 (file)
@@ -300,6 +300,10 @@ EncodedJSValue JSC_HOST_CALL objectConstructorAssign(ExecState* exec)
     JSObject* target = targetValue.toObject(exec);
     RETURN_IF_EXCEPTION(scope, { });
 
+    // FIXME: Extend this for non JSFinalObject. For example, we would like to use this fast path for function objects too.
+    // https://bugs.webkit.org/show_bug.cgi?id=185358
+    bool targetCanPerformFastPut = jsDynamicCast<JSFinalObject*>(vm, target) && target->canPerformFastPutInlineExcludingProto(vm);
+
     unsigned argsCount = exec->argumentCount();
     for (unsigned i = 1; i < argsCount; ++i) {
         JSValue sourceValue = exec->uncheckedArgument(i);
@@ -308,6 +312,57 @@ EncodedJSValue JSC_HOST_CALL objectConstructorAssign(ExecState* exec)
         JSObject* source = sourceValue.toObject(exec);
         RETURN_IF_EXCEPTION(scope, { });
 
+        if (targetCanPerformFastPut) {
+            if (!source->staticPropertiesReified()) {
+                source->reifyAllStaticProperties(exec);
+                RETURN_IF_EXCEPTION(scope, { });
+            }
+
+            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);
+            if (canPerformFastPropertyEnumerationForObjectAssign(structure)) {
+                // |source| Structure does not have any getters. And target can perform fast put.
+                // So enumerating properties and putting properties are non observable.
+                structure->forEachProperty(vm, [&] (const PropertyMapEntry& entry) -> bool {
+                    if (entry.attributes & PropertyAttribute::DontEnum)
+                        return true;
+
+                    PropertyName propertyName(entry.key);
+                    if (propertyName.isPrivateName())
+                        return true;
+
+                    // 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, propertyName, source->getDirect(entry.offset), putPropertySlot);
+                    return true;
+                });
+                continue;
+            }
+        }
+
+        // [[GetOwnPropertyNames]], [[Get]] etc. could modify target object and invalidate this assumption.
+        // For example, [[Get]] of source object could configure setter to target object. So disable the fast path.
+        targetCanPerformFastPut = false;
+
         PropertyNameArray properties(&vm, PropertyNameMode::StringsAndSymbols, PrivateSymbolMode::Exclude);
         source->methodTable(vm)->getOwnPropertyNames(source, exec, properties, EnumerationMode(DontEnumPropertiesMode::Include));
         RETURN_IF_EXCEPTION(scope, { });
index 3d6d24d..c490934 100644 (file)
@@ -193,6 +193,7 @@ Structure::Structure(VM& vm, JSGlobalObject* globalObject, JSValue prototype, co
     setHasGetterSetterProperties(classInfo->hasStaticSetterOrReadonlyProperties());
     setHasCustomGetterSetterProperties(false);
     setHasReadOnlyOrGetterSetterPropertiesExcludingProto(classInfo->hasStaticSetterOrReadonlyProperties());
+    setHasUnderscoreProtoPropertyExcludingOriginalProto(false);
     setIsQuickPropertyAccessAllowedForEnumeration(true);
     setAttributesInPrevious(0);
     setDidPreventExtensions(false);
@@ -226,6 +227,7 @@ Structure::Structure(VM& vm)
     setHasGetterSetterProperties(m_classInfo->hasStaticSetterOrReadonlyProperties());
     setHasCustomGetterSetterProperties(false);
     setHasReadOnlyOrGetterSetterPropertiesExcludingProto(m_classInfo->hasStaticSetterOrReadonlyProperties());
+    setHasUnderscoreProtoPropertyExcludingOriginalProto(false);
     setIsQuickPropertyAccessAllowedForEnumeration(true);
     setAttributesInPrevious(0);
     setDidPreventExtensions(false);
@@ -259,6 +261,7 @@ Structure::Structure(VM& vm, Structure* previous, DeferredStructureTransitionWat
     setHasGetterSetterProperties(previous->hasGetterSetterProperties());
     setHasCustomGetterSetterProperties(previous->hasCustomGetterSetterProperties());
     setHasReadOnlyOrGetterSetterPropertiesExcludingProto(previous->hasReadOnlyOrGetterSetterPropertiesExcludingProto());
+    setHasUnderscoreProtoPropertyExcludingOriginalProto(previous->hasUnderscoreProtoPropertyExcludingOriginalProto());
     setIsQuickPropertyAccessAllowedForEnumeration(previous->isQuickPropertyAccessAllowedForEnumeration());
     setAttributesInPrevious(0);
     setDidPreventExtensions(previous->didPreventExtensions());
index 0aaa139..4549c0e 100644 (file)
@@ -432,6 +432,9 @@ public:
     // to continue or false if it's done.
     template<typename Functor>
     void forEachPropertyConcurrently(const Functor&);
+
+    template<typename Functor>
+    void forEachProperty(VM&, const Functor&);
     
     PropertyOffset getConcurrently(UniquedStringImpl* uid);
     PropertyOffset getConcurrently(UniquedStringImpl* uid, unsigned& attributes);
@@ -679,6 +682,7 @@ public:
     DEFINE_BITFIELD(bool, transitionWatchpointIsLikelyToBeFired, TransitionWatchpointIsLikelyToBeFired, 1, 26);
     DEFINE_BITFIELD(bool, hasBeenDictionary, HasBeenDictionary, 1, 27);
     DEFINE_BITFIELD(bool, isAddingPropertyForTransition, IsAddingPropertyForTransition, 1, 28);
+    DEFINE_BITFIELD(bool, hasUnderscoreProtoPropertyExcludingOriginalProto, HasUnderscoreProtoPropertyExcludingOriginalProto, 1, 29);
 
 private:
     friend class LLIntOffsetsExtractor;
index 9047f22..8cf28a8 100644 (file)
@@ -163,6 +163,17 @@ void Structure::forEachPropertyConcurrently(const Functor& functor)
     }
 }
 
+template<typename Functor>
+void Structure::forEachProperty(VM& vm, const Functor& functor)
+{
+    if (PropertyTable* table = ensurePropertyTableIfNotEmpty(vm)) {
+        for (auto& entry : *table) {
+            if (!functor(entry))
+                return;
+        }
+    }
+}
+
 inline PropertyOffset Structure::getConcurrently(UniquedStringImpl* uid)
 {
     unsigned attributesIgnored;
@@ -376,6 +387,8 @@ inline PropertyOffset Structure::add(VM& vm, PropertyName propertyName, unsigned
     checkConsistency();
     if (attributes & PropertyAttribute::DontEnum || propertyName.isSymbol())
         setIsQuickPropertyAccessAllowedForEnumeration(false);
+    if (propertyName == vm.propertyNames->underscoreProto)
+        setHasUnderscoreProtoPropertyExcludingOriginalProto(true);
 
     auto rep = propertyName.uid();