Use PathTypeHandler's for function objects with non-default propertie…

…s/attributes. Move from creating new non-shared SimpleTypeHandler's when a new property is added to a SimpleTypeHandler, or an attribute is changed, to creating PathTypeHandler's. Build a new root and evolve from the root to the current state of the SimpleTypeHandler, then replace the type's existing SimpleTypeHandler with the new PathTypeHandler. From there, we can evolve to the state required by the new property/attribute, and future instances of the same function that make the same type transition will get the same type. Also fix up some shared SimpleTypeHandler's in the library that didn't represent the desired state of their library objects and were forcing type transitions during initialization.
chakra-core · Mar 20, 2018 · 978c171 · 978c171
1 parent e8c4ae6
commit 978c171
Show file tree

Hide file tree

Showing 10 changed files with 186 additions and 73 deletions.
diff --git a/lib/Runtime/Language/CacheOperators.cpp b/lib/Runtime/Language/CacheOperators.cpp
@@ -234,8 +234,16 @@ namespace Js
                 DynamicTypeHandler* oldTypeHandler = oldType->GetTypeHandler();
                 DynamicTypeHandler* newTypeHandler = newType->GetTypeHandler();
 
+#if ENABLE_FIXED_FIELDS
                 // the newType is a path-type so the old one should be too:
                 Assert(oldTypeHandler->IsPathTypeHandler());
+#else
+                // This may be the transition from deferred type handler to path type handler. Don't try to cache now.
+                if (!oldTypeHandler->IsPathTypeHandler())
+                {
+                    return;
+                }
+#endif
 
                 int oldCapacity = oldTypeHandler->GetSlotCapacity();
                 int newCapacity = newTypeHandler->GetSlotCapacity();

diff --git a/lib/Runtime/Library/JavascriptLibrary.cpp b/lib/Runtime/Library/JavascriptLibrary.cpp
@@ -32,6 +32,12 @@ namespace Js
         SimplePropertyDescriptor(NO_WRITE_BARRIER_TAG(BuiltInPropertyRecords::name), PropertyConfigurable)
     };
 
+    SimplePropertyDescriptor const JavascriptLibrary::SharedIdMappedFunctionPropertyDescriptors[2] =
+    {
+        SimplePropertyDescriptor(NO_WRITE_BARRIER_TAG(BuiltInPropertyRecords::prototype), PropertyNone),
+        SimplePropertyDescriptor(NO_WRITE_BARRIER_TAG(BuiltInPropertyRecords::name), PropertyConfigurable)
+    };
+
     SimplePropertyDescriptor const JavascriptLibrary::FunctionWithLengthAndNameTypeDescriptors[2] =
     {
         SimplePropertyDescriptor(NO_WRITE_BARRIER_TAG(BuiltInPropertyRecords::length), PropertyConfigurable),
@@ -40,14 +46,15 @@ namespace Js
 
     SimplePropertyDescriptor const JavascriptLibrary::ModuleNamespaceTypeDescriptors[1] =
     {
-        SimplePropertyDescriptor(NO_WRITE_BARRIER_TAG(BuiltInPropertyRecords::_symbolToStringTag), PropertyConfigurable)
+        SimplePropertyDescriptor(NO_WRITE_BARRIER_TAG(BuiltInPropertyRecords::_symbolToStringTag), PropertyNone)
     };
 
     SimpleTypeHandler<1> JavascriptLibrary::SharedPrototypeTypeHandler(NO_WRITE_BARRIER_TAG(BuiltInPropertyRecords::constructor), PropertyWritable | PropertyConfigurable, PropertyTypesWritableDataOnly, 4, sizeof(DynamicObject));
     SimpleTypeHandler<1> JavascriptLibrary::SharedFunctionWithoutPrototypeTypeHandler(NO_WRITE_BARRIER_TAG(BuiltInPropertyRecords::name), PropertyConfigurable);
     SimpleTypeHandler<1> JavascriptLibrary::SharedFunctionWithPrototypeTypeHandlerV11(NO_WRITE_BARRIER_TAG(BuiltInPropertyRecords::prototype), PropertyWritable);
     SimpleTypeHandler<2> JavascriptLibrary::SharedFunctionWithPrototypeTypeHandler(NO_WRITE_BARRIER_TAG(SharedFunctionPropertyDescriptors));
-    SimpleTypeHandler<1> JavascriptLibrary::SharedIdMappedFunctionWithPrototypeTypeHandler(NO_WRITE_BARRIER_TAG(BuiltInPropertyRecords::prototype));
+    SimpleTypeHandler<2> JavascriptLibrary::SharedIdMappedFunctionWithPrototypeTypeHandler(NO_WRITE_BARRIER_TAG(SharedIdMappedFunctionPropertyDescriptors));
+    SimpleTypeHandler<1> JavascriptLibrary::SharedFunctionWithConfigurableLengthTypeHandler(NO_WRITE_BARRIER_TAG(BuiltInPropertyRecords::length), PropertyConfigurable);
     SimpleTypeHandler<1> JavascriptLibrary::SharedFunctionWithLengthTypeHandler(NO_WRITE_BARRIER_TAG(BuiltInPropertyRecords::length));
     SimpleTypeHandler<2> JavascriptLibrary::SharedFunctionWithLengthAndNameTypeHandler(NO_WRITE_BARRIER_TAG(FunctionWithLengthAndNameTypeDescriptors));
     SimpleTypeHandler<1> JavascriptLibrary::SharedNamespaceSymbolTypeHandler(NO_WRITE_BARRIER_TAG(ModuleNamespaceTypeDescriptors));
@@ -898,6 +905,11 @@ namespace Js
             GetDeferredFunctionTypeHandler(), false, false);
     }
 
