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.

Sign up for GitHub

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

Jump to bottom

Improve JSON.stringify perf for TypedArrays #4831

Merged
merged 2 commits into from
Mar 19, 2018
Merged

Improve JSON.stringify perf for TypedArrays #4831

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
45caec1
lookup properties by numeric index for typed arrays during stringify
MikeHolman Mar 16, 2018
1e918e2
fix UT failure; remove unused method
MikeHolman Mar 16, 2018
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
82 changes: 56 additions & 26 deletions lib/Runtime/Library/JSONStringifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,6 @@ JSONStringifier::Stringify(_In_ ScriptContext* scriptContext, _In_ Var value, _I
wrapper,
prop,
value,
scriptContext->GetThreadContext()->GetEmptyStringPropertyRecord(),
&objStack);

if (prop->type == JSONContentType::Undefined)
Expand Down Expand Up @@ -340,7 +339,7 @@ JSONStringifier::ReadArrayElement(uint32 index, _In_ RecyclableObject* arr, _Out
JavascriptOperators::GetItem(arr, index, &value, this->scriptContext);
}
JavascriptString* indexString = this->scriptContext->GetIntegerString(index);
this->ReadProperty(indexString, arr, prop, value, nullptr, objectStack);
this->ReadProperty(indexString, arr, prop, value, objectStack);
}

JSONArray*
Expand Down Expand Up @@ -386,19 +385,13 @@ JSONStringifier::ReadArray(_In_ RecyclableObject* arr, _In_ JSONObjectStack* obj
}

void
JSONStringifier::ReadObjectElement(
JSONStringifier::AppendObjectElement(
_In_ JavascriptString* propertyName,
_In_opt_ PropertyRecord const* propertyRecord,
_In_ RecyclableObject* obj,
_In_ JSONObject* jsonObject,
_In_ JSONObjectStack* objectStack)
_In_ JSONObjectProperty* prop)
{
JSONObjectProperty prop;
prop.propertyName = propertyName;
this->ReadProperty(propertyName, obj, &prop.propertyValue, nullptr, propertyRecord, objectStack);

// Undefined result is not concatenated
if (prop.propertyValue.type != JSONContentType::Undefined)
if (prop->propertyValue.type != JSONContentType::Undefined)
{
// Increase length for the name of the property
this->totalStringLength = UInt32Math::Add(this->totalStringLength, CalculateStringElementLength(propertyName));
Expand All @@ -409,10 +402,47 @@ JSONStringifier::ReadObjectElement(
// If gap is specified, a space is appended
UInt32Math::Inc(this->totalStringLength);
}
jsonObject->Push(prop);

jsonObject->Push(*prop);
}
}

void
JSONStringifier::ReadObjectElement(
_In_ JavascriptString* propertyName,
_In_ uint32 numericIndex,
_In_ RecyclableObject* obj,
_In_ JSONObject* jsonObject,
_In_ JSONObjectStack* objectStack)
{
JSONObjectProperty prop;
prop.propertyName = propertyName;

Var value = JavascriptOperators::GetItem(obj, numericIndex, this->scriptContext);
Copy link
Contributor

Choose a reason for hiding this comment

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

:: [](start = 35, length = 2)

Fantastic wins.

So if I understand - obj is typed array, so this will call to the DirectGetItem of typed array which will have unnecessary checks being detached and others . Could that be avoided (or it is already avoided)?


this->ReadProperty(propertyName, obj, &prop.propertyValue, value, objectStack);

this->AppendObjectElement(propertyName, jsonObject, &prop);
}

void
JSONStringifier::ReadObjectElement(
_In_ JavascriptString* propertyName,
_In_opt_ PropertyRecord const* propertyRecord,
_In_ RecyclableObject* obj,
_In_ JSONObject* jsonObject,
_In_ JSONObjectStack* objectStack)
{
JSONObjectProperty prop;
prop.propertyName = propertyName;

Var value = this->ReadValue(propertyName, propertyRecord, obj);

this->ReadProperty(propertyName, obj, &prop.propertyValue, value, objectStack);

this->AppendObjectElement(propertyName, jsonObject, &prop);
}

