Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

optimize object.assign({}, obj) #4817

Merged
merged 5 commits into from
Mar 21, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
}
Copy link
Collaborator

@agarwal-sandeep agarwal-sandeep Mar 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add test cases for these ifs and boundary cases for aux and inline slot capacities? #Resolved


// 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 @@ -114,6 +114,8 @@ namespace Js
void ReplaceType(DynamicType * type);
void ReplaceTypeWithPredecessorType(DynamicType * previousType);

Field(Var)* GetInlineSlots() const;

protected:
DEFINE_VTABLE_CTOR(DynamicObject, RecyclableObject);
DEFINE_MARSHAL_OBJECT_TO_SCRIPT_CONTEXT(DynamicObject);
Expand Down Expand Up @@ -144,7 +146,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 @@ -156,6 +157,8 @@ namespace Js
#endif

private:
bool IsCompatibleForCopy(DynamicObject* from) const;

bool IsObjectHeaderInlinedTypeHandlerUnchecked() const;
public:
bool IsObjectHeaderInlinedTypeHandler() const;
Expand Down Expand Up @@ -210,6 +213,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 @@ -344,7 +349,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