+    DynamicType * JavascriptLibrary::CreateFunctionWithConfigurableLengthType(FunctionInfo * functionInfo)
+    {
+        return CreateFunctionWithConfigurableLengthType(this->GetFunctionPrototype(), functionInfo);
+    }
+
     DynamicType * JavascriptLibrary::CreateFunctionWithLengthType(FunctionInfo * functionInfo)
     {
         return CreateFunctionWithLengthType(this->GetFunctionPrototype(), functionInfo);
@@ -913,6 +925,14 @@ namespace Js
         return CreateFunctionWithLengthAndPrototypeType(this->GetFunctionPrototype(), functionInfo);
     }
 
+    DynamicType * JavascriptLibrary::CreateFunctionWithConfigurableLengthType(DynamicObject * prototype, FunctionInfo * functionInfo)
+    {
+        Assert(!functionInfo->HasBody());
+        return DynamicType::New(scriptContext, TypeIds_Function, prototype,
+            this->inProfileMode? ProfileEntryThunk : functionInfo->GetOriginalEntryPoint(),
+            &SharedFunctionWithConfigurableLengthTypeHandler);
+    }
+
     DynamicType * JavascriptLibrary::CreateFunctionWithLengthType(DynamicObject * prototype, FunctionInfo * functionInfo)
     {
         Assert(!functionInfo->HasBody());
@@ -5034,11 +5054,15 @@ namespace Js
             {
                 function->ReplaceType(crossSiteExternalConstructFunctionWithPrototypeType);
             }
-            else
+            else if (typeHandler == &SharedIdMappedFunctionWithPrototypeTypeHandler)
             {
-                Assert(typeHandler == &SharedIdMappedFunctionWithPrototypeTypeHandler);
                 function->ReplaceType(crossSiteIdMappedFunctionWithPrototypeType);
             }
+            else
+            {
+                function->ChangeType();
+                function->SetEntryPoint(scriptContext->CurrentCrossSiteThunk);
+            }
         }
     }
 

