[MERGE #4817 @MikeHolman] optimize object.assign({}, obj)

Merge pull request #4817 from MikeHolman:objassign

In case of simple object.assign({}, obj) we can memcpy the properties over to the new object.

lib/Common/ConfigFlagsList.h

@@ -358,6 +358,7 @@ PHASE(All)
         PHASE(Error)
         PHASE(PropertyRecord)
         PHASE(TypePathDynamicSize)
+        PHASE(ObjectCopy)
         PHASE(ShareTypesWithAttributes)
         PHASE(ShareAccessorTypes)
         PHASE(ConditionalCompilation)

lib/Runtime/Debug/TTSnapObjects.cpp

@@ -82,7 +82,7 @@ namespace TTD
                 TTDVar* cpyBase = snpObject->VarArray;
                 if(sHandler->InlineSlotCapacity != 0)
                 {
-                    Js::Var const* inlineSlots = dynObj->GetInlineSlots_TTD();
+                    Field(Js::Var) const* inlineSlots = dynObj->GetInlineSlots_TTD();
 
                     //copy all the properties (if they all fit into the inline slots) otherwise just copy all the inline slot values
                     uint32 inlineSlotCount = min(sHandler->MaxPropertyIndex, sHandler->InlineSlotCapacity);

lib/Runtime/Library/JavascriptObject.cpp

@@ -1507,43 +1507,63 @@ namespace Js
 
         // 4. Let sources be the List of argument values starting with the second argument.
         // 5. For each element nextSource of sources, in ascending index order,
-        for (unsigned int i = 2; i < args.Info.Count; i++)
+        AssignHelper<true>(args[2], to, scriptContext);
+        for (unsigned int i = 3; i < args.Info.Count; i++)
         {
-            //      a. If nextSource is undefined or null, let keys be an empty List.
-            //      b. Else,
-            //          i.Let from be ToObject(nextSource).
-            //          ii.ReturnIfAbrupt(from).
-            //          iii.Let keys be from.[[OwnPropertyKeys]]().
-            //          iv.ReturnIfAbrupt(keys).
+            AssignHelper<false>(args[i], to, scriptContext);
+        }
+
+        // 6. Return to.
+        return to;
+    }
 
-            RecyclableObject* from = nullptr;
-            if (!JavascriptConversion::ToObject(args[i], scriptContext, &from))
+    template <bool tryCopy>
+    void JavascriptObject::AssignHelper(Var fromArg, RecyclableObject* to, ScriptContext* scriptContext)
+    {
+        //      a. If nextSource is undefined or null, let keys be an empty List.
+        //      b. Else,
+        //          i.Let from be ToObject(nextSource).
+        //          ii.ReturnIfAbrupt(from).
+        //          iii.Let keys be from.[[OwnPropertyKeys]]().
+        //          iv.ReturnIfAbrupt(keys).
+
+        RecyclableObject* from = nullptr;
+        if (!JavascriptConversion::ToObject(fromArg, scriptContext, &from))
+        {
+            if (JavascriptOperators::IsUndefinedOrNull(fromArg))
             {
-                if (JavascriptOperators::IsUndefinedOrNull(args[i]))
-                {
-                    continue;
-                }
-                JavascriptError::ThrowTypeError(scriptContext, JSERR_FunctionArgument_NeedObject, _u("Object.assign"));
+                return;
             }
+            JavascriptError::ThrowTypeError(scriptContext, JSERR_FunctionArgument_NeedObject, _u("Object.assign"));
+        }
 
 #if ENABLE_COPYONACCESS_ARRAY
-            JavascriptLibrary::CheckAndConvertCopyOnAccessNativeIntArray<Var>(from);
+        JavascriptLibrary::CheckAndConvertCopyOnAccessNativeIntArray<Var>(from);
 #endif
 
-            // if proxy, take slow path by calling [[OwnPropertyKeys]] on source
-            if (JavascriptProxy::Is(from))
+        // if proxy, take slow path by calling [[OwnPropertyKeys]] on source
+        if (JavascriptProxy::Is(from))
+        {
+            AssignForProxyObjects(from, to, scriptContext);
+        }
+        // else use enumerator to extract keys from source
+        else
+        {
+            bool copied = false;
+            if (tryCopy)
             {
-                AssignForProxyObjects(from, to, scriptContext);
+                DynamicObject* fromObj = JavascriptOperators::TryFromVar<DynamicObject>(from);
+                DynamicObject* toObj = JavascriptOperators::TryFromVar<DynamicObject>(to);
+                if (toObj && fromObj && toObj->GetType() == scriptContext->GetLibrary()->GetObjectType())
+                {
+                    copied = toObj->TryCopy(fromObj);
+                }
             }
-            // else use enumerator to extract keys from source
-            else
+            if (!copied)
             {
                 AssignForGenericObjects(from, to, scriptContext);
             }
         }
-
-        // 6. Return to.
-        return to;
     }
 
     void JavascriptObject::AssignForGenericObjects(RecyclableObject* from, RecyclableObject* to, ScriptContext* scriptContext)

lib/Runtime/Library/JavascriptObject.h

@@ -110,6 +110,8 @@ namespace Js
         static JavascriptString* ToStringTagHelper(Var thisArg, ScriptContext* scriptContext, TypeId type);
 
     private:
+        template <bool tryCopy>
+        static void AssignHelper(Var fromArg, RecyclableObject* to, ScriptContext* scriptContext);
         static void AssignForGenericObjects(RecyclableObject* from, RecyclableObject* to, ScriptContext* scriptContext);
         static void AssignForProxyObjects(RecyclableObject* from, RecyclableObject* to, ScriptContext* scriptContext);
         static JavascriptArray* CreateKeysHelper(RecyclableObject* object, ScriptContext* scriptContext, BOOL enumNonEnumerable, bool includeSymbolProperties, bool includeStringProperties, bool includeSpecialProperties);

lib/Runtime/Types/DynamicObject.cpp

@@ -58,8 +58,8 @@ namespace Js
         int propertyCount = typeHandler->GetPropertyCount();
         int inlineSlotCapacity = GetTypeHandler()->GetInlineSlotCapacity();
         int inlineSlotCount = min(inlineSlotCapacity, propertyCount);
-        Var * srcSlots = reinterpret_cast<Var*>(reinterpret_cast<size_t>(instance) + typeHandler->GetOffsetOfInlineSlots());
-        Field(Var) * dstSlots = reinterpret_cast<Field(Var)*>(reinterpret_cast<size_t>(this) + typeHandler->GetOffsetOfInlineSlots());
+        Field(Var)* srcSlots = instance->GetInlineSlots();
+        Field(Var)* dstSlots = this->GetInlineSlots();
 #if !FLOATVAR
         ScriptContext * scriptContext = this->GetScriptContext();
 #endif
@@ -755,6 +755,120 @@ namespace Js
         }
     }
 