// Calculates how many additional characters are needed for printing the Object/Array structure
// This includes commas, brackets, and gap (if any)
void
Expand Down Expand Up @@ -520,18 +550,25 @@ JSONStringifier::ReadObject(_In_ RecyclableObject* obj, _In_ JSONObjectStack* ob
JavascriptStaticEnumerator enumerator;
if (obj->GetEnumerator(&enumerator, EnumeratorFlags::SnapShotSemantics | EnumeratorFlags::EphemeralReference, this->scriptContext))
{
enumerator.GetInitialPropertyCount();
Copy link
Contributor

@curtisman curtisman Mar 16, 2018 •
edited by MikeHolman
Loading

Choose a reason for hiding this comment

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

You might want to clean up the implementation in JAvascriptStaticEnumerator.h and DynamicObjectPropertyEnumerator.h as well, as I don't see other references to those. #Resolved

JavascriptString* propertyName = nullptr;
PropertyId nextKey = Constants::NoProperty;
while ((propertyName = enumerator.MoveAndGetNext(nextKey)) != nullptr)
{
PropertyRecord const * propertyRecord = nullptr;
if (nextKey == Constants::NoProperty)
const uint32 numericIndex = enumerator.GetCurrentItemIndex();
if (numericIndex != Constants::InvalidSourceIndex)
{
this->ReadObjectElement(propertyName, numericIndex, obj, jsonObject, &stack);
}
else
{
this->scriptContext->GetOrAddPropertyRecord(propertyName, &propertyRecord);
nextKey = propertyRecord->GetPropertyId();
PropertyRecord const * propertyRecord = nullptr;
if (nextKey == Constants::NoProperty)
{
this->scriptContext->GetOrAddPropertyRecord(propertyName, &propertyRecord);
nextKey = propertyRecord->GetPropertyId();
}
this->ReadObjectElement(propertyName, propertyRecord, obj, jsonObject, &stack);
}
this->ReadObjectElement(propertyName, propertyRecord, obj, jsonObject, &stack);
}
}
}
Expand Down Expand Up @@ -728,17 +765,10 @@ JSONStringifier::ReadProperty(
_In_ JavascriptString* key,
_In_opt_ RecyclableObject* holder,
_Out_ JSONProperty* prop,
_In_opt_ Var value,
_In_opt_ const PropertyRecord* propertyRecord,
_In_ Var value,
_In_ JSONObjectStack* objectStack)
{
PROBE_STACK(this->scriptContext, Constants::MinStackDefault);
if (value == nullptr)
{
// If we don't have a value, we must have an object from which we can read it
AnalysisAssert(holder != nullptr);
value = this->ReadValue(key, propertyRecord, holder);
}

// Save these to avoid having to recheck conditions unless value is modified by ToJSON or a replacer function
RecyclableObject* valueObj = JavascriptOperators::TryFromVar<RecyclableObject>(value);
Expand Down
15 changes: 13 additions & 2 deletions lib/Runtime/Library/JSONStringifier.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,18 @@ class JSONStringifier
uint32 ReadArrayLength(_In_ RecyclableObject* value);
JSONArray* ReadArray(_In_ RecyclableObject* arr, _In_ JSONObjectStack* objectStack);

void AppendObjectElement(
_In_ JavascriptString* propertyName,
_In_ JSONObject* jsonObject,
_In_ JSONObjectProperty* prop);

void ReadObjectElement(
_In_ JavascriptString* propertyName,
_In_ uint32 numericIndex,
_In_ RecyclableObject* obj,
_In_ JSONObject* jsonObject,
_In_ JSONObjectStack* objectStack);

void ReadObjectElement(
_In_ JavascriptString* propertyName,
_In_opt_ PropertyRecord const* propertyRecord,
Expand Down Expand Up @@ -95,8 +107,7 @@ class JSONStringifier
_In_ JavascriptString* key,
_In_opt_ RecyclableObject* holder,
_Out_ JSONProperty* prop,
_In_opt_ Var value,
_In_opt_ const PropertyRecord* propertyRecord,
_In_ Var value,
_In_ JSONObjectStack* objectStack);

const char16* GetGap() const { return this->gap; };
Expand Down
1 change: 1 addition & 0 deletions lib/Runtime/Library/JavascriptStringEnumerator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,6 @@ namespace Js
JavascriptStringEnumerator(JavascriptString* stringObject, ScriptContext * requestContext);
virtual void Reset() override;
virtual JavascriptString * MoveAndGetNext(PropertyId& propertyId, PropertyAttributes* attributes = nullptr) override;
virtual uint32 GetCurrentItemIndex() override { return index; }
};
}
1 change: 1 addition & 0 deletions lib/Runtime/Library/TypedArrayIndexEnumerator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,6 @@ namespace Js
TypedArrayIndexEnumerator(TypedArrayBase* typeArrayBase, EnumeratorFlags flags, ScriptContext* scriptContext);
virtual JavascriptString * MoveAndGetNext(PropertyId& propertyId, PropertyAttributes* attributes = nullptr) override;
virtual void Reset() override;
virtual uint32 GetCurrentItemIndex() override { return index; }
};
}
1 change: 0 additions & 1 deletion lib/Runtime/Types/DynamicObjectPropertyEnumerator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ namespace Js
bool GetSnapShotSemantics() const;
bool GetUseCache() const;
ScriptContext * GetScriptContext() const { return scriptContext; }
BigPropertyIndex GetInitialPropertyCount() const { return this->initialPropertyCount; }

bool Initialize(DynamicObject * object, EnumeratorFlags flags, ScriptContext * requestContext, ForInCache * forInCache);
bool IsNullEnumerator() const;
Expand Down
1 change: 0 additions & 1 deletion lib/Runtime/Types/JavascriptStaticEnumerator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ namespace Js
void Reset();
uint32 GetCurrentItemIndex();
JavascriptString * MoveAndGetNext(PropertyId& propertyId, PropertyAttributes* attributes = nullptr);
BigPropertyIndex GetInitialPropertyCount() const { return this->propertyEnumerator.GetInitialPropertyCount(); }

static uint32 GetOffsetOfCurrentEnumerator() { return offsetof(JavascriptStaticEnumerator, currentEnumerator); }
static uint32 GetOffsetOfPrefixEnumerator() { return offsetof(JavascriptStaticEnumerator, prefixEnumerator); }
Expand Down