Improve CheckIfTypeIsEquivalent

`CheckIfTypeIsEquivalent` helper is called from JITted code to perform equivalent type check.
It validates if the object's type is present in one of the 8 cached equivalent
types and if yes, updates the guard value and return back to JITted code.
If not, it takes slow path and verifies the prototype of object to that of cached
type. However if guard is not valid, we would just bailout `BailOutFailedEquivalentTypeCheck`.

Today in `PreSweepCallback` we would invalidate the guard if the underline type is not marked.
However we could have invalid guard value but if there are alive types present in equivTypes[], we could
use them to check if `type` in question matches any of them and if yes, update the guard value, so next
time JIT code will be able to take advantage of it next time.

On instrumenting code, noticed that there are high frequency that this happens (at least in acmeair) where
we have valid equivTypes[], but we don'take take its advantage just because guard value is invalid.
This gives approx. 2.5 % win in acmeair. Other benchmarks are flat.

Also, I noticed that the cache eviction policy was kicking out valid caches
despite of having empty slots in types[] where new type could have been
stored. Modified the eviction logic to take advantage of empty slots before
going to existing eviction logic.

I have also added logic to compact the equivTypes by moving non-null types towards front of array.

Test: Internal tools pass
Perf: Standard benchmarks are almost flat with minor improvement in Octane, Kraken. Acme-air shows 2.5% win.

Author: kunalspathak

lib/Runtime/Base/FunctionBody.cpp

@@ -8775,14 +8775,17 @@ namespace Js
             Type *type = reinterpret_cast<Type*>(this->guard->GetValue());
             if (!recycler->IsObjectMarked(type))
             {
-                this->guard->Invalidate();
+                this->guard->InvalidateDuringSweep();
             }
             else
             {
                 isAnyTypeLive = true;
             }
         }
-
+        uint16 nonNullIndex = 0;
+#if DBG
+        bool isGuardValuePresent = false;
+#endif
         for (int i = 0; i < EQUIVALENT_TYPE_CACHE_SIZE; i++)
         {
             Type *type = this->types[i];
@@ -8794,11 +8797,29 @@ namespace Js
                 }
                 else
                 {
+                    // compact the types array by moving non-null types
+                    // at the beginning.
+                    this->types[nonNullIndex++] = type;
                     isAnyTypeLive = true;
+#if DBG
+                    isGuardValuePresent = this->guard->GetValue() == reinterpret_cast<intptr_t>(type) ? true : isGuardValuePresent;
+#endif
                 }
             }
         }
+        if (nonNullIndex > 0)
+        {
+            memset((void*)(this->types + nonNullIndex), 0, sizeof(Js::Type*) * (EQUIVALENT_TYPE_CACHE_SIZE - nonNullIndex));
+        }
+        else if(guard->IsInvalidatedDuringSweep())
+        {
+            // just mark this as actual invalidated since there are no types
+            // present
+            guard->Invalidate();
+        }
 
+        // verify if guard value is valid, it is present in one of the types
+        AssertMsg(!this->guard->IsValid() || isGuardValuePresent, "After ClearUnusedTypes, valid guard value should be one of the cached equivalent types.");
         return isAnyTypeLive;
     }
 

lib/Runtime/Base/FunctionBody.h

@@ -73,20 +73,53 @@ namespace Js
     {
         friend class PropertyGuardValidator;
     private:
+        enum GuardValue : intptr_t {
+            Invalidated = 0,
+            Uninitialized = 1,
+            Invalidated_DuringSweep = 2
+        };
         intptr_t value;
+#if DBG
+        bool wasReincarnated = false;
+#endif
     public:
         static PropertyGuard* New(Recycler* recycler) { return RecyclerNewLeaf(recycler, Js::PropertyGuard); }
-        PropertyGuard() : value(1) {}
-        PropertyGuard(intptr_t value) : value(value) { Assert(this->value != 0); }
+        PropertyGuard() : value(GuardValue::Uninitialized) {}
+        PropertyGuard(intptr_t value) : value(value) 
+        { 
+            // GuardValue::Invalidated and GuardValue::Invalidated_DuringSweeping can only be set using
+            // Invalidate() and InvalidatedDuringSweep() methods respectively.
+            Assert(this->value != GuardValue::Invalidated && this->value != GuardValue::Invalidated_DuringSweep); 
+        }
 
         inline static size_t const GetSizeOfValue() { return sizeof(((PropertyGuard*)0)->value); }
         inline static size_t const GetOffsetOfValue() { return offsetof(PropertyGuard, value); }
 
         intptr_t GetValue() const { return this->value; }
-        bool IsValid() { return this->value != 0; }
-        void SetValue(intptr_t value) { Assert(value != 0); this->value = value; }
+        bool IsValid()
+        {
+            return this->value != GuardValue::Invalidated && this->value != GuardValue::Invalidated_DuringSweep;
+        }
+        bool IsInvalidatedDuringSweep() { return this->value == GuardValue::Invalidated_DuringSweep; }
+        void SetValue(intptr_t value)
+        { 
+            // GuardValue::Invalidated and GuardValue::Invalidated_DuringSweeping can only be set using
+            // Invalidate() and InvalidatedDuringSweep() methods respectively.
+            Assert(value != GuardValue::Invalidated && value != GuardValue::Invalidated_DuringSweep);
+            this->value = value;
+        }
         intptr_t const* GetAddressOfValue() { return &this->value; }
-        void Invalidate() { this->value = 0; }
+        void Invalidate() { this->value = GuardValue::Invalidated; }
+        void InvalidateDuringSweep()
+        { 
+#if DBG
+            wasReincarnated = true;
+#endif
+            this->value = GuardValue::Invalidated_DuringSweep;
+        }
+#if DBG
+        bool WasReincarnated() { return this->wasReincarnated; }
+#endif
     };
 
     class PropertyGuardValidator

