Avoid 2 times name iteration in Object.assign
https://bugs.webkit.org/show_bug.cgi?id=147268
Reviewed by Geoffrey Garen.
Object.assign calls Object.getOwnPropertyNames & Object.getOwnPropertySymbols to collect all the names.
But exposing the private API that collects both at the same time makes the API efficient when the given Object has so many non-indexed properties.
Since Object.assign is so generic API (some form of utility API), the form of the given Object is not expected.
So the taken object may have so many non-indexed properties.
In this patch, we introduce `ownEnumerablePropertyKeys` private function.
It is minor changed version of `[[OwnPropertyKeys]]` in the ES6 spec;
It only includes enumerable properties.
By filtering out the non-enumerable properties in the exposed private function,
we avoid calling @objectGetOwnPropertyDescriptor for each property at the same time.
* builtins/ObjectConstructor.js:
(assign):
* runtime/CommonIdentifiers.h:
* runtime/EnumerationMode.h:
* runtime/JSGlobalObject.cpp:
(JSC::JSGlobalObject::init):
* runtime/ObjectConstructor.cpp:
(JSC::ownEnumerablePropertyKeys):
* runtime/ObjectConstructor.h:
* tests/stress/object-assign-enumerable.js: Added.
(shouldBe):
* tests/stress/object-assign-order.js: Added.
(shouldBe):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@187363 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 14f8893..4bb4ca5 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,5 +1,38 @@
2015-07-24 Yusuke Suzuki <utatane.tea@gmail.com>
+ Avoid 2 times name iteration in Object.assign
+ https://bugs.webkit.org/show_bug.cgi?id=147268
+
+ Reviewed by Geoffrey Garen.
+
+ Object.assign calls Object.getOwnPropertyNames & Object.getOwnPropertySymbols to collect all the names.
+ But exposing the private API that collects both at the same time makes the API efficient when the given Object has so many non-indexed properties.
+ Since Object.assign is so generic API (some form of utility API), the form of the given Object is not expected.
+ So the taken object may have so many non-indexed properties.
+
+ In this patch, we introduce `ownEnumerablePropertyKeys` private function.
+ It is minor changed version of `[[OwnPropertyKeys]]` in the ES6 spec;
+ It only includes enumerable properties.
+
+ By filtering out the non-enumerable properties in the exposed private function,
+ we avoid calling @objectGetOwnPropertyDescriptor for each property at the same time.
+
+ * builtins/ObjectConstructor.js:
+ (assign):
+ * runtime/CommonIdentifiers.h:
+ * runtime/EnumerationMode.h:
+ * runtime/JSGlobalObject.cpp:
+ (JSC::JSGlobalObject::init):
+ * runtime/ObjectConstructor.cpp:
+ (JSC::ownEnumerablePropertyKeys):
+ * runtime/ObjectConstructor.h:
+ * tests/stress/object-assign-enumerable.js: Added.
+ (shouldBe):
+ * tests/stress/object-assign-order.js: Added.
+ (shouldBe):
+
+2015-07-24 Yusuke Suzuki <utatane.tea@gmail.com>
+
Remove runtime flags for symbols
https://bugs.webkit.org/show_bug.cgi?id=147246
diff --git a/Source/JavaScriptCore/builtins/ObjectConstructor.js b/Source/JavaScriptCore/builtins/ObjectConstructor.js
index eeec196..6b7ac19 100644
--- a/Source/JavaScriptCore/builtins/ObjectConstructor.js
+++ b/Source/JavaScriptCore/builtins/ObjectConstructor.js
@@ -30,27 +30,14 @@
throw new @TypeError("can't convert " + target + " to object");
var objTarget = @Object(target);
- var s, nextSource, from, i, keys, nextKey, desc;
- for (s = 1; s < arguments.length; ++s) {
- nextSource = arguments[s];
+ for (var s = 1, argumentsLength = arguments.length; s < argumentsLength; ++s) {
+ var nextSource = arguments[s];
if (nextSource != null) {
- from = @Object(nextSource);
- // TODO: replace @objectKeys + @objectGetOwnPropertySymbols with single @OwnPropertyKeys c++ operation
- keys = @objectKeys(from);
- for (i = 0; i < keys.length; ++i) {
- nextKey = keys[i];
- desc = @objectGetOwnPropertyDescriptor(from, nextKey);
- if (typeof desc !== "undefined" && desc.enumerable) {
- objTarget[nextKey] = from[nextKey];
- }
- }
- keys = @objectGetOwnPropertySymbols(from);
- for (i = 0; i < keys.length; ++i) {
- nextKey = keys[i];
- desc = @objectGetOwnPropertyDescriptor(from, nextKey);
- if (typeof desc !== "undefined" && desc.enumerable) {
- objTarget[nextKey] = from[nextKey];
- }
+ var from = @Object(nextSource);
+ var keys = @ownEnumerablePropertyKeys(from);
+ for (var i = 0, keysLength = keys.length; i < keysLength; ++i) {
+ var nextKey = keys[i];
+ objTarget[nextKey] = from[nextKey];
}
}
}
diff --git a/Source/JavaScriptCore/runtime/CommonIdentifiers.h b/Source/JavaScriptCore/runtime/CommonIdentifiers.h
index 9b93987..a1ea328 100644
--- a/Source/JavaScriptCore/runtime/CommonIdentifiers.h
+++ b/Source/JavaScriptCore/runtime/CommonIdentifiers.h
@@ -270,9 +270,7 @@
macro(deferred) \
macro(countdownHolder) \
macro(Object) \
- macro(objectKeys) \
- macro(objectGetOwnPropertyDescriptor) \
- macro(objectGetOwnPropertySymbols) \
+ macro(ownEnumerablePropertyKeys) \
macro(Number) \
macro(Array) \
macro(String) \
diff --git a/Source/JavaScriptCore/runtime/EnumerationMode.h b/Source/JavaScriptCore/runtime/EnumerationMode.h
index 83c0568..e842deb 100644
--- a/Source/JavaScriptCore/runtime/EnumerationMode.h
+++ b/Source/JavaScriptCore/runtime/EnumerationMode.h
@@ -39,11 +39,6 @@
Exclude
};
-enum class SymbolPropertiesMode {
- Include,
- Exclude
-};
-
enum class JSObjectPropertiesMode {
Include,
Exclude
diff --git a/Source/JavaScriptCore/runtime/JSGlobalObject.cpp b/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
index 4104524..7f741ad 100644
--- a/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
+++ b/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
@@ -458,9 +458,6 @@
JSFunction* privateFuncFloor = JSFunction::create(vm, this, 0, String(), mathProtoFuncFloor, FloorIntrinsic);
JSFunction* privateFuncIsFinite = JSFunction::create(vm, this, 0, String(), globalFuncIsFinite);
- JSFunction* privateFuncObjectKeys = JSFunction::create(vm, this, 0, String(), objectConstructorKeys);
- JSFunction* privateFuncObjectGetOwnPropertyDescriptor = JSFunction::create(vm, this, 0, String(), objectConstructorGetOwnPropertyDescriptor);
- JSFunction* privateFuncObjectGetOwnPropertySymbols = JSFunction::create(vm, this, 0, String(), objectConstructorGetOwnPropertySymbols);
JSFunction* privateFuncGetTemplateObject = JSFunction::create(vm, this, 0, String(), getTemplateObject);
JSFunction* privateFuncToLength = JSFunction::createBuiltinFunction(vm, globalObjectToLengthCodeGenerator(vm), this);
JSFunction* privateFuncToInteger = JSFunction::createBuiltinFunction(vm, globalObjectToIntegerCodeGenerator(vm), this);
@@ -471,9 +468,7 @@
GlobalPropertyInfo(vm.propertyNames->undefinedKeyword, jsUndefined(), DontEnum | DontDelete | ReadOnly),
GlobalPropertyInfo(vm.propertyNames->undefinedPrivateName, jsUndefined(), DontEnum | DontDelete | ReadOnly),
GlobalPropertyInfo(vm.propertyNames->ObjectPrivateName, objectConstructor, DontEnum | DontDelete | ReadOnly),
- GlobalPropertyInfo(vm.propertyNames->objectKeysPrivateName, privateFuncObjectKeys, DontEnum | DontDelete | ReadOnly),
- GlobalPropertyInfo(vm.propertyNames->objectGetOwnPropertyDescriptorPrivateName, privateFuncObjectGetOwnPropertyDescriptor, DontEnum | DontDelete | ReadOnly),
- GlobalPropertyInfo(vm.propertyNames->objectGetOwnPropertySymbolsPrivateName, privateFuncObjectGetOwnPropertySymbols, DontEnum | DontDelete | ReadOnly),
+ GlobalPropertyInfo(vm.propertyNames->ownEnumerablePropertyKeysPrivateName, JSFunction::create(vm, this, 0, String(), ownEnumerablePropertyKeys), DontEnum | DontDelete | ReadOnly),
GlobalPropertyInfo(vm.propertyNames->getTemplateObjectPrivateName, privateFuncGetTemplateObject, DontEnum | DontDelete | ReadOnly),
GlobalPropertyInfo(vm.propertyNames->enqueueJobPrivateName, JSFunction::create(vm, this, 0, String(), enqueueJob), DontEnum | DontDelete | ReadOnly),
GlobalPropertyInfo(vm.propertyNames->TypeErrorPrivateName, m_typeErrorConstructor.get(), DontEnum | DontDelete | ReadOnly),
diff --git a/Source/JavaScriptCore/runtime/ObjectConstructor.cpp b/Source/JavaScriptCore/runtime/ObjectConstructor.cpp
index 240086f..d53f652 100644
--- a/Source/JavaScriptCore/runtime/ObjectConstructor.cpp
+++ b/Source/JavaScriptCore/runtime/ObjectConstructor.cpp
@@ -293,6 +293,33 @@
return JSValue::encode(keys);
}
+EncodedJSValue JSC_HOST_CALL ownEnumerablePropertyKeys(ExecState* exec)
+{
+ JSObject* object = exec->argument(0).toObject(exec);
+ if (exec->hadException())
+ return JSValue::encode(jsNull());
+ PropertyNameArray properties(exec, PropertyNameMode::Both);
+ object->methodTable(exec->vm())->getOwnPropertyNames(object, exec, properties, EnumerationMode());
+
+ JSArray* keys = constructEmptyArray(exec, 0);
+ Vector<Identifier, 16> propertySymbols;
+ size_t numProperties = properties.size();
+ for (size_t i = 0; i < numProperties; i++) {
+ const auto& identifier = properties[i];
+ if (identifier.isSymbol()) {
+ if (!exec->propertyNames().isPrivateName(identifier))
+ propertySymbols.append(identifier);
+ } else
+ keys->push(exec, jsOwnedString(exec, identifier.string()));
+ }
+
+ // To ensure the order defined in the spec (9.1.12), we append symbols at the last elements of keys.
+ for (const auto& identifier : propertySymbols)
+ keys->push(exec, Symbol::create(exec->vm(), static_cast<SymbolImpl&>(*identifier.impl())));
+
+ return JSValue::encode(keys);
+}
+
// ES5 8.10.5 ToPropertyDescriptor
static bool toPropertyDescriptor(ExecState* exec, JSValue in, PropertyDescriptor& desc)
{
diff --git a/Source/JavaScriptCore/runtime/ObjectConstructor.h b/Source/JavaScriptCore/runtime/ObjectConstructor.h
index 8fc5c73..2aa2c9e 100644
--- a/Source/JavaScriptCore/runtime/ObjectConstructor.h
+++ b/Source/JavaScriptCore/runtime/ObjectConstructor.h
@@ -30,6 +30,7 @@
EncodedJSValue JSC_HOST_CALL objectConstructorGetOwnPropertyDescriptor(ExecState*);
EncodedJSValue JSC_HOST_CALL objectConstructorGetOwnPropertySymbols(ExecState*);
EncodedJSValue JSC_HOST_CALL objectConstructorKeys(ExecState*);
+EncodedJSValue JSC_HOST_CALL ownEnumerablePropertyKeys(ExecState*);
class ObjectPrototype;
diff --git a/Source/JavaScriptCore/tests/stress/object-assign-enumerable.js b/Source/JavaScriptCore/tests/stress/object-assign-enumerable.js
new file mode 100644
index 0000000..8223a0d
--- /dev/null
+++ b/Source/JavaScriptCore/tests/stress/object-assign-enumerable.js
@@ -0,0 +1,14 @@
+function shouldBe(actual, expected) {
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+
+var object = Object.defineProperties({}, {
+ nonEnumerable: {
+ enumerable: false,
+ value: 42
+ }
+});
+
+var result = Object.assign({}, object);
+shouldBe(result.nonEnumerable, undefined);
diff --git a/Source/JavaScriptCore/tests/stress/object-assign-order.js b/Source/JavaScriptCore/tests/stress/object-assign-order.js
new file mode 100644
index 0000000..8af3ed6
--- /dev/null
+++ b/Source/JavaScriptCore/tests/stress/object-assign-order.js
@@ -0,0 +1,40 @@
+function shouldBe(actual, expected) {
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+
+var symbol = Symbol('Cocoa');
+
+var object = {
+ [symbol]: 3,
+ 0: 0,
+ hello: 2,
+ 1: 1,
+};
+
+var count = 0;
+
+var tester = Object.defineProperties({}, {
+ 0: {
+ set: () => {
+ shouldBe(count++, 0);
+ }
+ },
+ 1: {
+ set: () => {
+ shouldBe(count++, 1);
+ }
+ },
+ 'hello': {
+ set: () => {
+ shouldBe(count++, 2);
+ }
+ },
+ [symbol]: {
+ set: () => {
+ shouldBe(count++, 3);
+ }
+ },
+});
+
+Object.assign(tester, object);