diff --git a/lib/Runtime/Library/JavascriptLibrary.h b/lib/Runtime/Library/JavascriptLibrary.h
@@ -519,13 +519,15 @@ namespace Js
         static SimpleTypeHandler<1> SharedFunctionWithoutPrototypeTypeHandler;
         static SimpleTypeHandler<1> SharedFunctionWithPrototypeTypeHandlerV11;
         static SimpleTypeHandler<2> SharedFunctionWithPrototypeTypeHandler;
+        static SimpleTypeHandler<1> SharedFunctionWithConfigurableLengthTypeHandler;
         static SimpleTypeHandler<1> SharedFunctionWithLengthTypeHandler;
         static SimpleTypeHandler<2> SharedFunctionWithLengthAndNameTypeHandler;
-        static SimpleTypeHandler<1> SharedIdMappedFunctionWithPrototypeTypeHandler;
+        static SimpleTypeHandler<2> SharedIdMappedFunctionWithPrototypeTypeHandler;
         static SimpleTypeHandler<1> SharedNamespaceSymbolTypeHandler;
         static MissingPropertyTypeHandler MissingPropertyHolderTypeHandler;
 
         static SimplePropertyDescriptor const SharedFunctionPropertyDescriptors[2];
+        static SimplePropertyDescriptor const SharedIdMappedFunctionPropertyDescriptors[2];
         static SimplePropertyDescriptor const HeapArgumentsPropertyDescriptors[3];
         static SimplePropertyDescriptor const FunctionWithLengthAndPrototypeTypeDescriptors[2];
         static SimplePropertyDescriptor const FunctionWithLengthAndNameTypeDescriptors[2];
@@ -984,9 +986,11 @@ namespace Js
         DynamicType * CreateDeferredPrototypeFunctionType(JavascriptMethod entrypoint);
         DynamicType * CreateDeferredPrototypeFunctionTypeNoProfileThunk(JavascriptMethod entrypoint, bool isShared = false, bool isLengthAvailable = false);
         DynamicType * CreateFunctionType(JavascriptMethod entrypoint, RecyclableObject* prototype = nullptr);
+        DynamicType * CreateFunctionWithConfigurableLengthType(FunctionInfo * functionInfo);
         DynamicType * CreateFunctionWithLengthType(FunctionInfo * functionInfo);
         DynamicType * CreateFunctionWithLengthAndNameType(FunctionInfo * functionInfo);
         DynamicType * CreateFunctionWithLengthAndPrototypeType(FunctionInfo * functionInfo);
+        DynamicType * CreateFunctionWithConfigurableLengthType(DynamicObject * prototype, FunctionInfo * functionInfo);
         DynamicType * CreateFunctionWithLengthType(DynamicObject * prototype, FunctionInfo * functionInfo);
         DynamicType * CreateFunctionWithLengthAndNameType(DynamicObject * prototype, FunctionInfo * functionInfo);
         DynamicType * CreateFunctionWithLengthAndPrototypeType(DynamicObject * prototype, FunctionInfo * functionInfo);

diff --git a/lib/Runtime/Library/JavascriptProxy.cpp b/lib/Runtime/Library/JavascriptProxy.cpp
@@ -130,7 +130,7 @@ namespace Js
 
         JavascriptProxy* proxy = JavascriptProxy::Create(scriptContext, args);
         JavascriptLibrary* library = scriptContext->GetLibrary();
-        DynamicType* type = library->CreateFunctionWithLengthType(&EntryInfo::Revoke);
+        DynamicType* type = library->CreateFunctionWithConfigurableLengthType(&EntryInfo::Revoke);
         RuntimeFunction* revoker = RecyclerNewEnumClass(scriptContext->GetRecycler(),
             JavascriptLibrary::EnumFunctionClass, RuntimeFunction,
             type, &EntryInfo::Revoke);

diff --git a/lib/Runtime/Library/ScriptFunction.cpp b/lib/Runtime/Library/ScriptFunction.cpp
@@ -106,6 +106,13 @@ namespace Js
 
             pfuncScriptWithInlineCache->SetHasSuperReference(hasSuperReference);
 
