remove old interceptors code and open a space under TypeFlagMask

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!
chakra-core · Jan 25, 2018 · 157fd4f · 157fd4f
1 parent e5e1c85
commit 157fd4f
Show file tree

Hide file tree

Showing 9 changed files with 23 additions and 29 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
@@ -12,7 +12,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();
@@ -158,7 +158,7 @@ namespace TTD
                     if(Js::IsInternalPropertyId(pid))
                     {
                         propertyReset.Clear();
-                        return true; 
+                        return true;
                     }
 
                     //someone added a property that is not simple to remove so let's just be safe an recreate contexts

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);
 
@@ -8290,7 +8290,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)
@@ -8368,7 +8368,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()