Attachment 339637 Details for Bug 185348 – WIP

[patch] WIP

bug-185348-20180505172255.patch (text/plain), 8.10 KB, created by Yusuke Suzuki on 2018-05-05 01:22:56 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2018-05-05 01:22:56 PDT

Size: 8.10 KB

patch

obsolete

>Subversion Revision: 231396
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index e61aafe70d4e23936e525f18ce92bafdb48c2b50..aded145c5333c92a4a12123da039c8768d5dd19d 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,33 @@
>+2018-05-05  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 NOBODY (OOPS!).
>+
>+        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.
>+
>+        This improves object-assign.es6 by 1.8x.
>+
>+                                        baseline                  patched
>+
>+            object-assign.es6      361.2537+-8.7837     ^    198.0301+-8.1980        ^ definitely 1.8242x faster
>+
>+        * runtime/JSObject.h:
>+        * runtime/JSObjectInlines.h:
>+        (JSC::JSObject::canPerformFastPutInlineExcludingProto):
>+        (JSC::JSObject::canPerformFastPutInline):
>+        * runtime/ObjectConstructor.cpp:
>+        (JSC::objectConstructorAssign):
>+        * runtime/Structure.h:
>+        * runtime/StructureInlines.h:
>+        (JSC::Structure::forEachProperty):
>+
> 2018-05-04  Keith Miller  <keith_miller@apple.com>
> 
>         isCacheableArrayLength should return true for undecided arrays
>diff --git a/Source/JavaScriptCore/runtime/JSObject.h b/Source/JavaScriptCore/runtime/JSObject.h
>index c2eb8e6936429f4f6acef77754bf636d33ea3674..e4e9b32ba083d1d98081bccbec68d1df7ab41f85 100644
>--- a/Source/JavaScriptCore/runtime/JSObject.h
>+++ b/Source/JavaScriptCore/runtime/JSObject.h
>@@ -872,6 +872,9 @@ class JSObject : public JSCell {
> 
>     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 @@ class JSObject : public JSCell {
>         
>     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&);
> 
>diff --git a/Source/JavaScriptCore/runtime/JSObjectInlines.h b/Source/JavaScriptCore/runtime/JSObjectInlines.h
>index 5e2701f17a433e32d1bc6e9c87a4a5589d4f3f8f..9c720a8b126b80f47b6633e4546235897dd7796b 100644
>--- a/Source/JavaScriptCore/runtime/JSObjectInlines.h
>+++ b/Source/JavaScriptCore/runtime/JSObjectInlines.h
>@@ -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
> {
>diff --git a/Source/JavaScriptCore/runtime/ObjectConstructor.cpp b/Source/JavaScriptCore/runtime/ObjectConstructor.cpp
>index e18c765473a024d33bf50ced0be9c1cc1a29b166..d60565a51d1de748c8516afdb2a1e6ae2f3e2705 100644
>--- a/Source/JavaScriptCore/runtime/ObjectConstructor.cpp
>+++ b/Source/JavaScriptCore/runtime/ObjectConstructor.cpp
>@@ -299,6 +299,7 @@ EncodedJSValue JSC_HOST_CALL objectConstructorAssign(ExecState* exec)
>         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, { });
>+    bool targetCanPerformFastPut = jsDynamicCast<JSFinalObject*>(vm, target) && target->canPerformFastPutInlineExcludingProto(vm);
> 
>     unsigned argsCount = exec->argumentCount();
>     for (unsigned i = 1; i < argsCount; ++i) {
>@@ -308,6 +309,27 @@ EncodedJSValue JSC_HOST_CALL objectConstructorAssign(ExecState* exec)
>         JSObject* source = sourceValue.toObject(exec);
>         RETURN_IF_EXCEPTION(scope, { });
> 
>+        if (targetCanPerformFastPut && jsDynamicCast<JSFinalObject*>(vm, source)) {
>+            Structure* structure = source->structure(vm);
>+            if (structure->canAccessPropertiesQuicklyForEnumeration()) {
>+                // 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;
>+
>+                    PutPropertySlot putPropertySlot(target, true);
>+                    target->putOwnDataProperty(vm, entry.key, 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, { });
>diff --git a/Source/JavaScriptCore/runtime/Structure.h b/Source/JavaScriptCore/runtime/Structure.h
>index 37696bb1d039c1be24507da409863a2b9bbe5805..4bbb27813d05960a5b49790f0ba10e3f52d49971 100644
>--- a/Source/JavaScriptCore/runtime/Structure.h
>+++ b/Source/JavaScriptCore/runtime/Structure.h
>@@ -430,6 +430,9 @@ class Structure final : public JSCell {
>     // 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);
>diff --git a/Source/JavaScriptCore/runtime/StructureInlines.h b/Source/JavaScriptCore/runtime/StructureInlines.h
>index 9047f2264bf5bcd78cf53d8ef6218c3b503a95ba..417d42b145f1d65969c60a996fb82bac3692b4c0 100644
>--- a/Source/JavaScriptCore/runtime/StructureInlines.h
>+++ b/Source/JavaScriptCore/runtime/StructureInlines.h
>@@ -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;

Actions: View | Formatted Diff | Diff

Attachments on bug 185348: 339637 | 339638 | 339639 | 339645 | 339666 | 339668 | 339671 | 339943