+            ScriptFunctionType *scFuncType = functionProxy->GetUndeferredFunctionType();
+            if (scFuncType)
+            {
+                Assert(pfuncScriptWithInlineCache->GetType() == functionProxy->GetDeferredPrototypeType());
+                pfuncScriptWithInlineCache->GetTypeHandler()->EnsureObjectReady(pfuncScriptWithInlineCache);
+            }
+
             if (PHASE_TRACE1(Js::ScriptFunctionWithInlineCachePhase))
             {
                 char16 debugStringBuffer[MAX_FUNCTION_BODY_DEBUG_STRING_SIZE];
@@ -123,6 +130,13 @@ namespace Js
 
             pfuncScript->SetHasSuperReference(hasSuperReference);
 
+            ScriptFunctionType *scFuncType = functionProxy->GetUndeferredFunctionType();
+            if (scFuncType)
+            {
+                Assert(pfuncScript->GetType() == functionProxy->GetDeferredPrototypeType());
+                pfuncScript->GetTypeHandler()->EnsureObjectReady(pfuncScript);
+            }
+
             JS_ETW(EventWriteJSCRIPT_RECYCLER_ALLOCATE_FUNCTION(pfuncScript, EtwTrace::GetFunctionId(functionProxy)));
 
             return pfuncScript;

diff --git a/lib/Runtime/Runtime.h b/lib/Runtime/Runtime.h
@@ -134,7 +134,7 @@ namespace Js
     class DeferredTypeHandlerBase;
     template <bool IsPrototype> class NullTypeHandler;
     template<size_t size> class SimpleTypeHandler;
-    class PathTypeHandler;
+    class PathTypeHandlerBase;
     class IndexPropertyDescriptor;
     class DynamicObject;
     class ArrayObject;

diff --git a/lib/Runtime/Types/PathTypeHandler.cpp b/lib/Runtime/Types/PathTypeHandler.cpp
@@ -130,6 +130,12 @@ namespace Js
 #endif
     }
 
+    void PathTypeSingleSuccessorInfo::ReplaceSuccessor(DynamicType * type, const PathTypeSuccessorKey key, RecyclerWeakReference<DynamicType> * typeWeakRef)
+    {
+        Assert(successorKey == key);
+        successorTypeWeakRef = typeWeakRef;
+    }
+
     template<class Fn>
     void PathTypeSingleSuccessorInfo::MapSingleSuccessor(Fn fn)
     {
@@ -168,6 +174,13 @@ namespace Js
         propertySuccessors->Item(key, typeWeakRef);
     }
 
+    void PathTypeMultiSuccessorInfo::ReplaceSuccessor(DynamicType * type, const PathTypeSuccessorKey key, RecyclerWeakReference<DynamicType> * typeWeakRef)
+    {
+        Assert(this->propertySuccessors);
+        Assert(propertySuccessors->Item(key));
+        propertySuccessors->Item(key, typeWeakRef);
+    }
+
     template<class Fn>
     void PathTypeMultiSuccessorInfo::MapMultiSuccessors(Fn fn)
     {
@@ -343,9 +356,21 @@ namespace Js
         return found;
     }
 