lib/Runtime/Language/FunctionCodeGenJitTimeData.cpp

@@ -667,20 +667,17 @@ namespace Js
 
         if (PHASE_TRACE(Js::ObjTypeSpecPhase, topFunctionBody) || PHASE_TRACE(Js::EquivObjTypeSpecPhase, topFunctionBody))
         {
-            if (PHASE_TRACE(Js::ObjTypeSpecPhase, topFunctionBody) || PHASE_TRACE(Js::EquivObjTypeSpecPhase, topFunctionBody))
+            if (typeSet)
             {
-                if (typeSet)
+                const PropertyRecord* propertyRecord = scriptContext->GetPropertyName(propertyId);
+                Output::Print(_u("Created ObjTypeSpecFldInfo: id %u, property %s(#%u), slot %u, type set: "),
+                              id, propertyRecord->GetBuffer(), propertyId, slotIndex);
+                for (uint16 ti = 0; ti < typeCount - 1; ti++)
                 {
-                    const PropertyRecord* propertyRecord = scriptContext->GetPropertyName(propertyId);
-                    Output::Print(_u("Created ObjTypeSpecFldInfo: id %u, property %s(#%u), slot %u, type set: "),
-                        id, propertyRecord->GetBuffer(), propertyId, slotIndex);
-                    for (uint16 ti = 0; ti < typeCount - 1; ti++)
-                    {
-                        Output::Print(_u("0x%p, "), typeSet->GetType(ti));
-                    }
-                    Output::Print(_u("0x%p\n"), typeSet->GetType(typeCount - 1));
-                    Output::Flush();
+                    Output::Print(_u("0x%p, "), typeSet->GetType(ti));
                 }
+                Output::Print(_u("0x%p\n"), typeSet->GetType(typeCount - 1));
+                Output::Flush();
             }
         }
 

lib/Runtime/Language/JavascriptOperators.cpp

@@ -8197,9 +8197,9 @@ namespace Js
             return false;
         }
 
-        if (guard->GetType()->GetScriptContext() != type->GetScriptContext())
+        if (!guard->IsInvalidatedDuringSweep() && guard->GetType()->GetScriptContext() != type->GetScriptContext())
         {
-            // Can't cache cross-context objects
+            // For valid guard value, can't cache cross-context objects
             return false;
         }
 
@@ -8215,6 +8215,17 @@ namespace Js
         if (type == equivTypes[0] || type == equivTypes[1] || type == equivTypes[2] || type == equivTypes[3] ||
             type == equivTypes[4] || type == equivTypes[5] || type == equivTypes[6] || type == equivTypes[7])
         {
+#if DBG
+            if (PHASE_TRACE1(Js::EquivObjTypeSpecPhase))
+            {
+                if (guard->WasReincarnated())
+                {
+                    Output::Print(_u("EquivObjTypeSpec: Guard 0x%p was reincarnated and working now \n"), guard);
+                    Output::Flush();
+                }
+            }
+#endif
+            
             guard->SetType(type);
             return true;
         }
@@ -8233,6 +8244,12 @@ namespace Js
         Type* refType = equivTypes[0];
         if (refType == nullptr)
         {
+#if DBG
+            for (int i = 1;i < EQUIVALENT_TYPE_CACHE_SIZE;i++)
+            {
+                AssertMsg(equivTypes[i] == nullptr, "In equiv typed caches, if first element is nullptr, all others should be nullptr");
+            }
+#endif
             return false;
         }
 
@@ -8292,38 +8309,66 @@ namespace Js
             return false;
         }
 
