[JSC] Object.values should be implemented in C++
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Jun 2017 15:18:53 +0000 (15:18 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Jun 2017 15:18:53 +0000 (15:18 +0000)
https://bugs.webkit.org/show_bug.cgi?id=173703

Reviewed by Sam Weinig.

JSTests:

* microbenchmarks/object-keys-map-values.js: Added.
(test):
* microbenchmarks/object-values.js: Added.
(test):
* stress/object-values-changing-properties.js: Added.
(shouldBe):
(throw.new.Error.let.source.get x):
(throw.new.Error):
(shouldBe.let.handler.get order):
(get let):
(shouldBe.let.handler.get return):
(let.handler.get order):

Source/JavaScriptCore:

As the same to Object.assign, Object.values() is also inherently polymorphic.
And allocating JSString / Symbol for Identifier and JSArray for Object.keys()
result is costly.

In this patch, we implement Object.values() in C++. It can avoid above allocations.
Furthermore, by using `slot.isTaintedByOpaqueObject()` information, we can skip
non-observable JSObject::get() calls.

This improves performance by 2.49x. And also now Object.values() beats
Object.keys(object).map(key => object[key]) implementation.

                                     baseline                  patched

    object-values               132.1551+-3.7209     ^     53.1254+-1.6139        ^ definitely 2.4876x faster
    object-keys-map-values       78.2008+-2.1378     ?     78.9078+-2.2121        ?

* builtins/ObjectConstructor.js:
(values): Deleted.
* runtime/ObjectConstructor.cpp:
(JSC::objectConstructorValues):

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

JSTests/ChangeLog
JSTests/microbenchmarks/object-keys-map-values.js [new file with mode: 0644]
JSTests/microbenchmarks/object-values.js [new file with mode: 0644]
JSTests/stress/object-values-changing-properties.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/builtins/ObjectConstructor.js
Source/JavaScriptCore/runtime/ObjectConstructor.cpp

index d5f6e87..9140313 100644 (file)
@@ -1,3 +1,23 @@
+2017-06-22  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Object.values should be implemented in C++
+        https://bugs.webkit.org/show_bug.cgi?id=173703
+
+        Reviewed by Sam Weinig.
+
+        * microbenchmarks/object-keys-map-values.js: Added.
+        (test):
+        * microbenchmarks/object-values.js: Added.
+        (test):
+        * stress/object-values-changing-properties.js: Added.
+        (shouldBe):
+        (throw.new.Error.let.source.get x):
+        (throw.new.Error):
+        (shouldBe.let.handler.get order):
+        (get let):
+        (shouldBe.let.handler.get return):
+        (let.handler.get order):
+
 2017-06-21  Saam Barati  <sbarati@apple.com>
 
         eval virtual call is incorrect in the baseline JIT
diff --git a/JSTests/microbenchmarks/object-keys-map-values.js b/JSTests/microbenchmarks/object-keys-map-values.js
new file mode 100644 (file)
index 0000000..19115e3
--- /dev/null
@@ -0,0 +1,13 @@
+var object = {};
+for (var i = 0; i < 1e3; ++i) {
+    object[i + 'prop'] = i;
+}
+
+function test(object)
+{
+    return Object.keys(object).map(key => object[key]);
+}
+noInline(test);
+
+for (var i = 0; i < 1e3; ++i)
+    test(object);
diff --git a/JSTests/microbenchmarks/object-values.js b/JSTests/microbenchmarks/object-values.js
new file mode 100644 (file)
index 0000000..672e7f8
--- /dev/null
@@ -0,0 +1,13 @@
+var object = {};
+for (var i = 0; i < 1e3; ++i) {
+    object[i + 'prop'] = i;
+}
+
+function test(object)
+{
+    return Object.values(object);
+}
+noInline(test);
+
+for (var i = 0; i < 1e3; ++i)
+    test(object);
diff --git a/JSTests/stress/object-values-changing-properties.js b/JSTests/stress/object-values-changing-properties.js
new file mode 100644 (file)
index 0000000..8697efd
--- /dev/null
@@ -0,0 +1,96 @@
+function shouldBe(actual, expected)
+{
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+{
+    let source = {
+        get x() {
+            delete this.y;
+            return 42;
+        },
+        y: 42
+    };
+    let result = Object.values(source);
+    shouldBe(result.length, 1);
+    shouldBe(result[0], 42);
+}
+
+{
+    let source = Object.defineProperties({}, {
+        nonEnumerable: {
+            enumerable: false,
+            value: 42
+        }
+    });
+
+    let result = Object.values(source);
+    shouldBe(result.length, 0);
+}
+
+{
+    let order = [];
+    let target = {x: 20, y:42};
+    let handler = {
+        getOwnPropertyDescriptor(theTarget, propName)
+        {
+            order.push(`getOwnPropertyDescriptor ${propName}`);
+            return {
+                enumerable: true,
+                configurable: true,
+                value: 42
+            };
+        },
+        get(theTarget, propName, receiver)
+        {
+            order.push(`get ${propName}`);
+            return 20;
+        }
+    };
+
+    let proxy = new Proxy(target, handler);
+    let result = Object.values(proxy);
+    shouldBe(result.length, 2);
+    shouldBe(result[0], 20);
+    shouldBe(result[1], 20);
+    shouldBe(order.join(','), `getOwnPropertyDescriptor x,get x,getOwnPropertyDescriptor y,get y`);
+}
+
+{
+    let order = [];
+    let target = Object.defineProperties({}, {
+        x: {
+            enumerable: false,
+            configurable: true,
+            value: 20
+        },
+        y: {
+            enumerable: false,
+            configurable: true,
+            value: 42
+        }
+    });
+
+    let handler = {
+        getOwnPropertyDescriptor(theTarget, propName)
+        {
+            order.push(`getOwnPropertyDescriptor ${propName}`);
+            return {
+                enumerable: false,
+                configurable: true,
+                value: 42
+            };
+        },
+        get(theTarget, propName, receiver)
+        {
+            order.push(`get ${propName}`);
+            return 42;
+        }
+    };
+
+    let proxy = new Proxy(target, handler);
+    let result = Object.values(proxy);
+    shouldBe(result.length, 0);
+    shouldBe(order.join(','), `getOwnPropertyDescriptor x,getOwnPropertyDescriptor y`);
+}
index ddcaac8..37cb9a2 100644 (file)
@@ -1,3 +1,31 @@
+2017-06-22  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Object.values should be implemented in C++
+        https://bugs.webkit.org/show_bug.cgi?id=173703
+
+        Reviewed by Sam Weinig.
+
+        As the same to Object.assign, Object.values() is also inherently polymorphic.
+        And allocating JSString / Symbol for Identifier and JSArray for Object.keys()
+        result is costly.
+
+        In this patch, we implement Object.values() in C++. It can avoid above allocations.
+        Furthermore, by using `slot.isTaintedByOpaqueObject()` information, we can skip
+        non-observable JSObject::get() calls.
+
+        This improves performance by 2.49x. And also now Object.values() beats
+        Object.keys(object).map(key => object[key]) implementation.
+
+                                             baseline                  patched
+
+            object-values               132.1551+-3.7209     ^     53.1254+-1.6139        ^ definitely 2.4876x faster
+            object-keys-map-values       78.2008+-2.1378     ?     78.9078+-2.2121        ?
+
+        * builtins/ObjectConstructor.js:
+        (values): Deleted.
+        * runtime/ObjectConstructor.cpp:
+        (JSC::objectConstructorValues):
+
 2017-06-21  Saam Barati  <sbarati@apple.com>
 
         ArrayPrototype.map builtin declares a var it does not use
index 14d3d6c..4c1096a 100644 (file)
@@ -47,16 +47,6 @@ function enumerableOwnProperties(object, kind)
     return properties;
 }
 
-function values(object)
-{
-    "use strict";
-    
-    if (object == null)
-        @throwTypeError("Object.values requires that input parameter not be null or undefined");
-
-    return @enumerableOwnProperties(object, @iterationKindValue);
-}
-
 function entries(object)
 {
     "use strict";
index 21c933b..a0dbbc6 100644 (file)
@@ -40,6 +40,7 @@
 namespace JSC {
 
 EncodedJSValue JSC_HOST_CALL objectConstructorAssign(ExecState*);
+EncodedJSValue JSC_HOST_CALL objectConstructorValues(ExecState*);
 EncodedJSValue JSC_HOST_CALL objectConstructorGetPrototypeOf(ExecState*);
 EncodedJSValue JSC_HOST_CALL objectConstructorSetPrototypeOf(ExecState*);
 EncodedJSValue JSC_HOST_CALL objectConstructorGetOwnPropertyNames(ExecState*);
@@ -84,7 +85,7 @@ const ClassInfo ObjectConstructor::s_info = { "Function", &InternalFunction::s_i
   isExtensible              objectConstructorIsExtensible               DontEnum|Function 1
   is                        objectConstructorIs                         DontEnum|Function 2
   assign                    objectConstructorAssign                     DontEnum|Function 2
-  values                    JSBuiltin                                   DontEnum|Function 1
+  values                    objectConstructorValues                     DontEnum|Function 1
   entries                   JSBuiltin                                   DontEnum|Function 1
 @end
 */
@@ -375,6 +376,54 @@ EncodedJSValue JSC_HOST_CALL objectConstructorAssign(ExecState* exec)
     return JSValue::encode(target);
 }
 
+EncodedJSValue JSC_HOST_CALL objectConstructorValues(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.values requires that input parameter not be null or undefined"));
+    JSObject* target = targetValue.toObject(exec);
+    RETURN_IF_EXCEPTION(scope, { });
+
+    JSArray* values = constructEmptyArray(exec, 0);
+    RETURN_IF_EXCEPTION(scope, { });
+
+    PropertyNameArray properties(exec, PropertyNameMode::Strings);
+    target->methodTable(vm)->getOwnPropertyNames(target, exec, properties, EnumerationMode(DontEnumPropertiesMode::Include));
+    RETURN_IF_EXCEPTION(scope, { });
+
+    auto addValue = [&] (PropertyName propertyName) {
+        PropertySlot slot(target, PropertySlot::InternalMethodType::GetOwnProperty);
+        if (!target->methodTable(vm)->getOwnPropertySlot(target, exec, propertyName, slot))
+            return;
+        if (slot.attributes() & DontEnum)
+            return;
+
+        JSValue value;
+        if (LIKELY(!slot.isTaintedByOpaqueObject()))
+            value = slot.getValue(exec, propertyName);
+        else
+            value = target->get(exec, propertyName);
+        RETURN_IF_EXCEPTION(scope, void());
+
+        values->push(exec, value);
+    };
+
+    for (unsigned i = 0, numProperties = properties.size(); i < numProperties; i++) {
+        const auto& propertyName = properties[i];
+        if (propertyName.isSymbol())
+            continue;
+
+        addValue(propertyName);
+        RETURN_IF_EXCEPTION(scope, { });
+    }
+
+    return JSValue::encode(values);
+}
+
+
 // ES6 6.2.4.5 ToPropertyDescriptor
 // https://tc39.github.io/ecma262/#sec-topropertydescriptor
 bool toPropertyDescriptor(ExecState* exec, JSValue in, PropertyDescriptor& desc)