[JSC] Implement Object.assign in C++
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 15 Jun 2017 19:37:21 +0000 (19:37 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 15 Jun 2017 19:37:21 +0000 (19:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=173414

Reviewed by Saam Barati.

JSTests:

* stress/object-assign-string-first.js: Added.
(shouldBe):
(source.get Symbol):
(source.get 1):
(source.get cocoa):

Source/JavaScriptCore:

Implementing Object.assign in JS is not so good compared to C++ version because,

1. JS version allocates JS array for object own keys. And we allocate JSString / Symbol for each key.
But basically, they can be handled as UniquedStringImpl in C++. Allocating these cells are wasteful.

2. While implementing builtins in JS offers some good type speculation chances, Object.assign is inherently super polymorphic.
So JS's type profile doesn't help well.

3. We have a chance to introduce various fast path for Object.assign in C++.

This patch moves implementation from JS to C++. It achieves the above (1) and (2). (3) is filed in [1].

We can see 1.65x improvement in SixSpeed object-assign.es6.

                            baseline                  patched

object-assign.es6      643.3253+-8.0521     ^    389.1075+-8.8840        ^ definitely 1.6533x faster

[1]: https://bugs.webkit.org/show_bug.cgi?id=173416

* builtins/ObjectConstructor.js:
(entries):
(assign): Deleted.
* runtime/JSCJSValueInlines.h:
(JSC::JSValue::putInline):
* runtime/JSCell.h:
* runtime/JSCellInlines.h:
(JSC::JSCell::putInline):
* runtime/JSObject.cpp:
(JSC::JSObject::put):
* runtime/JSObject.h:
* runtime/JSObjectInlines.h:
(JSC::JSObject::putInlineForJSObject):
(JSC::JSObject::putInline): Deleted.
* runtime/ObjectConstructor.cpp:
(JSC::objectConstructorAssign):

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

JSTests/ChangeLog
JSTests/stress/object-assign-string-first.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/builtins/ObjectConstructor.js
Source/JavaScriptCore/runtime/JSCJSValueInlines.h
Source/JavaScriptCore/runtime/JSCell.h
Source/JavaScriptCore/runtime/JSCellInlines.h
Source/JavaScriptCore/runtime/JSObject.cpp
Source/JavaScriptCore/runtime/JSObject.h
Source/JavaScriptCore/runtime/JSObjectInlines.h
Source/JavaScriptCore/runtime/ObjectConstructor.cpp

index 97b8669..320dd4b 100644 (file)
@@ -1,3 +1,16 @@
+2017-06-15  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Implement Object.assign in C++
+        https://bugs.webkit.org/show_bug.cgi?id=173414
+
+        Reviewed by Saam Barati.
+
+        * stress/object-assign-string-first.js: Added.
+        (shouldBe):
+        (source.get Symbol):
+        (source.get 1):
+        (source.get cocoa):
+
 2017-06-14  JF Bastien  <jfbastien@apple.com>
 
         WebAssembly: remove empty test files
diff --git a/JSTests/stress/object-assign-string-first.js b/JSTests/stress/object-assign-string-first.js
new file mode 100644 (file)
index 0000000..55c2489
--- /dev/null
@@ -0,0 +1,32 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+var order = [];
+
+var source = {
+    get [Symbol.iterator]()
+    {
+        order.push(`Symbol.iterator`);
+        return `Symbol.iterator`;
+    },
+
+    get 1()
+    {
+        order.push(`1`);
+        return `1`;
+    },
+
+    get cocoa()
+    {
+        order.push(`cocoa`);
+        return `cocoa`;
+    },
+};
+
+var result = Object.assign({}, source);
+shouldBe(result[1], `1`);
+shouldBe(result.cocoa, `cocoa`);
+shouldBe(result[Symbol.iterator], `Symbol.iterator`);
+shouldBe(order.join(','), `1,cocoa,Symbol.iterator`);
index 320a68f..5bd59ad 100644 (file)
@@ -1,3 +1,47 @@
+2017-06-15  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Implement Object.assign in C++
+        https://bugs.webkit.org/show_bug.cgi?id=173414
+
+        Reviewed by Saam Barati.
+
+        Implementing Object.assign in JS is not so good compared to C++ version because,
+
+        1. JS version allocates JS array for object own keys. And we allocate JSString / Symbol for each key.
+        But basically, they can be handled as UniquedStringImpl in C++. Allocating these cells are wasteful.
+
+        2. While implementing builtins in JS offers some good type speculation chances, Object.assign is inherently super polymorphic.
+        So JS's type profile doesn't help well.
+
+        3. We have a chance to introduce various fast path for Object.assign in C++.
+
+        This patch moves implementation from JS to C++. It achieves the above (1) and (2). (3) is filed in [1].
+
+        We can see 1.65x improvement in SixSpeed object-assign.es6.
+
+                                    baseline                  patched
+
+        object-assign.es6      643.3253+-8.0521     ^    389.1075+-8.8840        ^ definitely 1.6533x faster
+
+        [1]: https://bugs.webkit.org/show_bug.cgi?id=173416
+
+        * builtins/ObjectConstructor.js:
+        (entries):
+        (assign): Deleted.
+        * runtime/JSCJSValueInlines.h:
+        (JSC::JSValue::putInline):
+        * runtime/JSCell.h:
+        * runtime/JSCellInlines.h:
+        (JSC::JSCell::putInline):
+        * runtime/JSObject.cpp:
+        (JSC::JSObject::put):
+        * runtime/JSObject.h:
+        * runtime/JSObjectInlines.h:
+        (JSC::JSObject::putInlineForJSObject):
+        (JSC::JSObject::putInline): Deleted.
+        * runtime/ObjectConstructor.cpp:
+        (JSC::objectConstructorAssign):
+
 2017-06-14  Dan Bernstein  <mitz@apple.com>
 
         [Cocoa] Objective-C class whose name begins with an underscore can’t be exported to JavaScript
index 263b2fc..14d3d6c 100644 (file)
@@ -66,26 +66,3 @@ function entries(object)
     
     return @enumerableOwnProperties(object, @iterationKindKeyValue);
 }
-
-function assign(target/*[*/, /*...*/sources/*] */)
-{
-    "use strict";
-
-    if (target == null)
-        @throwTypeError("Object.assign requires that input parameter not be null or undefined");
-
-    let objTarget = @Object(target);
-    for (let s = 1, argumentsLength = arguments.length; s < argumentsLength; ++s) {
-        let nextSource = arguments[s];
-        if (nextSource != null) {
-            let from = @Object(nextSource);
-            let keys = @Reflect.@ownKeys(from);
-            for (let i = 0, keysLength = keys.length; i < keysLength; ++i) {
-                let nextKey = keys[i];
-                if (@propertyIsEnumerable(from, nextKey))
-                    objTarget[nextKey] = from[nextKey];
-            }
-        }
-    }
-    return objTarget;
-}
index 5bcb995..3ef26e3 100644 (file)
@@ -876,16 +876,7 @@ ALWAYS_INLINE bool JSValue::putInline(ExecState* exec, PropertyName propertyName
 {
     if (UNLIKELY(!isCell()))
         return putToPrimitive(exec, propertyName, value, slot);
-
-    JSCell* cell = asCell();
-    auto putMethod = cell->methodTable(exec->vm())->put;
-    if (LIKELY(putMethod == JSObject::put))
-        return JSObject::putInline(cell, exec, propertyName, value, slot);
-
-    PutPropertySlot otherSlot = slot;
-    bool result = putMethod(cell, exec, propertyName, value, otherSlot);
-    slot = otherSlot;
-    return result;
+    return asCell()->putInline(exec, propertyName, value, slot);
 }
 
 inline bool JSValue::putByIndex(ExecState* exec, unsigned propertyName, JSValue value, bool shouldThrow)
index 214b98a..e79064e 100644 (file)
@@ -176,6 +176,7 @@ public:
     const MethodTable* methodTable(VM&) const;
     static bool put(JSCell*, ExecState*, PropertyName, JSValue, PutPropertySlot&);
     static bool putByIndex(JSCell*, ExecState*, unsigned propertyName, JSValue, bool shouldThrow);
+    bool putInline(ExecState*, PropertyName, JSValue, PutPropertySlot&);
         
     static bool deleteProperty(JSCell*, ExecState*, PropertyName);
     static bool deletePropertyByIndex(JSCell*, ExecState*, unsigned propertyName);
index c52985c..3571a08 100644 (file)
@@ -357,6 +357,14 @@ inline JSObject* JSCell::toObject(ExecState* exec, JSGlobalObject* globalObject)
     return toObjectSlow(exec, globalObject);
 }
 
+ALWAYS_INLINE bool JSCell::putInline(ExecState* exec, PropertyName propertyName, JSValue value, PutPropertySlot& slot)
+{
+    auto putMethod = methodTable(exec->vm())->put;
+    if (LIKELY(putMethod == JSObject::put))
+        return JSObject::putInlineForJSObject(asObject(this), exec, propertyName, value, slot);
+    return putMethod(this, exec, propertyName, value, slot);
+}
+
 inline bool isWebAssemblyToJSCallee(const JSCell* cell)
 {
     return cell->type() == WebAssemblyToJSCalleeType;
index b71bd4e..1d12688 100644 (file)
@@ -748,7 +748,7 @@ bool ordinarySetSlow(ExecState* exec, JSObject* object, PropertyName propertyNam
 // ECMA 8.6.2.2
 bool JSObject::put(JSCell* cell, ExecState* exec, PropertyName propertyName, JSValue value, PutPropertySlot& slot)
 {
-    return putInline(cell, exec, propertyName, value, slot);
+    return putInlineForJSObject(cell, exec, propertyName, value, slot);
 }
 
 bool JSObject::putInlineSlow(ExecState* exec, PropertyName propertyName, JSValue value, PutPropertySlot& slot)
index f4615bf..3043dd1 100644 (file)
@@ -190,7 +190,7 @@ public:
         return m_butterfly.get()->vectorLength();
     }
     
-    static bool putInline(JSCell*, ExecState*, PropertyName, JSValue, PutPropertySlot&);
+    static bool putInlineForJSObject(JSCell*, ExecState*, PropertyName, JSValue, PutPropertySlot&);
     
     JS_EXPORT_PRIVATE static bool put(JSCell*, ExecState*, PropertyName, JSValue, PutPropertySlot&);
     // putByIndex assumes that the receiver is this JSCell object.
index d5b3080..1d60c7f 100644 (file)
@@ -194,7 +194,7 @@ ALWAYS_INLINE PropertyOffset JSObject::prepareToPutDirectWithoutTransition(VM& v
 }
 
 // ECMA 8.6.2.2
-ALWAYS_INLINE bool JSObject::putInline(JSCell* cell, ExecState* exec, PropertyName propertyName, JSValue value, PutPropertySlot& slot)
+ALWAYS_INLINE bool JSObject::putInlineForJSObject(JSCell* cell, ExecState* exec, PropertyName propertyName, JSValue value, PutPropertySlot& slot)
 {
     VM& vm = exec->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
index 803b909..5516c52 100644 (file)
@@ -39,6 +39,7 @@
 
 namespace JSC {
 
+EncodedJSValue JSC_HOST_CALL objectConstructorAssign(ExecState*);
 EncodedJSValue JSC_HOST_CALL objectConstructorGetPrototypeOf(ExecState*);
 EncodedJSValue JSC_HOST_CALL objectConstructorSetPrototypeOf(ExecState*);
 EncodedJSValue JSC_HOST_CALL objectConstructorGetOwnPropertyNames(ExecState*);
@@ -82,7 +83,7 @@ const ClassInfo ObjectConstructor::s_info = { "Function", &InternalFunction::s_i
   isFrozen                  objectConstructorIsFrozen                   DontEnum|Function 1
   isExtensible              objectConstructorIsExtensible               DontEnum|Function 1
   is                        objectConstructorIs                         DontEnum|Function 2
-  assign                    JSBuiltin                                   DontEnum|Function 2
+  assign                    objectConstructorAssign                     DontEnum|Function 2
   values                    JSBuiltin                                   DontEnum|Function 1
   entries                   JSBuiltin                                   DontEnum|Function 1
 @end
@@ -305,6 +306,73 @@ EncodedJSValue JSC_HOST_CALL ownEnumerablePropertyKeys(ExecState* exec)
     return JSValue::encode(ownPropertyKeys(exec, object, PropertyNameMode::StringsAndSymbols, DontEnumPropertiesMode::Exclude));
 }
 
+EncodedJSValue JSC_HOST_CALL objectConstructorAssign(ExecState* exec)
+{
+    VM& vm = exec->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
+    JSValue targetValue = exec->argument(0);
+    if (targetValue.isUndefinedOrNull())
+        return throwVMTypeError(exec, scope, ASCIILiteral("Object.assign requires that input parameter not be null or undefined"));
+    JSObject* target = targetValue.toObject(exec);
+    RETURN_IF_EXCEPTION(scope, { });
+
+    unsigned argsCount = exec->argumentCount();
+    for (unsigned i = 1; i < argsCount; ++i) {
+        JSValue sourceValue = exec->uncheckedArgument(i);
+        if (sourceValue.isUndefinedOrNull())
+            continue;
+        JSObject* source = sourceValue.toObject(exec);
+        RETURN_IF_EXCEPTION(scope, { });
+
+        PropertyNameArray properties(exec, PropertyNameMode::StringsAndSymbols);
+        source->methodTable(vm)->getOwnPropertyNames(source, exec, properties, EnumerationMode(DontEnumPropertiesMode::Include));
+        RETURN_IF_EXCEPTION(scope, { });
+
+        auto assign = [&] (PropertyName propertyName) {
+            // FIXME: We can avoid this enumerable look up by checking Structure's status.
+            // https://bugs.webkit.org/show_bug.cgi?id=173416
+            PropertySlot slot(source, PropertySlot::InternalMethodType::GetOwnProperty);
+            if (!source->methodTable(vm)->getOwnPropertySlot(source, exec, propertyName, slot))
+                return;
+            if (slot.attributes() & DontEnum)
+                return;
+
+            JSValue value = source->get(exec, propertyName);
+            RETURN_IF_EXCEPTION(scope, void());
+
+            PutPropertySlot putPropertySlot(target, true);
+            target->putInline(exec, propertyName, value, putPropertySlot);
+        };
+
+        // First loop is for strings. Second loop is for symbols to keep standardized order requirement in the spec.
+        // https://tc39.github.io/ecma262/#sec-ordinaryownpropertykeys
+        bool foundSymbol = false;
+        unsigned numProperties = properties.size();
+        for (unsigned j = 0; j < numProperties; j++) {
+            const auto& propertyName = properties[j];
+            if (propertyName.isSymbol()) {
+                foundSymbol = true;
+                continue;
+            }
+
+            assign(propertyName);
+            RETURN_IF_EXCEPTION(scope, { });
+        }
+
+        if (foundSymbol) {
+            for (unsigned j = 0; j < numProperties; j++) {
+                const auto& propertyName = properties[j];
+                if (propertyName.isSymbol() && !vm.propertyNames->isPrivateName(propertyName)) {
+                    assign(propertyName);
+                    RETURN_IF_EXCEPTION(scope, { });
+                }
+            }
+        }
+    }
+    return JSValue::encode(target);
+}
+
 // ES6 6.2.4.5 ToPropertyDescriptor
 // https://tc39.github.io/ecma262/#sec-topropertydescriptor
 bool toPropertyDescriptor(ExecState* exec, JSValue in, PropertyDescriptor& desc)