Convert TypeTransitionMap to WeaklyReferencedKeyDictionary

Share `JsrtExternalType` for `JsrtExternalObject`

- Today the type transition map was a `BaseDictionary` of `DynamicType *` to `DynamicType *`. This would have kept both old and new type alive. Changed it to `WeaklyReferencedKeyDictionary`
  where key is weak reference of old `DynamicType` and value is new `DynamicType *`. This will guarantee that when oldType is collected, we no longer use its corresponding newType but instead
  create a newer type.
- Added a map on `scriptContext` to share the `JsrtExternalType` for `JsrtExternalObject` based on `finalizeCallback`. So `JsrtExternalObject` created with same callback will get same type.
  This faciliate to use `JsrtExternalObject` in type sharing for `__proto__` optimization. The map on `scriptContext` is registered on scriptContext weakReferenceDictionaryList so the entries
  inside it gets cleanup up during PostCollectionCallback.
- Add Jsrttest for setPrototype
- Added an assert
- `PromoteType` called from `SetPrototype` was treating the instance as object literal and was creating a new `DynamicType` with `jsFunctionCallback` field of `JsrtExternalType` set to nullptr. Fixed it.

bin/NativeTests/JsRTApiTest.cpp

@@ -1192,6 +1192,82 @@ namespace JsRTApiTest
         JsRTApiTest::RunWithAttributes(JsRTApiTest::ObjectMethodTest);
     }
 