+    Field(Var)* DynamicObject::GetInlineSlots() const
+    {
+        return reinterpret_cast<Field(Var)*>(reinterpret_cast<size_t>(this) + this->GetOffsetOfInlineSlots());
+    }
+
+    bool DynamicObject::IsCompatibleForCopy(DynamicObject* from) const
+    {
+        if (this->GetTypeHandler()->GetInlineSlotCapacity() != from->GetTypeHandler()->GetInlineSlotCapacity())
+        {
+            if (PHASE_TRACE1(ObjectCopyPhase))
+            {
+                Output::Print(_u("ObjectCopy: Can't copy: inline slot capacity doesn't match, from: %u, to: %u\n"),
+                    from->GetTypeHandler()->GetInlineSlotCapacity(),
+                    this->GetTypeHandler()->GetInlineSlotCapacity());
+            }
+            return false;
+        }
+        if (!from->GetTypeHandler()->IsPathTypeHandler())
+        {
+            if (PHASE_TRACE1(ObjectCopyPhase))
+            {
+                Output::Print(_u("ObjectCopy: Can't copy: Don't have PathTypeHandler\n"));
+            }
+            return false;
+        }
+        if (PathTypeHandlerBase::FromTypeHandler(from->GetTypeHandler())->HasAccessors())
+        {
+            if (PHASE_TRACE1(ObjectCopyPhase))
+            {
+                Output::Print(_u("ObjectCopy: Can't copy: type handler has accessors\n"));
+            }
+            return false;
+        }
+        if (this->GetPrototype() != from->GetPrototype())
+        {
+            if (PHASE_TRACE1(ObjectCopyPhase))
+            {
+                Output::Print(_u("ObjectCopy: Can't copy: Protoytypes don't match\n"));
+            }
+            return false;
+        }
+        if (!from->GetTypeHandler()->AllPropertiesAreEnumerable())
+        {
+            if (PHASE_TRACE1(ObjectCopyPhase))
+            {
+                Output::Print(_u("ObjectCopy: Can't copy: from obj has non-enumerable properties\n"));
+            }
+            return false;
+        }
+        if (from->IsExternal())
+        {
+            if (PHASE_TRACE1(ObjectCopyPhase))
+            {
+                Output::Print(_u("ObjectCopy: Can't copy: from obj is External\n"));
+            }
+            return false;
+        }
+        if (this->GetScriptContext() != from->GetScriptContext())
+        {
+            if (PHASE_TRACE1(ObjectCopyPhase))
+            {
+                Output::Print(_u("ObjectCopy: Can't copy: from obj is from different ScriptContext\n"));
+            }
+            return false;
+        }
+
+        return true;
+    }
+
+    bool DynamicObject::TryCopy(DynamicObject* from)
+    {
+        // Validate that objects are compatible
+        if (!this->IsCompatibleForCopy(from))
+        {
+            return false;
+        }
+        // Share the type
+        // Note: this will mark type as shared in case of success
+        if (!from->GetDynamicType()->ShareType())
+        {
+            if (PHASE_TRACE1(ObjectCopyPhase))
+            {
+                Output::Print(_u("ObjectCopy: Can't copy: failed to share type\n"));
+            }
+            return false;
+        }
+
+        // Update this object
+        this->ReplaceType(from->GetDynamicType());
+        this->InitSlots(this);
+        const int slotCapacity = this->GetTypeHandler()->GetSlotCapacity();
+        const uint16 inlineSlotCapacity = this->GetTypeHandler()->GetInlineSlotCapacity();
+        const int auxSlotCapacity = slotCapacity - inlineSlotCapacity;
+
+        if (auxSlotCapacity > 0)
+        {
+            CopyArray(this->auxSlots, auxSlotCapacity, from->auxSlots, auxSlotCapacity);
+        }
+        if (inlineSlotCapacity != 0)
+        {
+            Field(Var)* thisInlineSlots = this->GetInlineSlots();
+            Field(Var)* fromInlineSlots = from->GetInlineSlots();
+
+            CopyArray(thisInlineSlots, inlineSlotCapacity, fromInlineSlots, inlineSlotCapacity);
+        }
+
+        if (PHASE_TRACE1(ObjectCopyPhase))
+        {
+            Output::Print(_u("ObjectCopy succeeded\n"));
+        }
+
+        return true;
+    }
+
     bool
     DynamicObject::GetHasNoEnumerableProperties()
     {
@@ -892,10 +1006,9 @@ namespace Js
         TTD::NSSnapObjects::StdExtractSetKindSpecificInfo<void*, TTD::NSSnapObjects::SnapObjectType::SnapDynamicObject>(objData, nullptr);
     }
 