-        // CONSIDER (EquivObjTypeSpec): Invent some form of least recently used eviction scheme.
-        uintptr_t index = (reinterpret_cast<uintptr_t>(type) >> 4) & (EQUIVALENT_TYPE_CACHE_SIZE - 1);
-        if (cache->nextEvictionVictim == EQUIVALENT_TYPE_CACHE_SIZE)
+        int emptySlotIndex = -1;
+        for (int i = 0;i < EQUIVALENT_TYPE_CACHE_SIZE;i++)
         {
-            __analysis_assume(index < EQUIVALENT_TYPE_CACHE_SIZE);
-            if (equivTypes[index] != nullptr)
+            if (equivTypes[i] == nullptr)
             {
-                uintptr_t initialIndex = index;
-                index = (initialIndex + 1) & (EQUIVALENT_TYPE_CACHE_SIZE - 1);
-                for (; index != initialIndex; index = (index + 1) & (EQUIVALENT_TYPE_CACHE_SIZE - 1))
-                {
-                    if (equivTypes[index] == nullptr) break;
-                }
-            }
-            __analysis_assume(index < EQUIVALENT_TYPE_CACHE_SIZE);
-            if (equivTypes[index] != nullptr)
+                emptySlotIndex = i;
+                break;
+            };
+        }
+
+        // We have some empty slots, let us use those first
+        if (emptySlotIndex != -1)
+        {
+            if (PHASE_TRACE1(Js::EquivObjTypeSpecPhase))
             {
-                cache->nextEvictionVictim = 0;
+                Output::Print(_u("EquivObjTypeSpec: Saving type in unused slot of equiv types cache. \n"));
+                Output::Flush();
             }
+            equivTypes[emptySlotIndex] = type;
         }
         else
         {
-            Assert(cache->nextEvictionVictim < EQUIVALENT_TYPE_CACHE_SIZE);
-            __analysis_assume(cache->nextEvictionVictim < EQUIVALENT_TYPE_CACHE_SIZE);
-            equivTypes[cache->nextEvictionVictim] = equivTypes[index];
-            cache->nextEvictionVictim = (cache->nextEvictionVictim + 1) & (EQUIVALENT_TYPE_CACHE_SIZE - 1);
-        }
-
-        Assert(index < EQUIVALENT_TYPE_CACHE_SIZE);
-        __analysis_assume(index < EQUIVALENT_TYPE_CACHE_SIZE);
-        equivTypes[index] = type;
+            // CONSIDER (EquivObjTypeSpec): Invent some form of least recently used eviction scheme.
+            uintptr_t index = (reinterpret_cast<uintptr_t>(type) >> 4) & (EQUIVALENT_TYPE_CACHE_SIZE - 1);
+            if (cache->nextEvictionVictim == EQUIVALENT_TYPE_CACHE_SIZE)
+            {
+                __analysis_assume(index < EQUIVALENT_TYPE_CACHE_SIZE);
+                if (equivTypes[index] != nullptr)
+                {
+                    uintptr_t initialIndex = index;
+                    index = (initialIndex + 1) & (EQUIVALENT_TYPE_CACHE_SIZE - 1);
+                    for (; index != initialIndex; index = (index + 1) & (EQUIVALENT_TYPE_CACHE_SIZE - 1))
+                    {
+                        if (equivTypes[index] == nullptr) break;
+                    }
+                }
+                __analysis_assume(index < EQUIVALENT_TYPE_CACHE_SIZE);
+                if (equivTypes[index] != nullptr)
+                {
+                    cache->nextEvictionVictim = 0;
+                }
+            }
+            else
+            {
+                Assert(cache->nextEvictionVictim < EQUIVALENT_TYPE_CACHE_SIZE);
+                __analysis_assume(cache->nextEvictionVictim < EQUIVALENT_TYPE_CACHE_SIZE);
+                equivTypes[cache->nextEvictionVictim] = equivTypes[index];
+                cache->nextEvictionVictim = (cache->nextEvictionVictim + 1) & (EQUIVALENT_TYPE_CACHE_SIZE - 1);
+            }
 
+            if (PHASE_TRACE1(Js::EquivObjTypeSpecPhase))
+            {
+                Output::Print(_u("EquivObjTypeSpec: Saving type in used slot of equiv types cache at index = %d. NextEvictionVictim = %d. \n"), index, cache->nextEvictionVictim);
+                Output::Flush();
+            }
+            Assert(index < EQUIVALENT_TYPE_CACHE_SIZE);
+            __analysis_assume(index < EQUIVALENT_TYPE_CACHE_SIZE);
+            equivTypes[index] = type;
+        }
+        
         // Fixed field checks allow us to assume a specific type ID, but the assumption is only
         // valid if we lock the type. Otherwise, the type ID may change out from under us without
         // evolving the type.