Skip to content

Commit

Permalink
[MERGE #4817 @MikeHolman] optimize object.assign({}, obj)
Browse files Browse the repository at this point in the history
Merge pull request #4817 from MikeHolman:objassign

In case of simple object.assign({}, obj) we can memcpy the properties over to the new object.
  • Loading branch information
MikeHolman committed Mar 21, 2018
2 parents e8c4ae6 + 9273771 commit 5a11b38
Show file tree
Hide file tree
Showing 9 changed files with 298 additions and 31 deletions.
1 change: 1 addition & 0 deletions lib/Common/ConfigFlagsList.h
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,7 @@ PHASE(All)
PHASE(Error)
PHASE(PropertyRecord)
PHASE(TypePathDynamicSize)
PHASE(ObjectCopy)
PHASE(ShareTypesWithAttributes)
PHASE(ShareAccessorTypes)
PHASE(ConditionalCompilation)
Expand Down
2 changes: 1 addition & 1 deletion lib/Runtime/Debug/TTSnapObjects.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
66 changes: 43 additions & 23 deletions lib/Runtime/Library/JavascriptObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions lib/Runtime/Library/JavascriptObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
123 changes: 118 additions & 5 deletions lib/Runtime/Types/DynamicObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
{
Expand Down Expand Up @@ -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
Expand Down
9 changes: 7 additions & 2 deletions lib/Runtime/Types/DynamicObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -150,6 +151,8 @@ namespace Js
#endif

private:
bool IsCompatibleForCopy(DynamicObject* from) const;

bool IsObjectHeaderInlinedTypeHandlerUnchecked() const;
public:
bool IsObjectHeaderInlinedTypeHandler() const;
Expand Down Expand Up @@ -204,6 +207,8 @@ namespace Js
void InvalidateHasOnlyWritableDataPropertiesInPrototypeChainCacheIfPrototype();
void ResetObject(DynamicType* type, BOOL keepProperties);

bool TryCopy(DynamicObject* from);

virtual void SetIsPrototype();

bool HasLockedType() const;
Expand Down Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions test/Object/assign.baseline
Original file line number Diff line number Diff line change
@@ -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
Loading

0 comments on commit 5a11b38

Please sign in to comment.