-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Improve JSON.stringify perf for TypedArrays #4831
Conversation
@@ -520,18 +552,25 @@ JSONStringifier::ReadObject(_In_ RecyclableObject* obj, _In_ JSONObjectStack* ob | |||
JavascriptStaticEnumerator enumerator; | |||
if (obj->GetEnumerator(&enumerator, EnumeratorFlags::SnapShotSemantics | EnumeratorFlags::EphemeralReference, this->scriptContext)) | |||
{ | |||
enumerator.GetInitialPropertyCount(); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JSONObjectProperty prop; | ||
prop.propertyName = propertyName; | ||
|
||
Var value = JavascriptOperators::GetItem(obj, numericIndex, this->scriptContext); |
There was a problem hiding this comment.
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)?
It's because spec has very different way they serialize normal arrays vs. other objects. Normal arrays only serialize elements up to length, but typed arrays will need to serialize all properties (e.g. if i say typedArr.prop = "blah" we need to serialize that). So I'd rather just use the normal enumerator rather than trying to be fancy In reply to: 373793434 [](ancestors = 373793434,373783535) Refers to: lib/Runtime/Library/JSONStringifier.cpp:811 in 45caec1. [](commit_id = 45caec1, deletion_comment = False) |
Merge pull request #4831 from MikeHolman:arrenum During enumeration, we can (and should) avoid creating PropertyRecords for TypedArrays. Instead we can use the numeric index. In the below micro-benchmark, this improves our perf by about 45%. ```` const view = new Int8Array(1 << 24); for (let i = 0; i < view.length; ++i) { view[i] = i|0; } var start = Date.now(); const str = JSON.stringify(view); console.log(Date.now() - start); ```` OS: 16385822
During enumeration, we can (and should) avoid creating PropertyRecords for TypedArrays. Instead we can use the numeric index.
In the below micro-benchmark, this improves our perf by about 45%.
OS: 16385822