+    void SetPrototypeTest(JsRuntimeAttributes attributes, JsRuntimeHandle runtime)
+    {
+        JsValueRef proto = JS_INVALID_REFERENCE;
+        JsValueRef object1 = JS_INVALID_REFERENCE;
+        JsValueRef object2 = JS_INVALID_REFERENCE;
+        JsPropertyIdRef obj1_a_pid= JS_INVALID_REFERENCE;
+        JsPropertyIdRef obj1_b_pid = JS_INVALID_REFERENCE;
+        JsPropertyIdRef obj2_x_pid = JS_INVALID_REFERENCE;
+        JsPropertyIdRef obj2_y_pid = JS_INVALID_REFERENCE;
+        JsPropertyIdRef obj2_z_pid = JS_INVALID_REFERENCE;
+        JsValueRef obj1_a_value = JS_INVALID_REFERENCE;
+        JsValueRef obj1_b_value = JS_INVALID_REFERENCE;
+        JsValueRef obj2_x_value = JS_INVALID_REFERENCE;
+        JsValueRef obj2_y_value = JS_INVALID_REFERENCE;
+        JsValueRef obj2_z_value = JS_INVALID_REFERENCE;
+
+        // var obj1 = {a : "obj1.a", b : "obj1.b"};
+        // var obj2 = {x : "obj2.x", y : "obj2.y", z : "obj2.z"}
+        REQUIRE(JsCreateObject(&proto) == JsNoError);
+        REQUIRE(JsCreateExternalObject((void *)0xdeadbeef, ExternalObjectFinalizeCallback, &object1) == JsNoError);
+        REQUIRE(JsCreateExternalObject((void *)0xdeadbeef, ExternalObjectFinalizeCallback, &object2) == JsNoError);
+
+        size_t propNameLength = wcslen(_u("obj1.a"));
+        REQUIRE(JsPointerToString(_u("obj1.a"), propNameLength, &obj1_a_value) == JsNoError);
+        REQUIRE(JsGetPropertyIdFromName(_u("a"), &obj1_a_pid) == JsNoError);
+        REQUIRE(JsSetProperty(object1, obj1_a_pid, obj1_a_value, true) == JsNoError);
+
+        REQUIRE(JsPointerToString(_u("obj1.b"), propNameLength, &obj1_b_value) == JsNoError);
+        REQUIRE(JsGetPropertyIdFromName(_u("b"), &obj1_b_pid) == JsNoError);
+        REQUIRE(JsSetProperty(object1, obj1_b_pid, obj1_b_value, true) == JsNoError);
+
+        REQUIRE(JsPointerToString(_u("obj2.x"), propNameLength, &obj2_x_value) == JsNoError);
+        REQUIRE(JsGetPropertyIdFromName(_u("x"), &obj2_x_pid) == JsNoError);
+        REQUIRE(JsSetProperty(object2, obj2_x_pid, obj2_x_value, true) == JsNoError);
+
+        REQUIRE(JsPointerToString(_u("obj1.y"), propNameLength, &obj2_y_value) == JsNoError);
+        REQUIRE(JsGetPropertyIdFromName(_u("y"), &obj2_y_pid) == JsNoError);
+        REQUIRE(JsSetProperty(object2, obj2_y_pid, obj2_y_value, true) == JsNoError);
+
+        REQUIRE(JsPointerToString(_u("obj1.z"), propNameLength, &obj2_z_value) == JsNoError);
+        REQUIRE(JsGetPropertyIdFromName(_u("z"), &obj2_z_pid) == JsNoError);
+        REQUIRE(JsSetProperty(object2, obj2_z_pid, obj2_z_value, true) == JsNoError);
+
+
+        REQUIRE(JsSetPrototype(object1, proto) == JsNoError);
+        REQUIRE(JsSetPrototype(object2, proto) == JsNoError);
+
+        JsValueRef objectProto = JS_INVALID_REFERENCE;
+
+        REQUIRE(JsGetPrototype(object1, &objectProto) == JsNoError);
+        CHECK(proto == objectProto);
+        REQUIRE(JsGetPrototype(object2, &objectProto) == JsNoError);
+        CHECK(proto == objectProto);
+
+        JsValueRef value = JS_INVALID_REFERENCE;
+        REQUIRE(JsGetProperty(object1, obj1_a_pid, &value) == JsNoError);
+        CHECK(value == obj1_a_value);
+
+        REQUIRE(JsGetProperty(object1, obj1_b_pid, &value) == JsNoError);
+        CHECK(value == obj1_b_value);
+
+        REQUIRE(JsGetProperty(object2, obj2_x_pid, &value) == JsNoError);
+        CHECK(value == obj2_x_value);
+
+        REQUIRE(JsGetProperty(object2, obj2_y_pid, &value) == JsNoError);
+        CHECK(value == obj2_y_value);
+
+        REQUIRE(JsGetProperty(object2, obj2_z_pid, &value) == JsNoError);
+        CHECK(value == obj2_z_value);
+    }
+
+    TEST_CASE("ApiTest_SetPrototypeTest", "[ApiTest]")
+    {
+        JsRTApiTest::RunWithAttributes(JsRTApiTest::SetPrototypeTest);
+    }
+
     void DisableEval(JsRuntimeAttributes attributes, JsRuntimeHandle runtime)
     {
         JsValueRef result = JS_INVALID_REFERENCE;

lib/Jsrt/Jsrt.cpp

@@ -183,7 +183,7 @@ void CALLBACK CreateExternalObject_TTDCallback(Js::ScriptContext* ctx, Js::Var*
 {
     TTDAssert(object != nullptr, "This should always be a valid location");
 
-    *object = RecyclerNewFinalized(ctx->GetRecycler(), JsrtExternalObject, RecyclerNew(ctx->GetRecycler(), JsrtExternalType, ctx, nullptr), nullptr);
+    *object = JsrtExternalObject::Create(nullptr, nullptr, ctx);
 }
 
 static void CALLBACK TTDDummyPromiseContinuationCallback(JsValueRef task, void *callbackState)
@@ -1245,7 +1245,7 @@ CHAKRA_API JsCreateExternalObject(_In_opt_ void *data, _In_opt_ JsFinalizeCallba
 
         PARAM_NOT_NULL(object);
 
-        *object = RecyclerNewFinalized(scriptContext->GetRecycler(), JsrtExternalObject, RecyclerNew(scriptContext->GetRecycler(), JsrtExternalType, scriptContext, finalizeCallback), data);
+        *object = JsrtExternalObject::Create(data, finalizeCallback, scriptContext);
 
         PERFORM_JSRT_TTD_RECORD_ACTION_RESULT(scriptContext, object);
 

lib/Jsrt/JsrtExternalObject.cpp

@@ -27,6 +27,23 @@ JsrtExternalObject::JsrtExternalObject(JsrtExternalType * type, void *data) :
 {
 }
 
+/* static */
+JsrtExternalObject* JsrtExternalObject::Create(void *data, JsFinalizeCallback finalizeCallback, Js::ScriptContext *scriptContext)
+{
+    Js::DynamicType * dynamicType = scriptContext->GetLibrary()->GetCachedJsrtExternalType(reinterpret_cast<uintptr_t>(finalizeCallback));
+
+    if (dynamicType == nullptr)
+    {
+        dynamicType = RecyclerNew(scriptContext->GetRecycler(), JsrtExternalType, scriptContext, finalizeCallback);
+        scriptContext->GetLibrary()->CacheJsrtExternalType(reinterpret_cast<uintptr_t>(finalizeCallback), dynamicType);
+    }
+
+    Assert(dynamicType->IsJsrtExternal());
+    Assert(dynamicType->GetIsShared());
+
+    return RecyclerNewFinalized(scriptContext->GetRecycler(), JsrtExternalObject, static_cast<JsrtExternalType*>(dynamicType), data);
+}
+
 bool JsrtExternalObject::Is(Js::Var value)
 {
     if (Js::TaggedNumber::Is(value))

lib/Jsrt/JsrtExternalObject.h

@@ -49,6 +49,7 @@ public:
 
     static bool Is(Js::Var value);
     static JsrtExternalObject * FromVar(Js::Var value);
+    static JsrtExternalObject * Create(void *data, JsFinalizeCallback finalizeCallback, Js::ScriptContext *scriptContext);
 
     JsrtExternalType * GetExternalType() const { return (JsrtExternalType *)this->GetType(); }
 

lib/Runtime/Library/JavascriptLibrary.cpp

@@ -4735,6 +4735,28 @@ namespace Js
         return function;
     }
 
+    DynamicType* JavascriptLibrary::GetCachedJsrtExternalType(uintptr_t finalizeCallback)
+    {
+        RecyclerWeakReference<DynamicType>* dynamicTypeWeakRef = nullptr;
+        DynamicType* dynamicType = nullptr;
+        if (jsrtExternalTypesCache == nullptr)
+        {
+            jsrtExternalTypesCache = RecyclerNew(recycler, JsrtExternalTypesCache, recycler, 3);
+            // Register for periodic cleanup
+            scriptContext->RegisterWeakReferenceDictionary(jsrtExternalTypesCache);
+        }
+        if (jsrtExternalTypesCache->TryGetValue(finalizeCallback, &dynamicTypeWeakRef))
+        {
+            dynamicType = dynamicTypeWeakRef->Get();
+        }
+        return dynamicType;
+    }
+
+    void JavascriptLibrary::CacheJsrtExternalType(uintptr_t finalizeCallback, DynamicType* dynamicTypeToCache)
+    {
+        jsrtExternalTypesCache->Item(finalizeCallback, recycler->CreateWeakReferenceHandle<DynamicType>(dynamicTypeToCache));
+    }
+
     RuntimeFunction* JavascriptLibrary::DefaultCreateFunction(FunctionInfo * functionInfo, int length, DynamicObject * prototype, DynamicType * functionType, PropertyId nameId)
     {
         Assert(nameId >= Js::InternalPropertyIds::Count && scriptContext->IsTrackedPropertyId(nameId));

lib/Runtime/Library/JavascriptLibrary.h

@@ -449,6 +449,7 @@ namespace Js
         void *nativeHostPromiseContinuationFunctionState;
 
         typedef SList<Js::FunctionProxy*, Recycler> FunctionReferenceList;
+        typedef JsUtil::WeakReferenceDictionary<uintptr_t, DynamicType, DictionarySizePolicy<PowerOf2Policy, 1>> JsrtExternalTypesCache;
 
         void * bindRefChunkBegin;
         void ** bindRefChunkCurrent;
@@ -458,6 +459,7 @@ namespace Js
         FunctionReferenceList* dynamicFunctionReference;
         uint dynamicFunctionReferenceDepth;
         FinalizableObject* jsrtContextObject;
+        JsrtExternalTypesCache* jsrtExternalTypesCache;
 
         typedef JsUtil::BaseHashSet<RecyclerWeakReference<RecyclableObject>*, Recycler, PowerOf2SizePolicy, RecyclerWeakReference<RecyclableObject>*, StringTemplateCallsiteObjectComparer> StringTemplateCallsiteObjectList;
 
@@ -540,6 +542,7 @@ namespace Js
                               identityFunction(nullptr),
                               throwerFunction(nullptr),
                               jsrtContextObject(nullptr),
+                              jsrtExternalTypesCache(nullptr),
                               scriptContextCache(nullptr),
                               externalLibraryList(nullptr),
 #if ENABLE_COPYONACCESS_ARRAY
@@ -968,6 +971,8 @@ namespace Js
         JavascriptExternalFunction* CreateIdMappedExternalFunction(MethodType entryPoint, DynamicType *pPrototypeType);
         JavascriptExternalFunction* CreateExternalConstructor(Js::ExternalMethod entryPoint, PropertyId nameId, RecyclableObject * prototype);
         JavascriptExternalFunction* CreateExternalConstructor(Js::ExternalMethod entryPoint, PropertyId nameId, InitializeMethod method, unsigned short deferredTypeSlots, bool hasAccessors);
+        DynamicType* GetCachedJsrtExternalType(uintptr_t finalizeCallback);
+        void CacheJsrtExternalType(uintptr_t finalizeCallback, DynamicType* dynamicType);
         static DynamicTypeHandler * GetDeferredPrototypeGeneratorFunctionTypeHandler(ScriptContext* scriptContext);
         static DynamicTypeHandler * GetDeferredPrototypeAsyncFunctionTypeHandler(ScriptContext* scriptContext);
         DynamicType * CreateDeferredPrototypeGeneratorFunctionType(JavascriptMethod entrypoint, bool isAnonymousFunction, bool isShared = false);

lib/Runtime/Types/PathTypeHandler.cpp

@@ -1519,8 +1519,7 @@ namespace Js
                                             DynamicTypeHandler::RoundUpInlineSlotCapacity(requestedInlineSlotCapacity));
         ScriptContext* scriptContext = instance->GetScriptContext();
         DynamicType* cachedDynamicType = nullptr;
-        BOOL isJsrtExternalType = instance->GetType()->IsJsrtExternal();
-        DynamicType* oldType = isJsrtExternalType ? 0 : instance->GetDynamicType();
+        DynamicType* oldType = instance->GetDynamicType();
 
         bool useCache = instance->GetScriptContext() == newPrototype->GetScriptContext();
 
@@ -1530,14 +1529,12 @@ namespace Js
         char16 reason[1024];
         swprintf_s(reason, 1024, _u("Cache not populated."));
 #endif
-        AssertMsg(isJsrtExternalType || typeid(DynamicType) == typeid(*oldType), "PathTypeHandler is used either by JsrtExternalType or DynamicType");
-
         if (useCache && newPrototype->GetInternalProperty(newPrototype, Js::InternalPropertyIds::TypeOfPrototypeObjectDictionary, (Js::Var*)&oldTypeToPromotedTypeMap, nullptr, scriptContext))
         {
             Assert(oldTypeToPromotedTypeMap && (Js::Var)oldTypeToPromotedTypeMap != scriptContext->GetLibrary()->GetUndefined());
             oldTypeToPromotedTypeMap = reinterpret_cast<TypeTransitionMap*>(oldTypeToPromotedTypeMap);
 
-            if (oldTypeToPromotedTypeMap->TryGetValue(reinterpret_cast<uintptr_t>(oldType), &cachedDynamicType))
+            if (oldTypeToPromotedTypeMap->TryGetValue(oldType, &cachedDynamicType))
             {
 #if DBG
                 oldCachedType = cachedDynamicType;
@@ -1585,7 +1582,7 @@ namespace Js
                 Js::PropertyId propertyId = GetPropertyId(scriptContext, i);
 
                 PropertyIndex propertyIndex = GetPropertyIndex(propertyId);
-                cachedDynamicType = pathTypeHandler->PromoteType<true>(cachedDynamicType, scriptContext->GetPropertyName(propertyId), true, scriptContext, nullptr, &propertyIndex);
+                cachedDynamicType = pathTypeHandler->PromoteType<false>(cachedDynamicType, scriptContext->GetPropertyName(propertyId), true, scriptContext, instance, &propertyIndex);
             }
 
             if (useCache)
@@ -1596,12 +1593,10 @@ namespace Js
                     newPrototype->SetInternalProperty(Js::InternalPropertyIds::TypeOfPrototypeObjectDictionary, (Var)oldTypeToPromotedTypeMap, PropertyOperationFlags::PropertyOperation_Force, nullptr);
                 }
 
-                // oldType is kind of weakReference here
+                oldTypeToPromotedTypeMap->Item(oldType, cachedDynamicType);
 #if DBG
                 cachedDynamicType->SetIsCachedForChangePrototype();
 #endif
-                oldTypeToPromotedTypeMap->Item(reinterpret_cast<uintptr_t>(oldType), cachedDynamicType);
-
                 if (PHASE_TRACE1(TypeShareForChangePrototypePhase))
                 {
 #if DBG

lib/Runtime/Types/PathTypeHandler.h

@@ -19,7 +19,7 @@ namespace Js
 
     public:
         DEFINE_GETCPPNAME();
-        typedef JsUtil::BaseDictionary<uintptr_t, DynamicType *, Recycler, PowerOf2SizePolicy> TypeTransitionMap;
+        typedef JsUtil::WeaklyReferencedKeyDictionary<DynamicType, DynamicType*> TypeTransitionMap;
 
     protected:
         PathTypeHandlerBase(TypePath* typePath, uint16 pathLength, const PropertyIndex slotCapacity, uint16 inlineSlotCapacity, uint16 offsetOfInlineSlots, bool isLocked = false, bool isShared = false, DynamicType* predecessorType = nullptr);