Optimize MethodDataObject::FillEntryDataForAncestor for MethodImpl hierarchies

src/coreclr/vm/methodtable.cpp

@@ -6698,7 +6698,76 @@ void MethodTable::MethodDataObject::FillEntryDataForAncestor(MethodTable * pMT)
     if (m_containsMethodImpl && pMT != m_pDeclMT)
         return;
 
+    NewArrayHolder<bool> pSlotFlags;
+    bool nonAllocatedBitvector[16];
     unsigned nVirtuals = pMT->GetNumVirtuals();
+    unsigned unprocessedNonOverriddenSlots = 0;
+
+    bool* pBitVector = NULL;
+
+    if (pMT == m_pDeclMT)
+    {
+        // We have a concept of methods which have never been overridden by a subclass or called, and in that case we don't actually
+        // need to fill in the MethodTable's vtable entry with a stub. We can detect that here on the Canonical methodtable,
+        // and setup the Decl/Impl MethodDescs for a slot even if there are MethodImpls in the inheritance chain, since the
+        // vtable entry will only be NULL if the method could not have been involved with a MethodImpl. This optimization
+        // is only really important for the scenario where we have these NULL entries, since the MethodImpl fallback
+        // case will end up using MethodTable::GetMethodDescForSlot_NoThrow to get the MethodDesc for the slot, and that is O(N)
+        // for the number of Method defined in the type hierarchy, and the usage of MethodDataObject tends to be O(V) for the number
+        // of virtual method slots on the type, so we get O(V*N) processing time when the not MethodImpl optimization case
+        // fails, which needs addressing.
+
+        // This optimization is only an improvement, if there is a type which is containsMethodImpl in the hierarchy, so do not run it
+        // unless there is a MethodImpl in the hierarchy.
+        bool containsMethodImplInHierarchy = m_containsMethodImpl;
+        MethodTable *pMTWalk = pMT;
+        while (!containsMethodImplInHierarchy && pMTWalk != NULL)
+        {
+            containsMethodImplInHierarchy = pMTWalk->GetClass()->ContainsMethodImpls();
+            pMTWalk = pMTWalk->GetParentMethodTable();
+        }
+
+        if (containsMethodImplInHierarchy)
+        {
+            MethodTable *pCanonMT = pMT->GetCanonicalMethodTable();
+
+            if (nVirtuals <= ARRAY_SIZE(nonAllocatedBitvector))
+            {
+                pBitVector = nonAllocatedBitvector;
+                memset(pBitVector, 0, sizeof(nonAllocatedBitvector));
+            }
+            else
+            {
+                // Use a non-throwing allocation to keep this method within its NOTHROW contract.
+                // If the allocation fails, pBitVector remains NULL and we simply fall back to the
+                // conservative (correct, but potentially slower) behavior below.
+                pSlotFlags = new (nothrow) bool[nVirtuals];
+                pBitVector = pSlotFlags;
+                if (pBitVector != NULL)
+                    memset(pBitVector, 0, nVirtuals * sizeof(*pBitVector));
+            }
+
+            if (pBitVector != NULL)
+            {
+                for (unsigned slot = 0; slot < nVirtuals; slot++)
+                {
+                    PCODE pCode = pCanonMT->GetSlotForVirtualVolatileLoadWithoutBarrier(slot);
+
+                    if (pCode == (PCODE)NULL)
+                    {
+                        pBitVector[slot] = true;
+                        unprocessedNonOverriddenSlots += 1;
+                    }
+                }
+
+                if (unprocessedNonOverriddenSlots == 0)
+                {
+                    pBitVector = NULL;
+                }
+            }
+        }
+    }
+
     unsigned nVTableLikeSlots = pMT->GetCanonicalMethodTable()->GetNumVtableSlots();
 
     MethodTable::IntroducedMethodIterator it(pMT, FALSE);
@@ -6714,8 +6783,20 @@ void MethodTable::MethodDataObject::FillEntryDataForAncestor(MethodTable * pMT)
         // data for, and the virtual methods of the parent and above
         if (pMT == m_pDeclMT)
         {
-            if (m_containsMethodImpl && slot < nVirtuals)
-                continue;
+            if (slot < nVirtuals)
+            {
+                if (pBitVector == NULL || !pBitVector[slot])
+                {
+                    if (m_containsMethodImpl)
+                        continue;
+                }
+                else
+                {
+                    _ASSERTE(unprocessedNonOverriddenSlots > 0);
+                    pBitVector[slot] = false;
+                    unprocessedNonOverriddenSlots -= 1;
+                }
+            }
 
             if (m_virtualsOnly && slot >= nVTableLikeSlots)
             {
@@ -6741,6 +6822,50 @@ void MethodTable::MethodDataObject::FillEntryDataForAncestor(MethodTable * pMT)
             pEntry->SetImplMethodDesc(pMD);
         }
     }
+
+    while (unprocessedNonOverriddenSlots > 0)
+    {
+        _ASSERTE(pBitVector != NULL);
+        _ASSERTE(unprocessedNonOverriddenSlots > 0);
+        pMT = pMT->GetParentMethodTable();
+        _ASSERTE(pMT != NULL);
+        nVirtuals = pMT->GetNumVirtuals();
+        MethodTable::IntroducedMethodIterator it(pMT, FALSE);
+        for (; it.IsValid(); it.Next())
+        {
+            MethodDesc * pMD = it.GetMethodDesc();
+
+            unsigned slot = pMD->GetSlot();
+            if (slot == MethodTable::NO_SLOT)
+                continue;
+
+            if (slot >= nVirtuals)
+                continue;
+
+            if (!pBitVector[slot])
+            {
+                continue;
+            }
+            else
+            {
+                _ASSERTE(unprocessedNonOverriddenSlots > 0);
+                pBitVector[slot] = false;
+                unprocessedNonOverriddenSlots -= 1;
+            }
+
+            MethodDataObjectEntry * pEntry = GetEntry(slot);
+
+            if (pEntry->GetDeclMethodDesc() == NULL)
+            {
+                pEntry->SetDeclMethodDesc(pMD);
+            }
+
+            if (pEntry->GetImplMethodDesc() == NULL)
+            {
+                pEntry->SetImplMethodDesc(pMD);
+            }
+        }
+    }
 } // MethodTable::MethodDataObject::FillEntryDataForAncestor
 
 //==========================================================================================

src/coreclr/vm/methodtable.h

@@ -1591,6 +1591,17 @@ class MethodTable
         return *GetSlotPtrRaw(slotNumber);
     }
 
+#ifndef DACCESS_COMPILE
+    PCODE GetSlotForVirtualVolatileLoadWithoutBarrier(UINT32 slotNum)
+    {
+        LIMITED_METHOD_CONTRACT;
+
+        CONSISTENCY_CHECK(slotNum < GetNumVirtuals());
+        // Virtual slots live in chunks pointed to by vtable indirections
+        return VolatileLoadWithoutBarrier(GetVtableIndirections()[GetIndexOfVtableIndirection(slotNum)] + GetIndexAfterVtableIndirection(slotNum));
+    }
+#endif // DACCESS_COMPILE
+
     // Special-case for when we know that the slot number corresponds
     // to a virtual method.
     inline PCODE GetSlotForVirtual(UINT32 slotNum)