[MERGE #3717 @obastemur] remove old interceptors code and open a spac…

…e 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.
chakra-core · Jan 31, 2018 · e93b29b · e93b29b
2 parents a231b21 + 157fd4f
commit e93b29b
Show file tree

Hide file tree

Showing 9 changed files with 22 additions and 28 deletions.
diff --git a/lib/Runtime/Debug/DiagObjectModel.cpp b/lib/Runtime/Debug/DiagObjectModel.cpp
@@ -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;
@@ -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
                         {

diff --git a/lib/Runtime/Debug/TTSnapObjects.cpp b/lib/Runtime/Debug/TTSnapObjects.cpp
@@ -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();

diff --git a/lib/Runtime/Debug/TTSnapshotExtractor.cpp b/lib/Runtime/Debug/TTSnapshotExtractor.cpp
@@ -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))

diff --git a/lib/Runtime/Language/JavascriptOperators.cpp b/lib/Runtime/Language/JavascriptOperators.cpp
@@ -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))
@@ -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))
@@ -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);
 
@@ -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)
@@ -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)
                         {

diff --git a/lib/Runtime/Language/JavascriptOperators.inl b/lib/Runtime/Language/JavascriptOperators.inl
@@ -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;
     }
 

diff --git a/lib/Runtime/Library/JavascriptObject.cpp b/lib/Runtime/Library/JavascriptObject.cpp
@@ -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);

diff --git a/lib/Runtime/Types/RecyclableObject.h b/lib/Runtime/Types/RecyclableObject.h
@@ -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; }
@@ -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:

diff --git a/lib/Runtime/Types/RecyclableObject.inl b/lib/Runtime/Types/RecyclableObject.inl
@@ -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));

diff --git a/lib/Runtime/Types/Type.h b/lib/Runtime/Types/Type.h
@@ -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
 };
@@ -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()