Skip to content
This repository has been archived by the owner on Oct 15, 2020. It is now read-only.

Commit

Permalink
[Merge chakra-core/ChakraCore@c6b3f201b1] [MERGE #3751 @obastemur] pe…
Browse files Browse the repository at this point in the history
…rf: Improve JSON.stringify performance

Merge pull request #3751 from obastemur:fjs

Kraken/json-stringify-* perf ~25% better. Acme Air - LTO gain ~1.5%

PR Details:

- Improve `replacer != function && !HasObjectArray` case. (most common use of JSON.stringify)

- Add JSON.stringify test cases for ObjectArray, toJSON, and replacer function

- IsNumericPropertyId: fast path for internal properties

- use requestContext instead of instance->scriptContext

- use wmemcpy instead of memcpy
  • Loading branch information
chakrabot authored and kfarnung committed Jan 9, 2018
1 parent 2a7d89c commit 230ecbd
Show file tree
Hide file tree
Showing 12 changed files with 220 additions and 84 deletions.
1 change: 1 addition & 0 deletions deps/chakrashim/core/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ tags
Makefile
pal/src/config.h
DbgController.js.h
lib/wabt/built/config.h

# Generated by other tools
*.lldb.cmd
Expand Down
6 changes: 4 additions & 2 deletions deps/chakrashim/core/lib/Common/Core/Api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
//-------------------------------------------------------------------------------------------------------
#include "CommonCorePch.h"
#ifdef _WIN32
#include <wchar.h> // wmemcpy_s
#endif

void __stdcall js_memcpy_s(__bcount(sizeInBytes) void *dst, size_t sizeInBytes, __in_bcount(count) const void *src, size_t count)
{
Expand All @@ -21,8 +24,7 @@ void __stdcall js_wmemcpy_s(__ecount(sizeInWords) char16 *dst, size_t sizeInWord
{
Js::Throw::FatalInternalError();
}

memcpy(dst, src, count * sizeof(char16));
wmemcpy_s(dst, count, src, count);
}

#if defined(_M_IX86) || defined(_M_X64)
Expand Down
5 changes: 5 additions & 0 deletions deps/chakrashim/core/lib/Runtime/Base/ThreadContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1235,6 +1235,11 @@ void ThreadContext::AddBuiltInPropertyRecord(const Js::PropertyRecord *propertyR

BOOL ThreadContext::IsNumericPropertyId(Js::PropertyId propertyId, uint32* value)
{
if (Js::IsInternalPropertyId(propertyId))
{
return false;
}

Js::PropertyRecord const * propertyRecord = this->GetPropertyName(propertyId);
Assert(propertyRecord != nullptr);
if (propertyRecord == nullptr || !propertyRecord->IsNumeric())
Expand Down
169 changes: 117 additions & 52 deletions deps/chakrashim/core/lib/Runtime/Library/JSON.cpp

Large diffs are not rendered by default.

8 changes: 5 additions & 3 deletions deps/chakrashim/core/lib/Runtime/Library/JSON.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ namespace JSON
}
void CompleteInit(Js::Var space, ArenaAllocator* alloc);

Js::Var Str(Js::JavascriptString* key, Js::PropertyId keyId, Js::Var holder);
Js::Var Str(Js::JavascriptString* key, Js::PropertyId keyId, Js::Var holder, Js::Var value = nullptr);
Js::Var Str(uint32 index, Js::Var holder);

private:
Expand All @@ -74,8 +74,10 @@ namespace JSON
Js::JavascriptString* GetPropertySeparator();
Js::JavascriptString* GetIndentString(uint count);
Js::JavascriptString* GetMemberSeparator(Js::JavascriptString* indentString);
void StringifyMemberObject( Js::JavascriptString* propertyName, Js::PropertyId id, Js::Var value, Js::ConcatStringBuilder* result,
Js::JavascriptString* &indentString, Js::JavascriptString* &memberSeparator, bool &isFirstMember, bool &isEmpty );
void StringifyMemberObject(Js::JavascriptString* propertyName, Js::PropertyId id,
Js::Var value, Js::ConcatStringBuilder* result, Js::JavascriptString* &indentString,
Js::JavascriptString* &memberSeparator, bool &isFirstMember, bool &isEmpty,
Js::Var propertyValue = nullptr );

uint32 GetPropertyCount(Js::RecyclableObject* object, Js::JavascriptStaticEnumerator* enumerator);
uint32 GetPropertyCount(Js::RecyclableObject* object, Js::JavascriptStaticEnumerator* enumerator, bool* isPrecise);
Expand Down
3 changes: 2 additions & 1 deletion deps/chakrashim/core/lib/Runtime/Types/DynamicObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ namespace Js
// For boxing stack instance
DynamicObject(DynamicObject * instance);

DynamicTypeHandler * GetTypeHandler() const;
uint16 GetOffsetOfInlineSlots() const;

template <class T>
Expand All @@ -136,6 +135,8 @@ namespace Js
void EnsureSlots(int oldCount, int newCount, ScriptContext * scriptContext, DynamicTypeHandler * newTypeHandler = nullptr);
void EnsureSlots(int newCount, ScriptContext *scriptContext);

DynamicTypeHandler * GetTypeHandler() const;

Var GetSlot(int index);
Var GetInlineSlot(int index);
Var GetAuxSlot(int index);
Expand Down
3 changes: 1 addition & 2 deletions deps/chakrashim/core/lib/Runtime/Types/PathTypeHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,7 @@ namespace Js

// Check numeric propertyId only if objectArray available
uint32 indexVal;
ScriptContext* scriptContext = instance->GetScriptContext();
if (instance->HasObjectArray() && scriptContext->IsNumericPropertyId(propertyId, &indexVal))
if (instance->HasObjectArray() && requestContext->IsNumericPropertyId(propertyId, &indexVal))
{
return PathTypeHandlerBase::GetItem(instance, originalInstance, indexVal, value, requestContext);
}
Expand Down
39 changes: 39 additions & 0 deletions deps/chakrashim/core/test/JSON/replacerFunction.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
//-------------------------------------------------------------------------------------------------------
// Copyright (C) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
//-------------------------------------------------------------------------------------------------------


var TEST = function(a, b) {
if (a != b) {
throw new Error(a + " != " + b);
}
}

var obj = { str:6 };
obj[0] = 'value0'
obj[6] = 'value6';
TEST(JSON.stringify(obj, function(k, v) {
if (!k) return v;
return v + 1
}), '{"0":"value01","6":"value61","str":7}');

// test ObjectArray
TEST(JSON.stringify({0:0, 1:1, "two":2}), '{"0":0,"1":1,"two":2}')

var a = new Object();

function replacer(k, v)
{
return v;
}

var until = (!WScript.Platform || WScript.Platform.BUILD_TYPE == 'Debug') ? 12 : 1290;
for (var i = 0; i < until; i++)
{
a[i + 10] = 0;
}

TEST(JSON.stringify(a, replacer).substring(0,20), '{"10":0,"11":0,"12":');

console.log("PASS")
10 changes: 6 additions & 4 deletions deps/chakrashim/core/test/JSON/rlexe.xml
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,7 @@
</test>
<test>
<default>
<files>stringify-replacerfunc.js</files>
<baseline>stringify-replacerfunc.baseline</baseline>
<compile-flags>-recyclerstress</compile-flags>
<tags>exclude_fre,Slow</tags>
<files>replacerFunction.js</files>
<timeout>300</timeout>
</default>
</test>
Expand Down Expand Up @@ -95,4 +92,9 @@
<baseline>syntaxError.baseline</baseline>
</default>
</test>
<test>
<default>
<files>toJSON.js</files>
</default>
</test>
</regress-exe>

This file was deleted.

19 changes: 0 additions & 19 deletions deps/chakrashim/core/test/JSON/stringify-replacerfunc.js

This file was deleted.

40 changes: 40 additions & 0 deletions deps/chakrashim/core/test/JSON/toJSON.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
//-------------------------------------------------------------------------------------------------------
// Copyright (C) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
//-------------------------------------------------------------------------------------------------------


var TEST = function(a, b) {
if (a != b) {
throw new Error(a + " != " + b);
}
}

var fnc = function(n) { this.number = n };
fnc.prototype.toJSON = function() {
return this.number.toString();
}

// test - function prototype new instance
TEST("\"1\"", JSON.stringify(new fnc(1)))

// test - pre-post alter Date toJSON definition
var dateString = JSON.stringify(new Date(0));
TEST("1970", dateString.substr(dateString.indexOf("1970"), 4))

Date.prototype.toJSON = 1;
TEST("{}", JSON.stringify(new Date(0)))

delete Date.prototype.toJSON
TEST("{}", JSON.stringify(new Date(0)))

// test - use from Object prototype
Object.prototype.toJSON = function() { return 2; }
delete fnc.prototype.toJSON;
TEST(2, JSON.stringify(new fnc(1)))

// test - symbol
Object.prototype[Symbol("toJSON")] = function() { return 3; }
TEST(2, JSON.stringify(new fnc(1)))

console.log("PASS")

0 comments on commit 230ecbd

Please sign in to comment.