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 JSON #4077

Merged
merged 4 commits into from
Nov 3, 2017
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
11 changes: 0 additions & 11 deletions lib/Backend/JITTimeFunctionBody.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -957,17 +957,6 @@ JITTimeFunctionBody::GetConstTable() const
return m_bodyData.constTable;
}

bool
JITTimeFunctionBody::IsConstRegPropertyString(Js::RegSlot reg, ScriptContextInfo * context) const
{
RecyclableObjectIDL * content = m_bodyData.constTableContent->content[reg - Js::FunctionBody::FirstRegSlot];
if (content != nullptr && content->vtbl == context->GetVTableAddress(VtablePropertyString))
{
return true;
}
return false;
}

intptr_t
JITTimeFunctionBody::GetRootObject() const
{
Expand Down
1 change: 0 additions & 1 deletion lib/Backend/JITTimeFunctionBody.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@ class JITTimeFunctionBody
intptr_t GetIsInstInlineCache(uint index) const;
Js::TypeId GetConstantType(Js::RegSlot location) const;
void * GetConstTable() const;
bool IsConstRegPropertyString(Js::RegSlot reg, ScriptContextInfo * context) const;

intptr_t GetRootObject() const;
intptr_t GetLoopHeaderAddr(uint loopNum) const;
Expand Down
2 changes: 1 addition & 1 deletion lib/Backend/JnHelperMethodList.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ HELPERCALL(ScrObj_LdStrictInnerFrameDisplayNoParent, Js::JavascriptOperators::OP
HELPERCALL(ScrObj_OP_IsInst, Js::JavascriptOperators::OP_IsInst, AttrCanThrow)

HELPERCALL(Op_IsIn, Js::JavascriptOperators::IsIn, AttrCanThrow)
HELPERCALL(Op_IsObject, Js::JavascriptOperators::IsObject, AttrCanThrow)
HELPERCALL(Op_IsObject, (BOOL (*) (Js::Var))Js::JavascriptOperators::IsObject, AttrCanThrow)
HELPERCALL(Op_IsClassConstructor, Js::JavascriptOperators::IsClassConstructor, AttrCanThrow)
HELPERCALL(Op_IsBaseConstructorKind, Js::JavascriptOperators::IsBaseConstructorKind, AttrCanThrow)
HELPERCALL(Op_LoadHeapArguments, Js::JavascriptOperators::LoadHeapArguments, 0)
Expand Down
1 change: 1 addition & 0 deletions lib/Common/BackendApi.h
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ enum VTableValue {
VtableDynamicObject,
VtableInvalid,
VtablePropertyString,
VtableLazyJSONString,
VtableLiteralStringWithPropertyStringPtr,
VtableJavascriptBoolean,
VtableJavascriptArray,
Expand Down
2 changes: 1 addition & 1 deletion lib/JITIDL/JITTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ typedef unsigned char boolean;
#define __JITTypes_h__

// TODO: OOP JIT, how do we make this better?
const int VTABLE_COUNT = 48;
const int VTABLE_COUNT = 49;
Copy link
Contributor

@digitalinfinity digitalinfinity Oct 26, 2017

Choose a reason for hiding this comment

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

Huh- didn't know this existed...is there a compile assert or test that exists to remind us to update this? #Resolved

Copy link
Contributor Author

@MikeHolman MikeHolman Oct 27, 2017

Choose a reason for hiding this comment

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

Yeah there is a compile assert #Closed

const int EQUIVALENT_TYPE_CACHE_SIZE = 8;

typedef IDL_DEF([context_handle]) void * PTHREADCONTEXT_HANDLE;
Expand Down
19 changes: 11 additions & 8 deletions lib/Runtime/Base/Constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,14 +155,17 @@ namespace Js
static const char16 StringReplace[];
static const char16 StringMatch[];

static const size_t AnonymousFunctionLength = _countof(_u("Anonymous function")) - 1;
static const size_t AnonymousLength = _countof(_u("anonymous")) - 1;
static const size_t AnonymousClassLength = _countof(_u("Anonymous class")) - 1;
static const size_t FunctionCodeLength = _countof(_u("Function code")) - 1;
static const size_t GlobalFunctionLength = _countof(_u("glo")) - 1;
static const size_t GlobalCodeLength = _countof(_u("Global code")) - 1;
static const size_t EvalCodeLength = _countof(_u("eval code")) - 1;
static const size_t UnknownScriptCodeLength = _countof(_u("Unknown script code")) - 1;
static const charcount_t AnonymousFunctionLength = _countof(_u("Anonymous function")) - 1;
static const charcount_t AnonymousLength = _countof(_u("anonymous")) - 1;
static const charcount_t AnonymousClassLength = _countof(_u("Anonymous class")) - 1;
static const charcount_t FunctionCodeLength = _countof(_u("Function code")) - 1;
static const charcount_t GlobalFunctionLength = _countof(_u("glo")) - 1;
static const charcount_t GlobalCodeLength = _countof(_u("Global code")) - 1;
static const charcount_t EvalCodeLength = _countof(_u("eval code")) - 1;
static const charcount_t UnknownScriptCodeLength = _countof(_u("Unknown script code")) - 1;
static const charcount_t NullStringLength = _countof(_u("Null")) - 1;
static const charcount_t TrueStringLength = _countof(_u("True")) - 1;
static const charcount_t FalseStringLength = _countof(_u("False")) - 1;
};

extern const FrameDisplay NullFrameDisplay;
Expand Down
32 changes: 15 additions & 17 deletions lib/Runtime/Language/JavascriptConversion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -538,27 +538,25 @@ namespace Js
}

// If exoticToPrim is not undefined, then
if (nullptr != exoticToPrim)
Assert(nullptr != exoticToPrim);
Copy link
Contributor

@digitalinfinity digitalinfinity Oct 26, 2017

Choose a reason for hiding this comment

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

Should this be a fail-fast? What guarantees that this is always not null? #Resolved

Copy link
Contributor Author

@MikeHolman MikeHolman Oct 27, 2017

Choose a reason for hiding this comment

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

If this is null then result is nullptr and it's not valid to pass nullptr to GetTypeId (prefast took issue with this due to some of the annotations I added).

If we get to this point, we will always have called JavascriptOperators::GetPropertyReference for varMethod, and we always assign exoticToPrim = varMethod. #Closed

ThreadContext * threadContext = requestContext->GetThreadContext();
result = threadContext->ExecuteImplicitCall(exoticToPrim, ImplicitCall_ToPrimitive, [=]()->Js::Var
{
ThreadContext * threadContext = requestContext->GetThreadContext();
result = threadContext->ExecuteImplicitCall(exoticToPrim, ImplicitCall_ToPrimitive, [=]()->Js::Var
{
// Stack object should have a pre-op bail on implicit call. We shouldn't see them here.
Assert(!ThreadContext::IsOnStack(recyclableObject));

// Let result be the result of calling the[[Call]] internal method of exoticToPrim, with input as thisArgument and(hint) as argumentsList.
return CALL_FUNCTION(threadContext, exoticToPrim, CallInfo(CallFlags_Value, 2), recyclableObject, hintString);
});
// Stack object should have a pre-op bail on implicit call. We shouldn't see them here.
Assert(!ThreadContext::IsOnStack(recyclableObject));

if (!result)
{
// There was an implicit call and implicit calls are disabled. This would typically cause a bailout.
Assert(threadContext->IsDisableImplicitCall());
return requestContext->GetLibrary()->GetNull();
}
// Let result be the result of calling the[[Call]] internal method of exoticToPrim, with input as thisArgument and(hint) as argumentsList.
return CALL_FUNCTION(threadContext, exoticToPrim, CallInfo(CallFlags_Value, 2), recyclableObject, hintString);
});

Assert(!CrossSite::NeedMarshalVar(result, requestContext));
if (!result)
{
// There was an implicit call and implicit calls are disabled. This would typically cause a bailout.
Assert(threadContext->IsDisableImplicitCall());
return requestContext->GetLibrary()->GetNull();
}

Assert(!CrossSite::NeedMarshalVar(result, requestContext));
// If result is an ECMAScript language value and Type(result) is not Object, then return result.
if (TaggedInt::Is(result) || !JavascriptOperators::IsObjectType(JavascriptOperators::GetTypeId(result)))
{
Expand Down
39 changes: 29 additions & 10 deletions lib/Runtime/Language/JavascriptOperators.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1325,32 +1325,36 @@ namespace Js
return type->GetPrototype();
}

BOOL JavascriptOperators::IsArray(Var instanceVar)
BOOL JavascriptOperators::IsArray(_In_ RecyclableObject* instance)
{
if (!RecyclableObject::Is(instanceVar))
{
return FALSE;
}
RecyclableObject* instance = RecyclableObject::UnsafeFromVar(instanceVar);
if (DynamicObject::IsAnyArray(instance))
{
return TRUE;
}

JavascriptProxy* proxy = JavascriptOperators::TryFromVar<JavascriptProxy>(instanceVar);
JavascriptProxy* proxy = JavascriptOperators::TryFromVar<JavascriptProxy>(instance);
if (proxy)
{
return IsArray(proxy->GetTarget());
}
TypeId remoteTypeId = TypeIds_Limit;
if (JavascriptOperators::GetRemoteTypeId(instanceVar, &remoteTypeId) &&
if (JavascriptOperators::GetRemoteTypeId(instance, &remoteTypeId) &&
DynamicObject::IsAnyArrayTypeId(remoteTypeId))
{
return TRUE;
}
return FALSE;
}

BOOL JavascriptOperators::IsArray(_In_ Var instanceVar)
{
if (!RecyclableObject::Is(instanceVar))
{
return FALSE;
}
return IsArray(RecyclableObject::FromVar(instanceVar));
}

BOOL JavascriptOperators::IsConstructor(Var instanceVar)
{
if (!RecyclableObject::Is(instanceVar))
Expand Down Expand Up @@ -10045,9 +10049,14 @@ namespace Js
return JavascriptNumber::ToVarNoCheck(JavascriptConversion::ToNumber_Full(aRight, scriptContext), scriptContext);
}

BOOL JavascriptOperators::IsObject(Var aValue)
BOOL JavascriptOperators::IsObject(_In_ RecyclableObject* instance)
{
return GetTypeId(instance) > TypeIds_LastJavascriptPrimitiveType;
}

BOOL JavascriptOperators::IsObject(_In_ Var instance)
{
return GetTypeId(aValue) > TypeIds_LastJavascriptPrimitiveType;
return GetTypeId(instance) > TypeIds_LastJavascriptPrimitiveType;
}

BOOL JavascriptOperators::IsObjectType(TypeId typeId)
Expand All @@ -10066,6 +10075,11 @@ namespace Js
return IsObjectType(typeId) || typeId == TypeIds_Null;
}

BOOL JavascriptOperators::IsUndefined(_In_ RecyclableObject* instance)
{
return JavascriptOperators::GetTypeId(instance) == TypeIds_Undefined;
}

BOOL JavascriptOperators::IsUndefined(Var instance)
{
return JavascriptOperators::GetTypeId(instance) == TypeIds_Undefined;
Expand Down Expand Up @@ -10408,6 +10422,11 @@ namespace Js

BOOL JavascriptOperators::GetItem(RecyclableObject* instance, uint64 index, Var* value, ScriptContext* requestContext)
{
if (index < JavascriptArray::InvalidIndex)
{
// In case index fits in uint32, we can avoid the (slower) big-index path
return GetItem(instance, static_cast<uint32>(index), value, requestContext);
}
PropertyRecord const * propertyRecord = nullptr;
JavascriptOperators::GetPropertyIdForInt(index, requestContext, &propertyRecord);
return JavascriptOperators::GetProperty(instance, propertyRecord->GetPropertyId(), value, requestContext);
Expand Down
10 changes: 7 additions & 3 deletions lib/Runtime/Language/JavascriptOperators.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ namespace Js
{
// Methods
public:
static BOOL IsArray(Var instanceVar);
static BOOL IsArray(_In_ RecyclableObject* instanceObj);
static BOOL IsArray(_In_ Var instanceVar);
static BOOL IsConstructor(Var instanceVar);
static BOOL IsConcatSpreadable(Var instanceVar);
static RecyclableObject* ToObject(Var aRight,ScriptContext* scriptContext);
Expand Down Expand Up @@ -207,17 +208,20 @@ namespace Js
static BOOL DeletePropertyUnscopables(RecyclableObject* instance, PropertyId propertyId, PropertyOperationFlags propertyOperationFlags = PropertyOperation_None);
template<bool unscopables>
static BOOL DeleteProperty_Impl(RecyclableObject* instance, PropertyId propertyId, PropertyOperationFlags propertyOperationFlags = PropertyOperation_None);
static TypeId GetTypeId(Var instance);
static TypeId GetTypeId(_In_ const Var instance);
static TypeId GetTypeId(_In_ RecyclableObject* instance);
static TypeId GetTypeIdNoCheck(Var instance);
template <typename T>
__forceinline static T* TryFromVar(Var value)
{
return T::Is(value) ? T::UnsafeFromVar(value) : nullptr;
}
static BOOL IsObject(Var instance);
static BOOL IsObject(_In_ Var instance);
static BOOL IsObject(_In_ RecyclableObject* instance);
static BOOL IsExposedType(TypeId typeId);
static BOOL IsObjectType(TypeId typeId);
static BOOL IsObjectOrNull(Var instance);
static BOOL IsUndefined(_In_ RecyclableObject* instance);
static BOOL IsUndefined(Var instance);
static BOOL IsUndefinedObject(RecyclableObject* instance);
static BOOL IsUndefinedOrNullType(TypeId);
Expand Down
21 changes: 14 additions & 7 deletions lib/Runtime/Language/JavascriptOperators.inl
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,19 @@

namespace Js
{
__forceinline TypeId JavascriptOperators::GetTypeId(const Var aValue)
__forceinline TypeId JavascriptOperators::GetTypeId(_In_ RecyclableObject* obj)
{
AssertMsg(obj != nullptr, "GetTypeId aValue is null");

auto typeId = obj->GetTypeId();
#if DBG
auto isExternal = obj->CanHaveInterceptors();
AssertMsg(typeId < TypeIds_Limit || isExternal, "GetTypeId aValue has invalid TypeId");
#endif
return typeId;
}

__forceinline TypeId JavascriptOperators::GetTypeId(_In_ const Var aValue)
{
AssertMsg(aValue != nullptr, "GetTypeId aValue is null");

Expand All @@ -22,12 +34,7 @@ namespace Js
#endif
else
{
auto typeId = RecyclableObject::UnsafeFromVar(aValue)->GetTypeId();
#if DBG
auto isExternal = RecyclableObject::FromVar(aValue)->CanHaveInterceptors();
AssertMsg(typeId < TypeIds_Limit || isExternal, "GetTypeId aValue has invalid TypeId");
#endif
return typeId;
return JavascriptOperators::GetTypeId(RecyclableObject::UnsafeFromVar(aValue));
}
}

Expand Down
3 changes: 3 additions & 0 deletions lib/Runtime/Library/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ set(CRLIB_SOURCE_CODES
JSONScanner.cpp
JSONStack.cpp
JSONString.cpp
JSONStringBuilder.cpp
JSONStringifier.cpp
JavascriptArray.cpp
JavascriptArrayIndexEnumerator.cpp
JavascriptArrayIndexEnumeratorBase.cpp
Expand Down Expand Up @@ -101,6 +103,7 @@ set(CRLIB_SOURCE_CODES
JavascriptVariantDate.cpp
JavascriptWeakMap.cpp
JavascriptWeakSet.cpp
LazyJSONString.cpp
LiteralString.cpp
MathLibrary.cpp
ModuleRoot.cpp
Expand Down
6 changes: 6 additions & 0 deletions lib/Runtime/Library/Chakra.Runtime.Library.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,9 @@
<ClCompile Include="$(MSBuildThisFileDirectory)CustomExternalIterator.cpp" />
<ClCompile Include="$(MSBuildThisFileDirectory)JavascriptExceptionMetadata.cpp" />
<ClCompile Include="$(MSBuildThisFileDirectory)VerifyMarkFalseReference.cpp" />
<ClCompile Include="JSONStringBuilder.cpp" />
<ClCompile Include="JSONStringifier.cpp" />
<ClCompile Include="LazyJSONString.cpp" />
</ItemGroup>
<ItemGroup>
<ClInclude Include="..\DetachedStateBase.h" />
Expand All @@ -193,6 +196,9 @@
<ClInclude Include="JavascriptSimdBool32x4.h" />
<ClInclude Include="JavascriptSimdBool16x8.h" />
<ClInclude Include="JavascriptSimdBool8x16.h" />
<ClInclude Include="JSONStringBuilder.h" />
<ClInclude Include="JSONStringifier.h" />
<ClInclude Include="LazyJSONString.h" />
<ClInclude Include="SharedArrayBuffer.h" />
<ClInclude Include="SimdFloat32x4Lib.h" />
<ClInclude Include="SimdFloat64x2Lib.h" />
Expand Down
6 changes: 6 additions & 0 deletions lib/Runtime/Library/Chakra.Runtime.Library.vcxproj.filters
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,9 @@
<ClCompile Include="$(MSBuildThisFileDirectory)WabtInterface.cpp" />
<ClCompile Include="$(MSBuildThisFileDirectory)CustomExternalIterator.cpp" />
<ClCompile Include="$(MSBuildThisFileDirectory)VerifyMarkFalseReference.cpp" />
<ClCompile Include="LazyJSONString.cpp" />
<ClCompile Include="JSONStringifier.cpp" />
<ClCompile Include="JSONStringBuilder.cpp" />
</ItemGroup>
<ItemGroup>
<ClInclude Include="..\InternalPropertyList.h" />
Expand Down Expand Up @@ -259,6 +262,9 @@
<ClInclude Include="CustomExternalIterator.h" />
<ClInclude Include="JavascriptExceptionMetadata.h" />
<ClInclude Include="..\DetachedStateBase.h" />
<ClInclude Include="LazyJSONString.h" />
<ClInclude Include="JSONStringifier.h" />
<ClInclude Include="JSONStringBuilder.h" />
</ItemGroup>
<ItemGroup>
<None Include="ConcatString.inl" />
Expand Down
Loading