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

Commit

Permalink
deps: update ChakraCore to chakra-core/ChakraCore@e93b29b80d
Browse files Browse the repository at this point in the history
[MERGE #3717 @obastemur] remove old interceptors code and open a space under TypeFlagMask

Merge pull request #3717 from obastemur:rem_inter

CanHaveInterceptors was being triggered heavily on some of the benchmarks and looking deeper shown that there is no reason keeping this code around.

Besides.. Removing this, comes with a very useful space within `TypeFlagMask` that I could use for ExternalData support.

So.. unless someone comes and say that this particular mask is in use at some place (which I could not find), I will update the external data PR too.

Reminder: Both TypeFlagMask_External and TypeFlagMask_CanHaveInterceptors are set at the same place in full code. There is no place that they were being set separately.

Reviewed-By: chakrabot <[email protected]>
  • Loading branch information
obastemur authored and kfarnung committed Feb 1, 2018
1 parent 54f38ce commit bcecf8e
Show file tree
Hide file tree
Showing 9 changed files with 22 additions and 28 deletions.
4 changes: 2 additions & 2 deletions deps/chakrashim/core/lib/Runtime/Debug/DiagObjectModel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2141,7 +2141,7 @@ namespace Js
auto funcPtr = [&]()
{
IGNORE_STACKWALK_EXCEPTION(scriptContext);
if (object->CanHaveInterceptors())
if (object->IsExternal())
{
Js::ForInObjectEnumerator enumerator(object, object->GetScriptContext(), /* enumSymbols */ true);
Js::PropertyId propertyId;
Expand Down Expand Up @@ -2461,7 +2461,7 @@ namespace Js

if (JavascriptOperators::IsObject(object))
{
if (object->CanHaveInterceptors() || JavascriptOperators::GetTypeId(object) == TypeIds_Proxy)
if (object->IsExternal() || JavascriptOperators::GetTypeId(object) == TypeIds_Proxy)
{
try
{
Expand Down
2 changes: 1 addition & 1 deletion deps/chakrashim/core/lib/Runtime/Debug/TTSnapObjects.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ namespace TTD
{
void ExtractCompoundObject(NSSnapObjects::SnapObject* sobj, Js::RecyclableObject* obj, bool isWellKnown, const TTDIdentifierDictionary<TTD_PTR_ID, NSSnapType::SnapType*>& idToTypeMap, SlabAllocator& alloc)
{
TTDAssert(!obj->CanHaveInterceptors(), "We are not prepared for custom external objects yet");
TTDAssert(!obj->IsExternal(), "We are not prepared for custom external objects yet");

sobj->ObjectPtrId = TTD_CONVERT_VAR_TO_PTR_ID(obj);
sobj->SnapObjectTag = obj->GetSnapTag_TTD();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ namespace TTD
void SnapshotExtractor::MarkVisitVar(Js::Var var)
{
TTDAssert(var != nullptr, "I don't think this should happen but not 100% sure.");
TTDAssert(Js::JavascriptOperators::GetTypeId(var) < Js::TypeIds_Limit || Js::RecyclableObject::FromVar(var)->CanHaveInterceptors(), "Not cool.");
TTDAssert(Js::JavascriptOperators::GetTypeId(var) < Js::TypeIds_Limit || Js::RecyclableObject::FromVar(var)->IsExternal(), "Not cool.");

//We don't need to visit tagged things
if(JsSupport::IsVarTaggedInline(var))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1045,7 +1045,7 @@ namespace Js
#endif
}

if (RecyclableObject::FromVar(aLeft)->CanHaveInterceptors())
if (RecyclableObject::FromVar(aLeft)->IsExternal())
{
BOOL result;
if (RecyclableObject::FromVar(aLeft)->StrictEquals(aRight, &result, requestContext))
Expand All @@ -1057,7 +1057,7 @@ namespace Js
}
}

if (!TaggedNumber::Is(aRight) && RecyclableObject::FromVar(aRight)->CanHaveInterceptors())
if (!TaggedNumber::Is(aRight) && RecyclableObject::FromVar(aRight)->IsExternal())
{
BOOL result;
if (RecyclableObject::FromVar(aRight)->StrictEquals(aLeft, &result, requestContext))
Expand Down Expand Up @@ -2356,7 +2356,7 @@ namespace Js
// in 9.1.9, step 5, we should return false if receiver is not object, and that will happen in default RecyclableObject operation anyhow.
if (receiverObject->SetProperty(propertyKey, newValue, propertyOperationFlags, info))
{
if (!JavascriptProxy::Is(receiver) && info->GetPropertyString() && info->GetFlags() != InlineCacheSetterFlag && !object->CanHaveInterceptors())
if (!JavascriptProxy::Is(receiver) && info->GetPropertyString() && info->GetFlags() != InlineCacheSetterFlag && !object->IsExternal())
{
CacheOperators::CachePropertyWrite(RecyclableObject::FromVar(receiver), false, typeWithoutProperty, info->GetPropertyString()->GetPropertyId(), info, requestContext);

Expand Down Expand Up @@ -8276,7 +8276,7 @@ namespace Js
// It is valid for some objects to not-support getters and setters, specifically, for projection of an ABI method
// (CustomExternalObject => MapWithStringKey) which SetAccessors returns VBSErr_ActionNotSupported.
// But for non-external objects SetAccessors should succeed.
Assert(isSetAccessorsSuccess || obj->CanHaveInterceptors());
Assert(isSetAccessorsSuccess || obj->IsExternal());

// If SetAccessors failed, the property wasn't created, so no need to change the attributes.
if (isSetAccessorsSuccess)
Expand Down Expand Up @@ -8354,7 +8354,7 @@ namespace Js
// It is valid for some objects to not-support getters and setters, specifically, for projection of an ABI method
// (CustomExternalObject => MapWithStringKey) which SetAccessors returns VBSErr_ActionNotSupported.
// But for non-external objects SetAccessors should succeed.
Assert(isSetAccessorsSuccess || obj->CanHaveInterceptors());
Assert(isSetAccessorsSuccess || obj->IsExternal());

if (isSetAccessorsSuccess)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,7 @@ namespace Js
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
AssertMsg(typeId < TypeIds_Limit || obj->IsExternal(), "GetTypeId aValue has invalid TypeId");
return typeId;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ namespace Js
BOOL JavascriptObject::GetOwnPropertyDescriptorHelper(RecyclableObject* obj, PropertyId propertyId, ScriptContext* scriptContext, PropertyDescriptor& propertyDescriptor)
{
BOOL isPropertyDescriptorDefined;
if (obj->CanHaveInterceptors())
if (obj->IsExternal())
{
isPropertyDescriptorDefined = obj->HasOwnProperty(propertyId) ?
JavascriptOperators::GetOwnPropertyDescriptor(obj, propertyId, scriptContext, &propertyDescriptor) : obj->GetDefaultPropertyDescriptor(propertyDescriptor);
Expand Down
2 changes: 0 additions & 2 deletions deps/chakrashim/core/lib/Runtime/Types/RecyclableObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,6 @@ namespace Js {
virtual BOOL HasInstance(Var instance, ScriptContext* scriptContext, IsInstInlineCache* inlineCache = NULL);

BOOL SkipsPrototype() const;
BOOL CanHaveInterceptors() const;
BOOL IsExternal() const;
// Used only in JsVarToExtension where it may be during dispose and the type is not available
virtual BOOL IsExternalVirtual() const { return FALSE; }
Expand Down Expand Up @@ -402,7 +401,6 @@ namespace Js {
// Used to Assert that the object may safely be cast to a DynamicObject
virtual bool DbgIsDynamicObject() const { return false; }
virtual BOOL DbgSkipsPrototype() const { return FALSE; }
virtual BOOL DbgCanHaveInterceptors() const { return false; }
#endif
#if defined(PROFILE_RECYCLER_ALLOC) && defined(RECYCLER_DUMP_OBJECT_GRAPH)
public:
Expand Down
8 changes: 0 additions & 8 deletions deps/chakrashim/core/lib/Runtime/Types/RecyclableObject.inl
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,6 @@ namespace Js
return this->GetLibrary()->GetScriptContext();
}

inline BOOL RecyclableObject::CanHaveInterceptors() const
{
#if !defined(USED_IN_STATIC_LIB)
Assert(this->DbgCanHaveInterceptors() == this->GetType()->CanHaveInterceptors());
#endif
return this->GetType()->CanHaveInterceptors();
}

inline BOOL RecyclableObject::HasItem(uint32 index)
{
return JavascriptConversion::PropertyQueryFlagsToBoolean(HasItemQuery(index));
Expand Down
15 changes: 11 additions & 4 deletions deps/chakrashim/core/lib/Runtime/Types/Type.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ enum TypeFlagMask : uint8
TypeFlagMask_AreThisAndPrototypesEnsuredToHaveOnlyWritableDataProperties = 0x01,
TypeFlagMask_IsFalsy = 0x02,
TypeFlagMask_HasSpecialPrototype = 0x04,
TypeFlagMask_External = 0x08,
TypeFlagMask_EngineExternal = 0x08,
TypeFlagMask_SkipsPrototype = 0x10,
TypeFlagMask_CanHaveInterceptors = 0x20,
TypeFlagMask_RESERVED = 0x20,
TypeFlagMask_JsrtExternal = 0x40,
TypeFlagMask_HasBeenCached = 0x80
};
Expand Down Expand Up @@ -60,10 +60,17 @@ namespace Js
BOOL AreThisAndPrototypesEnsuredToHaveOnlyWritableDataProperties() const;
void SetAreThisAndPrototypesEnsuredToHaveOnlyWritableDataProperties(const bool truth);

inline BOOL IsExternal() const { return (this->flags & TypeFlagMask_External) != 0; }
inline BOOL IsExternal() const
{
#ifdef NTBUILD
return (this->flags & TypeFlagMask_EngineExternal) != 0;
#else
AssertMsg((this->flags & TypeFlagMask_EngineExternal) == 0, "Not expected.");
return false;
#endif
}
inline BOOL IsJsrtExternal() const { return (this->flags & TypeFlagMask_JsrtExternal) != 0; }
inline BOOL SkipsPrototype() const { return (this->flags & TypeFlagMask_SkipsPrototype) != 0 ; }
inline BOOL CanHaveInterceptors() const { return (this->flags & TypeFlagMask_CanHaveInterceptors) != 0; }
inline BOOL IsFalsy() const { return flags & TypeFlagMask_IsFalsy; }
inline BOOL HasBeenCached() const { return flags & TypeFlagMask_HasBeenCached; }
inline void SetHasBeenCached()
Expand Down

0 comments on commit bcecf8e

Please sign in to comment.