-    BOOL PathTypeHandlerBase::SetAttributesHelper(DynamicObject* instance, PropertyId propertyId, PropertyIndex propertyIndex, ObjectSlotAttributes * instanceAttributes, ObjectSlotAttributes propertyAttributes)
+    BOOL PathTypeHandlerBase::SetAttributesHelper(DynamicObject* instance, PropertyId propertyId, PropertyIndex propertyIndex, ObjectSlotAttributes * instanceAttributes, ObjectSlotAttributes propertyAttributes, bool isInit)
     {
-        if (instanceAttributes == nullptr ? propertyAttributes == ObjectSlotAttr_Default : propertyAttributes == instanceAttributes[propertyIndex])
+        if (instanceAttributes)
+        {
+            if (!isInit)
+            {
+                // Preserve non-default bits like accessors
+                propertyAttributes = (ObjectSlotAttributes)(propertyAttributes | (instanceAttributes[propertyIndex] & ~ObjectSlotAttr_PropertyAttributesMask));
+            }
+            if (propertyAttributes == instanceAttributes[propertyIndex])
+            {
+                return true;
+            }
+        }
+        else if (propertyAttributes == ObjectSlotAttr_Default)
         {
             return true;
         }
@@ -714,18 +739,16 @@ namespace Js
             // In CacheOperators::CachePropertyWrite we ensure that we never cache property adds for types that aren't shared.
             Assert(!instance->GetDynamicType()->GetIsShared() || GetIsShared());
 
-            Assert(instance->GetDynamicType()->GetIsShared() == GetIsShared());
-
             if (setAttributes)
             {
-                this->SetAttributesHelper(instance, propertyId, index, GetAttributeArray(), attr);
+                this->SetAttributesHelper(instance, propertyId, index, GetAttributeArray(), attr, isInit);
             }
             else if (isInit)
             {
                 ObjectSlotAttributes * attributes = this->GetAttributeArray();
                 if (attributes && (attributes[index] & ObjectSlotAttr_Accessor))
                 {
-                    this->SetAttributesHelper(instance, propertyId, index, attributes, (ObjectSlotAttributes)(attributes[index] & ~ObjectSlotAttr_Accessor));
+                    this->SetAttributesHelper(instance, propertyId, index, attributes, (ObjectSlotAttributes)(attributes[index] & ~ObjectSlotAttr_Accessor), true);
                 }
             }
             PathTypeHandlerBase *newTypeHandler = PathTypeHandlerBase::FromTypeHandler(instance->GetDynamicType()->GetTypeHandler());
@@ -1722,7 +1745,12 @@ namespace Js
             return true;
         }
 
-        return SetAttributesHelper(instance, propertyId, propertyIndex, GetAttributeArray(), PropertyAttributesToObjectSlotAttributes(attributes));
+        return SetAttributesAtIndex(instance, propertyId, propertyIndex, attributes);
+    }
+
+    BOOL PathTypeHandlerBase::SetAttributesAtIndex(DynamicObject* instance, PropertyId propertyId, PropertyIndex index, PropertyAttributes attributes)
+    {
+        return SetAttributesHelper(instance, propertyId, index, GetAttributeArray(), PropertyAttributesToObjectSlotAttributes(attributes));
     }
 
     BOOL PathTypeHandlerBase::GetAttributesWithPropertyIndex(DynamicObject * instance, PropertyId propertyId, BigPropertyIndex index, PropertyAttributes * attributes)

diff --git a/lib/Runtime/Types/PathTypeHandler.h b/lib/Runtime/Types/PathTypeHandler.h
@@ -39,6 +39,7 @@ namespace Js
         bool IsMultiSuccessor() const { return !IsSingleSuccessor(); }
         virtual bool GetSuccessor(const PathTypeSuccessorKey successorKey, RecyclerWeakReference<DynamicType> ** typeWeakRef) const = 0;
         virtual void SetSuccessor(DynamicType * type, const PathTypeSuccessorKey successorKey, RecyclerWeakReference<DynamicType> * typeWeakRef, ScriptContext * scriptContext) = 0;
+        virtual void ReplaceSuccessor(DynamicType * type, PathTypeSuccessorKey successorKey, RecyclerWeakReference<DynamicType> * typeWeakRef) = 0;
 
         template<class Fn> void MapSuccessors(Fn fn);
         template<class Fn> void MapSuccessorsUntil(Fn fn);
