Log Message
[JSC] Object.assign for final objects should be faster https://bugs.webkit.org/show_bug.cgi?id=185348
Reviewed by Saam Barati.
JSTests:
* stress/object-assign-fast-path.js: Added.
(shouldBe):
(checkProperty):
Source/_javascript_Core:
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):
Modified Paths
- trunk/JSTests/ChangeLog
- trunk/Source/_javascript_Core/ChangeLog
- trunk/Source/_javascript_Core/runtime/JSObject.h
- trunk/Source/_javascript_Core/runtime/JSObjectInlines.h
- trunk/Source/_javascript_Core/runtime/ObjectConstructor.cpp
- trunk/Source/_javascript_Core/runtime/Structure.cpp
- trunk/Source/_javascript_Core/runtime/Structure.h
- trunk/Source/_javascript_Core/runtime/StructureInlines.h
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (231686 => 231687)
--- trunk/JSTests/ChangeLog 2018-05-11 02:07:25 UTC (rev 231686)
+++ trunk/JSTests/ChangeLog 2018-05-11 03:38:10 UTC (rev 231687)
@@ -1,3 +1,14 @@
+2018-05-09 Yusuke Suzuki <[email protected]>
+
+ [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 <[email protected]>
Update Test262 tests through the new import script - 20180509
Added: trunk/JSTests/stress/object-assign-fast-path.js (0 => 231687)
--- trunk/JSTests/stress/object-assign-fast-path.js (rev 0)
+++ trunk/JSTests/stress/object-assign-fast-path.js 2018-05-11 03:38:10 UTC (rev 231687)
@@ -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");
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (231686 => 231687)
--- trunk/Source/_javascript_Core/ChangeLog 2018-05-11 02:07:25 UTC (rev 231686)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-05-11 03:38:10 UTC (rev 231687)
@@ -1,3 +1,42 @@
+2018-05-09 Yusuke Suzuki <[email protected]>
+
+ [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 <[email protected]>
DFG CFA should pick the right time to inject OSR entry data
Modified: trunk/Source/_javascript_Core/runtime/JSObject.h (231686 => 231687)
--- trunk/Source/_javascript_Core/runtime/JSObject.h 2018-05-11 02:07:25 UTC (rev 231686)
+++ trunk/Source/_javascript_Core/runtime/JSObject.h 2018-05-11 03:38:10 UTC (rev 231687)
@@ -872,6 +872,9 @@
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 @@
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&);
Modified: trunk/Source/_javascript_Core/runtime/JSObjectInlines.h (231686 => 231687)
--- trunk/Source/_javascript_Core/runtime/JSObjectInlines.h 2018-05-11 02:07:25 UTC (rev 231686)
+++ trunk/Source/_javascript_Core/runtime/JSObjectInlines.h 2018-05-11 03:38:10 UTC (rev 231687)
@@ -60,11 +60,8 @@
}
}
-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 @@
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
{
Modified: trunk/Source/_javascript_Core/runtime/ObjectConstructor.cpp (231686 => 231687)
--- trunk/Source/_javascript_Core/runtime/ObjectConstructor.cpp 2018-05-11 02:07:25 UTC (rev 231686)
+++ trunk/Source/_javascript_Core/runtime/ObjectConstructor.cpp 2018-05-11 03:38:10 UTC (rev 231687)
@@ -300,6 +300,10 @@
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 @@
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, { });
Modified: trunk/Source/_javascript_Core/runtime/Structure.cpp (231686 => 231687)
--- trunk/Source/_javascript_Core/runtime/Structure.cpp 2018-05-11 02:07:25 UTC (rev 231686)
+++ trunk/Source/_javascript_Core/runtime/Structure.cpp 2018-05-11 03:38:10 UTC (rev 231687)
@@ -193,6 +193,7 @@
setHasGetterSetterProperties(classInfo->hasStaticSetterOrReadonlyProperties());
setHasCustomGetterSetterProperties(false);
setHasReadOnlyOrGetterSetterPropertiesExcludingProto(classInfo->hasStaticSetterOrReadonlyProperties());
+ setHasUnderscoreProtoPropertyExcludingOriginalProto(false);
setIsQuickPropertyAccessAllowedForEnumeration(true);
setAttributesInPrevious(0);
setDidPreventExtensions(false);
@@ -226,6 +227,7 @@
setHasGetterSetterProperties(m_classInfo->hasStaticSetterOrReadonlyProperties());
setHasCustomGetterSetterProperties(false);
setHasReadOnlyOrGetterSetterPropertiesExcludingProto(m_classInfo->hasStaticSetterOrReadonlyProperties());
+ setHasUnderscoreProtoPropertyExcludingOriginalProto(false);
setIsQuickPropertyAccessAllowedForEnumeration(true);
setAttributesInPrevious(0);
setDidPreventExtensions(false);
@@ -259,6 +261,7 @@
setHasGetterSetterProperties(previous->hasGetterSetterProperties());
setHasCustomGetterSetterProperties(previous->hasCustomGetterSetterProperties());
setHasReadOnlyOrGetterSetterPropertiesExcludingProto(previous->hasReadOnlyOrGetterSetterPropertiesExcludingProto());
+ setHasUnderscoreProtoPropertyExcludingOriginalProto(previous->hasUnderscoreProtoPropertyExcludingOriginalProto());
setIsQuickPropertyAccessAllowedForEnumeration(previous->isQuickPropertyAccessAllowedForEnumeration());
setAttributesInPrevious(0);
setDidPreventExtensions(previous->didPreventExtensions());
Modified: trunk/Source/_javascript_Core/runtime/Structure.h (231686 => 231687)
--- trunk/Source/_javascript_Core/runtime/Structure.h 2018-05-11 02:07:25 UTC (rev 231686)
+++ trunk/Source/_javascript_Core/runtime/Structure.h 2018-05-11 03:38:10 UTC (rev 231687)
@@ -432,6 +432,9 @@
// 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 @@
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;
Modified: trunk/Source/_javascript_Core/runtime/StructureInlines.h (231686 => 231687)
--- trunk/Source/_javascript_Core/runtime/StructureInlines.h 2018-05-11 02:07:25 UTC (rev 231686)
+++ trunk/Source/_javascript_Core/runtime/StructureInlines.h 2018-05-11 03:38:10 UTC (rev 231687)
@@ -163,6 +163,17 @@
}
}
+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 @@
checkConsistency();
if (attributes & PropertyAttribute::DontEnum || propertyName.isSymbol())
setIsQuickPropertyAccessAllowedForEnumeration(false);
+ if (propertyName == vm.propertyNames->underscoreProto)
+ setHasUnderscoreProtoPropertyExcludingOriginalProto(true);
auto rep = propertyName.uid();
_______________________________________________ webkit-changes mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-changes