-    Js::Var const* DynamicObject::GetInlineSlots_TTD() const
+    Field(Js::Var) const* DynamicObject::GetInlineSlots_TTD() const
     {
-        return reinterpret_cast<Var const*>(
-            reinterpret_cast<size_t>(this) + this->GetTypeHandler()->GetOffsetOfInlineSlots());
+        return this->GetInlineSlots();
     }
 
     Js::Var const* DynamicObject::GetAuxSlots_TTD() const

lib/Runtime/Types/DynamicObject.h

@@ -106,6 +106,8 @@ namespace Js
         void InitSlots(DynamicObject * instance, ScriptContext * scriptContext);
         void SetTypeHandler(DynamicTypeHandler * typeHandler, bool hasChanged);
 
+        Field(Var)* GetInlineSlots() const;
+
     protected:
         DEFINE_VTABLE_CTOR(DynamicObject, RecyclableObject);
         DEFINE_MARSHAL_OBJECT_TO_SCRIPT_CONTEXT(DynamicObject);
@@ -138,7 +140,6 @@ namespace Js
         Var GetSlot(int index);
         Var GetInlineSlot(int index);
         Var GetAuxSlot(int index);
-
 #if DBG
         void SetSlot(PropertyId propertyId, bool allowLetConst, int index, Var value);
         void SetInlineSlot(PropertyId propertyId, bool allowLetConst, int index, Var value);
@@ -150,6 +151,8 @@ namespace Js
 #endif
 
     private:
+        bool IsCompatibleForCopy(DynamicObject* from) const;
+
         bool IsObjectHeaderInlinedTypeHandlerUnchecked() const;
     public:
         bool IsObjectHeaderInlinedTypeHandler() const;
@@ -204,6 +207,8 @@ namespace Js
         void InvalidateHasOnlyWritableDataPropertiesInPrototypeChainCacheIfPrototype();
         void ResetObject(DynamicType* type, BOOL keepProperties);
 
+        bool TryCopy(DynamicObject* from);
+
         virtual void SetIsPrototype();
 
         bool HasLockedType() const;