@@ -61,6 +62,7 @@ namespace Js
 
         virtual bool GetSuccessor(const PathTypeSuccessorKey successorKey, RecyclerWeakReference<DynamicType> ** typeWeakRef) const override;
         virtual void SetSuccessor(DynamicType * type, const PathTypeSuccessorKey successorKey, RecyclerWeakReference<DynamicType> * typeWeakRef, ScriptContext * scriptContext) override;
+        virtual void ReplaceSuccessor(DynamicType * type, PathTypeSuccessorKey successorKey, RecyclerWeakReference<DynamicType> * typeWeakRef) override;
 
         template<class Fn> void MapSingleSuccessor(Fn fn);
 
@@ -78,6 +80,7 @@ namespace Js
 
         virtual bool GetSuccessor(const PathTypeSuccessorKey successorKey, RecyclerWeakReference<DynamicType> ** typeWeakRef) const override;
         virtual void SetSuccessor(DynamicType * type, const PathTypeSuccessorKey successorKey, RecyclerWeakReference<DynamicType> * typeWeakRef, ScriptContext * scriptContext) override;
+        virtual void ReplaceSuccessor(DynamicType * type, PathTypeSuccessorKey successorKey, RecyclerWeakReference<DynamicType> * typeWeakRef) override;
 
         template<class Fn> void MapMultiSuccessors(Fn fn);
         template<class Fn> void MapMultiSuccessorsUntil(Fn fn);
@@ -89,6 +92,8 @@ namespace Js
 
     class PathTypeHandlerBase : public DynamicTypeHandler
     {
+        template<size_t size>
+        friend class SimpleTypeHandler;
         friend class PathTypeHandlerNoAttr;
         friend class PathTypeHandlerWithAttr;
         friend class DynamicObject;
@@ -115,6 +120,7 @@ namespace Js
         template<class Fn> void MapSuccessorsUntil(Fn fn);
         PathTypeSuccessorInfo * GetSuccessorInfo() const { return successorInfo; }
         void SetSuccessorInfo(PathTypeSuccessorInfo * info) { successorInfo = info; }
+        void ReplaceSuccessor(DynamicType * type, PathTypeSuccessorKey key, RecyclerWeakReference<DynamicType> * typeWeakRef) { return successorInfo->ReplaceSuccessor(type, key, typeWeakRef); }
 
         static PropertyAttributes ObjectSlotAttributesToPropertyAttributes(const ObjectSlotAttributes attr) { return attr & ObjectSlotAttr_PropertyAttributesMask; }
         static ObjectSlotAttributes PropertyAttributesToObjectSlotAttributes(const PropertyAttributes attr) { return (ObjectSlotAttributes)(attr & ObjectSlotAttr_PropertyAttributesMask); }
@@ -182,7 +188,8 @@ namespace Js
 
         BOOL FindNextPropertyHelper(ScriptContext* scriptContext, ObjectSlotAttributes * objectAttributes, PropertyIndex& index, JavascriptString** propertyString,
             PropertyId* propertyId, PropertyAttributes* attributes, Type* type, DynamicType *typeToEnumerate, EnumeratorFlags flags, DynamicObject* instance, PropertyValueInfo* info);
-        BOOL SetAttributesHelper(DynamicObject* instance, PropertyId propertyId, PropertyIndex propertyIndex, ObjectSlotAttributes * instanceAttributes, ObjectSlotAttributes propertyAttributes);
+        BOOL SetAttributesAtIndex(DynamicObject* instance, PropertyId propertyId, PropertyIndex index, PropertyAttributes attributes);
+        BOOL SetAttributesHelper(DynamicObject* instance, PropertyId propertyId, PropertyIndex propertyIndex, ObjectSlotAttributes * instanceAttributes, ObjectSlotAttributes propertyAttributes, bool isInit = false);
         BOOL SetAccessorsHelper(DynamicObject* instance, PropertyId propertyId, ObjectSlotAttributes * attributes, PathTypeSetterSlotIndex * setters, Var getter, Var setter, PropertyOperationFlags flags);
 
 #if ENABLE_NATIVE_CODEGEN