PlaceInterfaceMethod is now more optimized (dotnet#96703)

* PlaceInterfaceMethod is now more optimized - It naturally has an O(N*M) algorithm, we reduce it to O(N*(Msmaller)), and make the constant factor faster - We take a local copy of the declared methods and name hashes, and the scan is now a straight linear scan of 32 bit memory DWORDS, and naturally skips nonpublic and non virtual methods. --------- Co-authored-by: Aaron Robinson <[email protected]>
steveharter · Jan 10, 2024 · 904c439 · 904c439
1 parent a3775a4
commit 904c439
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 24 deletions.
diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp
@@ -7530,6 +7530,14 @@ MethodTableBuilder::PlaceInterfaceMethods()
     BOOL fParentInterface;
     DispatchMapTypeID * rgInterfaceDispatchMapTypeIDs = NULL;
 
+    // Optimization for fast discovery of possible matches below
+    // Lazily initialized the first time we want to walk the list of methods
+    // The memory allocated for these pointers is on the StackingAllocator, which
+    // has the same lifetime as the MethodTableBuilder
+    uint32_t *pNameHashArray = NULL;
+    bmtMDMethod **pMDMethodArray = NULL;
+    DWORD interfaceImplCandidateArraySize = 0;
+
     for (DWORD dwCurInterface = 0;
          dwCurInterface < bmtInterface->dwInterfaceMapSize;
          dwCurInterface++)
@@ -7719,33 +7727,58 @@ MethodTableBuilder::PlaceInterfaceMethods()
             // First, try to find the method explicitly declared in our class
             //
 
-            DeclaredMethodIterator methIt(*this);
-            while (methIt.Next())
+            if (pNameHashArray == NULL)
             {
-                // Note that non-publics can legally be exposed via an interface, but only
-                // through methodImpls.
-                if (IsMdVirtual(methIt.Attrs()) && IsMdPublic(methIt.Attrs()))
+                S_SIZE_T cbAllocPointers = S_SIZE_T(NumDeclaredMethods()) * S_SIZE_T(sizeof(bmtMDMethod*));
+                S_SIZE_T cbAllocHashes = S_SIZE_T(NumDeclaredMethods()) * S_SIZE_T(sizeof(uint32_t));
+
+                pNameHashArray = (uint32_t *)GetStackingAllocator()->Alloc(cbAllocHashes);
+                pMDMethodArray = (bmtMDMethod **)GetStackingAllocator()->Alloc(cbAllocPointers);
+
+                DeclaredMethodIterator methIt(*this);
+                while (methIt.Next())
                 {
+                    // Note that non-publics and statics can legally be exposed via an interface, but only
+                    // through methodImpls.
+                    bmtMDMethod* mdMethod = methIt.GetMDMethod();
+                    DWORD attrs = mdMethod->GetDeclAttrs();
+                    if (IsMdVirtual(attrs) && IsMdPublic(attrs))
+                    {
+                        pNameHashArray[interfaceImplCandidateArraySize] = mdMethod->GetMethodSignature().GetNameHash();
+                        pMDMethodArray[interfaceImplCandidateArraySize++] = mdMethod;
+                    }
+                }
+            }
+
+            DeclaredMethodIterator methIt(*this);
+            UINT32 nameHashItfMethod = pCurItfMethod->GetMethodSignature().GetNameHash();
+
+            for (DWORD iPublicVirtualNonStaticMethod = 0; iPublicVirtualNonStaticMethod < interfaceImplCandidateArraySize; ++iPublicVirtualNonStaticMethod)
+            {
+                if (pNameHashArray[iPublicVirtualNonStaticMethod] != nameHashItfMethod)
+                    continue;
+
+                bmtMDMethod* declaredMethod = pMDMethodArray[iPublicVirtualNonStaticMethod];
+                const MethodSignature& declaredMethodSig = declaredMethod->GetMethodSignature();
 #ifdef _DEBUG
-                    if(GetHalfBakedClass()->m_fDebuggingClass && g_pConfig->ShouldBreakOnMethod(methIt.Name()))
-                        CONSISTENCY_CHECK_MSGF(false, ("BreakOnMethodName: '%s' ", methIt.Name()));
+                if(GetHalfBakedClass()->m_fDebuggingClass && g_pConfig->ShouldBreakOnMethod(declaredMethodSig.GetName()))
+                    CONSISTENCY_CHECK_MSGF(false, ("BreakOnMethodName: '%s' ", declaredMethodSig.GetName()));
 #endif // _DEBUG
 
-                    if (pCurItfMethod->GetMethodSignature().Equivalent(methIt->GetMethodSignature()))
-                    {
-                        fFoundMatchInBuildingClass = TRUE;
-                        curItfSlot.Impl() = methIt->GetSlotIndex();
+                if (pCurItfMethod->GetMethodSignature().Equivalent(declaredMethodSig))
+                {
+                    fFoundMatchInBuildingClass = TRUE;
+                    curItfSlot.Impl() = declaredMethod->GetSlotIndex();
 
-                        DispatchMapTypeID dispatchMapTypeID =
-                            DispatchMapTypeID::InterfaceClassID(dwCurInterface);
-                        bmtVT->pDispatchMapBuilder->InsertMDMapping(
-                            dispatchMapTypeID,
-                            static_cast<UINT32>(itfSlotIt.CurrentIndex()),
-                            methIt->GetMethodDesc(),
-                            FALSE);
+                    DispatchMapTypeID dispatchMapTypeID =
+                        DispatchMapTypeID::InterfaceClassID(dwCurInterface);
+                    bmtVT->pDispatchMapBuilder->InsertMDMapping(
+                        dispatchMapTypeID,
+                        static_cast<UINT32>(itfSlotIt.CurrentIndex()),
+                        declaredMethod->GetMethodDesc(),
+                        FALSE);
 
-                        break;
-                    }
+                    break;
                 }
             } // end ... try to find method
 

diff --git a/src/coreclr/vm/methodtablebuilder.h b/src/coreclr/vm/methodtablebuilder.h
@@ -808,6 +808,11 @@ class MethodTableBuilder
         operator PCCOR_SIGNATURE() const
             { return GetSignature(); }
 
+        //-----------------------------------------------------------------------------------------
+        // Get a hash of the Name that can be compared with hashes from other MethodSignatures
+        UINT32
+        GetNameHash() const;
+
     protected:
         //-----------------------------------------------------------------------------------------
         Module *                m_pModule;
@@ -835,10 +840,6 @@ class MethodTableBuilder
         void
         GetMethodAttributes() const;
 
-        //-----------------------------------------------------------------------------------------
-        UINT32
-        GetNameHash() const;
-
     private:
         //-----------------------------------------------------------------------------------
         // Private to prevent use.