@@ -338,7 +343,7 @@ namespace Js
         virtual TTD::NSSnapObjects::SnapObjectType GetSnapTag_TTD() const override;
         virtual void ExtractSnapObjectDataInto(TTD::NSSnapObjects::SnapObject* objData, TTD::SlabAllocator& alloc) override;
 
-        Js::Var const* GetInlineSlots_TTD() const;
+        Field(Js::Var) const* GetInlineSlots_TTD() const;
         Js::Var const* GetAuxSlots_TTD() const;
 
 #if ENABLE_OBJECT_SOURCE_TRACKING

test/Object/assign.baseline

@@ -0,0 +1,8 @@
+ObjectCopy succeeded
+ObjectCopy: Can't copy: Don't have PathTypeHandler
+ObjectCopy: Can't copy: type handler has accessors
+ObjectCopy: Can't copy: type handler has accessors
+ObjectCopy: Can't copy: Protoytypes don't match
+ObjectCopy: Can't copy: from obj has non-enumerable properties
+ObjectCopy succeeded
+pass

test/Object/assign.js

@@ -0,0 +1,111 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+WScript.LoadScriptFile("..\\UnitTestFramework\\UnitTestFramework.js");
+
+var tests = [
+    {
+        name: "simple copy",
+        body: function ()
+        {
+            let orig = {};
+            let sym = Symbol("c");
+            orig.a = 1;
+            orig.b = "asdf";
+            orig[sym] = "qwert";
+            let newObj = Object.assign({}, orig);
+            assert.areEqual(newObj.b, orig.b);
+            assert.areEqual(newObj.a, orig.a);
+            assert.areEqual(newObj[sym], orig[sym]);
+        }
+    },
+    {
+        name: "non-path type handler",
+        body: function ()
+        {
+            let orig = {};
+            orig.a = 1;
+            orig.b = "asdf";
+            delete orig.a;
+            let newObj = Object.assign({}, orig);
+            assert.areEqual(newObj.b, orig.b);
+            assert.areEqual(newObj.a, orig.a);
+        }
+    },
+    {
+        name: "has getter",
+        body: function ()
+        {
+            let orig = {};
+            orig.a = 1;
+            Object.defineProperty(orig, 'b', {
+                get: function() { return "asdf"; }, enumerable: true
+              });
+            let newObj = Object.assign({}, orig);
+            assert.areEqual(newObj.b, orig.b);
+            assert.areEqual(newObj.a, orig.a);
+        }
+    },
+    {
+        name: "has setter",
+        body: function ()
+        {
+            let orig = {};
+            orig.a = 1;
+            Object.defineProperty(orig, 'b', {
+                set: function() {  }, enumerable: true
+              });
+            let newObj = Object.assign({}, orig);
+            assert.areEqual(newObj.b, orig.b);
+            assert.areEqual(newObj.a, orig.a);
+        }
+    },
+    {
+        name: "different proto",
+        body: function ()
+        {
+            let protoObj = {};
+            let orig = Object.create(protoObj);
+            orig.a = 1;
+            orig.b = "asdf";
+            
+            let newObj = Object.assign({}, orig);
+            assert.areEqual(newObj.b, orig.b);
+            assert.areEqual(newObj.a, orig.a);
+        }
+    },
+    {
+        name: "non-enumerable",
+        body: function ()
+        {
+            let orig = {};
+            orig.a = 1;
+            Object.defineProperty(orig, 'b', {
+                value: "asdf", enumerable: false
+              });
+            
+            let newObj = Object.assign({}, orig);
+            assert.areEqual(newObj.b, undefined);
+            assert.areEqual(newObj.a, orig.a);
+        }
+    },
+    {
+        name: "proto accessor",
+        body: function ()
+        {
+            Object.defineProperty(Object.prototype, 'b', {
+                get: function() { return "asdf"; }
+              });
+            let orig = {};
+            orig.a = 1;
+            
+            let newObj = Object.assign({}, orig);
+            assert.areEqual(newObj.b, "asdf");
+            assert.areEqual(newObj.a, orig.a);
+        }
+    }
+];
+
+testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });

test/Object/rlexe.xml

@@ -436,4 +436,11 @@
       <baseline />
     </default>
   </test>
+  <test>
+    <default>
+      <files>assign.js</files>
+      <compile-flags>-args summary -endargs -trace:ObjectCopy</compile-flags>
+      <baseline>assign.baseline</baseline>
+    </default>
+  </test>
 </regress-exe>