Reduce types loaded on startup significantly (#120459)

Reduce the number of type loads triggered by complex generic constraints. Experiments show about a 20% reduction in startup time on a proxy of the AWS lambda scenario.

The fixes are:
1. When a method/type has constraints, do not load the constraint types directly if they are generic interfaces. Instead, do accessibility checking and the constraint validation on the signature of the constraint instead.
2. When a type is loaded, enhance the special marker type optimization in the interface map to allow Interfaces which require implementation of other interfaces to use the special marker type. Notably, use the special marker type so that if a generic interface's first type parameter is used as all of the generic type parameters of a required interface we use the special marker type to indicate that the type is loaded.
    - This also forces us to have a somewhat more complicated implementation of casting, since the previous model of special marker types could not experience an equivalent interface match
3. When resolving a default interface method, attempt to avoid loading all the interface types associated with a type. Do so by changing the iteration of interfaces to check to see if the approximate interface could possibly have an implementation
4. When resolving a static virtual method, attempt to avoid loading all the types of the MethodImpl records. Do so by doing a Method name check before loading the type.

In addition, fix a test so that it still throws when loading an invalid type. This required adding a path which forced the interface type to be loaded.

As part of testing this work, the lack of generic variable type safety checks was noticed in crossgen2, and the following changes were made
- Add support for doing circularity checks on type and method generic parameters to the TypeValidationChecker in crossgen2
- Add support for doing variance safety checks to the type loader in crossgen2

And in the runtime
- Remove the circularity of type variables checks happening in generic method load, as it is redundant
- Put all of the variance safety checks in the runtime under the SkipTypeValidation flag as crossgen2 can now do it reliably

Related to the work in #120407

Author: davidwrighton

src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeValidationChecker.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Linq;
 using System.Reflection;
 using System.Text;
@@ -133,6 +134,18 @@ Task<bool> ValidateTypeWorkerHelper(TypeDesc typeToCheckForSkipValidation)
                         AddTypeValidationError(type, $"Interface type {interfaceType} failed validation");
                         return false;
                     }
+
+                    if (!interfaceType.IsInterface)
+                    {
+                        AddTypeValidationError(type, $"Runtime interface {interfaceType} is not an interface");
+                        return false;
+                    }
+
+                    if (interfaceType.HasInstantiation)
+                    {
+                        // Interfaces behave covariantly
+                        ValidateVarianceInType(type.Instantiation, interfaceType, Internal.TypeSystem.GenericVariance.Covariant);
+                    }
                 }
 
                 // Validate that each interface type explicitly implemented on this type is accessible to this type -- UNIMPLEMENTED
@@ -293,14 +306,76 @@ Task<bool> ValidateTypeWorkerHelper(TypeDesc typeToCheckForSkipValidation)
                         AddTypeValidationError(type, $"'{method}' is an runtime-impl generic method");
                         return false;
                     }
-                    // Validate that generic variance is properly respected in method signatures -- UNIMPLEMENTED
-                    // Validate that there are no cyclical method constraints -- UNIMPLEMENTED
+
+
+                    // Validate that generic variance is properly respected in method signatures
+                    if (type.IsInterface && method.IsVirtual && !method.Signature.IsStatic && type.HasInstantiation)
+                    {
+                        if (!ValidateVarianceInSignature(type.Instantiation, method.Signature, Internal.TypeSystem.GenericVariance.Covariant, Internal.TypeSystem.GenericVariance.Contravariant))
+                        {
+                            AddTypeValidationError(type, $"'{method}' has invalid variance in signature");
+                            return false;
+                        }
+                    }
+
+                    foreach (var genericParameterType in method.Instantiation)
+                    {
+                        foreach (var constraint in ((GenericParameterDesc)genericParameterType).TypeConstraints)
+                        {
+                            if (!ValidateVarianceInType(type.Instantiation, constraint, Internal.TypeSystem.GenericVariance.Contravariant))
+                            {
+                                AddTypeValidationError(type, $"'{method}' has invalid variance in generic parameter constraint {constraint} on type {genericParameterType}");
+                                return false;
+                            }
+                        }
+                    }
+
+                    if (!BoundedCheckForInstantiation(method.Instantiation))
+                    {
+                        AddTypeValidationError(type, $"'{method}' has cyclical generic parameter constraints");
+                        return false;
+                    }
                     // Validate that constraints are all acccessible to the method using them -- UNIMPLEMENTED
                 }
 
                 // Generic class special rules
                 // Validate that a generic class cannot be a ComImport class, or a ComEventInterface class -- UNIMPLEMENTED
-                // Validate that there are no cyclical class or method constraints, and that constraints are all acccessible to the type using them -- UNIMPLEMENTED
+
+                foreach (var genericParameterType in type.Instantiation)
+                {
+                    foreach (var constraint in ((GenericParameterDesc)genericParameterType).TypeConstraints)
+                    {
+                        if (!ValidateVarianceInType(type.Instantiation, constraint, Internal.TypeSystem.GenericVariance.Contravariant))
+                        {
+                            AddTypeValidationError(type, $"'{genericParameterType}' has invalid variance in generic parameter constraint {constraint}");
+                            return false;
+                        }
+                    }
+                }
+
+                // Validate that generic variance is properly respected in interface signatures
+                if (type.HasInstantiation && type.IsInterface)
+                {
+                    foreach (var interfaceType in type.ExplicitlyImplementedInterfaces)
+                    {
+                        if (interfaceType.HasInstantiation)
+                        {
+                            // Interfaces behave covariantly
+                            if (!ValidateVarianceInType(type.Instantiation, interfaceType, Internal.TypeSystem.GenericVariance.Covariant))
+                            {
+                                AddTypeValidationError(type, $"'{type}' has invalid variance in in interface impl of {interfaceType}");
+                                return false;
+                            }
+                        }
+                    }
+                }
+
+                // Validate that there are no cyclical class or method constraints, and that constraints are all acccessible to the type using them
+                if (!BoundedCheckForInstantiation(type.Instantiation))
+                {
+                    AddTypeValidationError(type, $"'{type}' has cyclical generic parameter constraints");
+                    return false;
+                }
 
                 // Override rules
                 // Validate that each override results does not violate accessibility rules -- UNIMPLEMENTED
@@ -579,6 +654,114 @@ async Task<bool> ValidateTypeHelperFunctionPointerType(FunctionPointerType funct
                 }
                 return true;
             }
+
+            static bool BoundedCheckForInstantiation(Instantiation instantiation)
+            {
+                foreach (var type in instantiation)
+                {
+                    Debug.Assert(type.IsGenericParameter);
+                    GenericParameterDesc genericParameter = (GenericParameterDesc)type;
+
+                    if (!BoundedCheckForType(genericParameter, instantiation.Length))
+                        return false;
+                }
+                return true;
+            }
+
+            static bool BoundedCheckForType(GenericParameterDesc genericParameter, int maxDepth)
+            {
+                if (maxDepth <= 0)
+                    return false;
+                foreach (var type in genericParameter.TypeConstraints)
+                {
+                    if (type is GenericParameterDesc possiblyCircularGenericParameter)
+                    {
+                        if (possiblyCircularGenericParameter.Kind == genericParameter.Kind)
+                        {
+                            if (!BoundedCheckForType(possiblyCircularGenericParameter, maxDepth - 1))
+                                return false;
+                        }
+                    }
+                }
+                return true;
+            }
+
+            static bool ValidateVarianceInSignature(Instantiation associatedTypeInstantiation, MethodSignature signature, Internal.TypeSystem.GenericVariance returnTypePosition, Internal.TypeSystem.GenericVariance argTypePosition)
+            {
+                for (int i = 0; i < signature.Length; i++)
+                {
+                    if (!ValidateVarianceInType(associatedTypeInstantiation, signature[i], argTypePosition))
+                        return false;
+                }
+
+                if (!ValidateVarianceInType(associatedTypeInstantiation, signature.ReturnType, returnTypePosition))
+                    return false;
+
+                return true;
+            }
+
+            static bool ValidateVarianceInType(Instantiation associatedTypeInstantiation, TypeDesc type, Internal.TypeSystem.GenericVariance position)
+            {
+                if (type is SignatureTypeVariable signatureTypeVar)
+                {
+                    GenericParameterDesc gp = (GenericParameterDesc)associatedTypeInstantiation[signatureTypeVar.Index];
+                    if (gp.Variance != Internal.TypeSystem.GenericVariance.None)
+                    {
+                        if (position != gp.Variance)
+                        {
+                            // Covariant and contravariant parameters can *only* appear in resp. covariant and contravariant positions
+                            return false;
+                        }
+                    }
+                }
+                else if (type is InstantiatedType instantiatedType)
+                {
+                    var typeDef = instantiatedType.GetTypeDefinition();
+                    if (typeDef.IsValueType || position == Internal.TypeSystem.GenericVariance.None)
+                    {
+                        // Value types and non-variant positions require all parameters to be non-variant
+                        foreach (TypeDesc instType in instantiatedType.Instantiation)
+                        {
+                            if (!ValidateVarianceInType(associatedTypeInstantiation, instType, Internal.TypeSystem.GenericVariance.None))
+                                return false;
+                        }
+                    }
+                    else
+                    {
+                        int index = 0;
+                        for (index = 0; index < typeDef.Instantiation.Length; index++)
+                        {
+                            Internal.TypeSystem.GenericVariance positionForParameter = ((GenericParameterDesc)typeDef.Instantiation[index]).Variance;
+                            // If the surrounding context is contravariant then we need to flip the variance of this parameter
+                            if (position == Internal.TypeSystem.GenericVariance.Contravariant)
+                            {
+                                if (positionForParameter == Internal.TypeSystem.GenericVariance.Covariant)
+                                    positionForParameter = Internal.TypeSystem.GenericVariance.Contravariant;
+                                else if (positionForParameter == Internal.TypeSystem.GenericVariance.Contravariant)
+                                    positionForParameter = Internal.TypeSystem.GenericVariance.Covariant;
+                                else
+                                    positionForParameter = Internal.TypeSystem.GenericVariance.None;
+                            }
+
+                            if (!ValidateVarianceInType(associatedTypeInstantiation, instantiatedType.Instantiation[index], positionForParameter))
+                                return false;
+                        }
+                    }
+                }
+                else if (type is ParameterizedType parameterizedType)
+                {
+                    // Arrays behave as covariant, byref and pointer types are non-variant
+                    if (!ValidateVarianceInType(associatedTypeInstantiation, parameterizedType.ParameterType, (parameterizedType.IsByRef || parameterizedType.IsPointer) ? Internal.TypeSystem.GenericVariance.None : position))
+                        return false;
+                }
+                else if (type is FunctionPointerType functionPointerType)
+                {
+                    if (!ValidateVarianceInSignature(associatedTypeInstantiation, functionPointerType.Signature, Internal.TypeSystem.GenericVariance.None, Internal.TypeSystem.GenericVariance.None))
+                        return false;
+                }
+
+                return true;
+            }
         }
     }
 }

src/coreclr/vm/ceeload.cpp

@@ -434,10 +434,16 @@ void Module::Initialize(AllocMemTracker *pamTracker, LPCWSTR szName)
     AllocateMaps();
     m_dwTransientFlags &= ~((DWORD)CLASSES_FREED);  // Set flag indicating LookupMaps are now in a consistent and destructable state
 
+    if (IsSystem())
+        m_dwPersistedFlags |= SKIP_TYPE_VALIDATION; // Skip type validation on System
+
 #ifdef FEATURE_READYTORUN
     m_pNativeImage = NULL;
     if ((m_pReadyToRunInfo = ReadyToRunInfo::Initialize(this, pamTracker)) != NULL)
     {
+        if (m_pReadyToRunInfo->SkipTypeValidation())
+            m_dwPersistedFlags |= SKIP_TYPE_VALIDATION; // Skip type validation on System
+
         m_pNativeImage = m_pReadyToRunInfo->GetNativeImage();
         if (m_pNativeImage != NULL)
         {

src/coreclr/vm/ceeload.h

@@ -686,6 +686,8 @@ class Module : public ModuleBase
         RUNTIME_MARSHALLING_ENABLED_IS_CACHED = 0x00008000,
         //If runtime marshalling is enabled for this assembly
         RUNTIME_MARSHALLING_ENABLED = 0x00010000,
+
+        SKIP_TYPE_VALIDATION = 0x00020000,
     };
 
     Volatile<DWORD>          m_dwTransientFlags;
@@ -1500,6 +1502,12 @@ class Module : public ModuleBase
 #endif
     }
 
+    bool SkipTypeValidation() const
+    {
+        LIMITED_METHOD_DAC_CONTRACT;
+
+        return (m_dwPersistedFlags & SKIP_TYPE_VALIDATION) != 0;
+    }
 #ifdef FEATURE_READYTORUN
     PTR_ReadyToRunInfo GetReadyToRunInfo() const
     {

src/coreclr/vm/class.cpp

@@ -941,7 +941,7 @@ EEClass::CheckVarianceInSig(
                 uint32_t cArgs;
                 IfFailThrow(psig.GetData(&cArgs));
 
-                // Conservatively, assume non-variance of function pointer types
+                // Conservatively, assume non-variance of function pointer types, if we ever change this, update the TypeValidationChecker in crossgen2 also
                 if (!CheckVarianceInSig(numGenericArgs, pVarianceInfo, pModule, psig, gpNonVariant))
                     return FALSE;
 
@@ -1359,7 +1359,7 @@ void ClassLoader::ValidateMethodsWithCovariantReturnTypes(MethodTable* pMT)
         return;
 
     // Step 1: Validate compatibility of return types on overriding methods
-    if (pMT->GetClass()->HasCovariantOverride() && (!pMT->GetModule()->IsReadyToRun() || !pMT->GetModule()->GetReadyToRunInfo()->SkipTypeValidation()))
+    if (pMT->GetClass()->HasCovariantOverride() && !pMT->GetModule()->SkipTypeValidation())
     {
         for (WORD i = 0; i < pParentMT->GetNumVirtuals(); i++)
         {

src/coreclr/vm/comdelegate.cpp

@@ -2403,16 +2403,16 @@ static bool IsLocationAssignable(TypeHandle fromHandle, TypeHandle toHandle, boo
                 // We need to check whether constraints of fromHandle have been loaded, because the
                 // CanCastTo operation might have made its decision without enumerating constraints
                 // (e.g. when toHandle is System.Object).
-                if (!fromHandleVar->ConstraintsLoaded())
-                    fromHandleVar->LoadConstraints(CLASS_DEPENDENCIES_LOADED);
+                if (!fromHandleVar->ConstraintsLoaded(WhichConstraintsToLoad::TypeOrMethodVarsAndNonInterfacesOnly))
+                    fromHandleVar->LoadConstraints(CLASS_DEPENDENCIES_LOADED, WhichConstraintsToLoad::TypeOrMethodVarsAndNonInterfacesOnly);
 
                 if (toHandle.IsGenericVariable())
                 {
                     TypeVarTypeDesc *toHandleVar = toHandle.AsGenericVariable();
 
                     // Constraints of toHandleVar were not touched by CanCastTo.
-                    if (!toHandleVar->ConstraintsLoaded())
-                        toHandleVar->LoadConstraints(CLASS_DEPENDENCIES_LOADED);
+                    if (!toHandleVar->ConstraintsLoaded(WhichConstraintsToLoad::TypeOrMethodVarsAndNonInterfacesOnly))
+                        toHandleVar->LoadConstraints(CLASS_DEPENDENCIES_LOADED, WhichConstraintsToLoad::TypeOrMethodVarsAndNonInterfacesOnly);
 
                     // Both handles are type variables. The following table lists all possible combinations.
                     //

src/coreclr/vm/genmeth.cpp

@@ -1536,7 +1536,7 @@ BOOL Bounded(TypeVarTypeDesc *tyvar, DWORD depth) {
     }
 
     DWORD numConstraints;
-    TypeHandle *constraints = tyvar->GetConstraints(&numConstraints, CLASS_DEPENDENCIES_LOADED);
+    TypeHandle *constraints = tyvar->GetConstraints(&numConstraints, CLASS_DEPENDENCIES_LOADED, WhichConstraintsToLoad::TypeOrMethodVarsAndNonInterfacesOnly);
     for (unsigned i = 0; i < numConstraints; i++)
     {
         TypeHandle constraint = constraints[i];
@@ -1554,56 +1554,31 @@ BOOL Bounded(TypeVarTypeDesc *tyvar, DWORD depth) {
     return TRUE;
 }
 
-void MethodDesc::LoadConstraintsForTypicalMethodDefinition(BOOL *pfHasCircularClassConstraints, BOOL *pfHasCircularMethodConstraints, ClassLoadLevel level/* = CLASS_LOADED*/)
+void MethodDesc::CheckConstraintMetadataValidity(BOOL *pfHasCircularMethodConstraints)
 {
     CONTRACTL {
-        THROWS;
-        GC_TRIGGERS;
-        MODE_ANY;
+        STANDARD_VM_CHECK;
         PRECONDITION(IsTypicalMethodDefinition());
-        PRECONDITION(CheckPointer(pfHasCircularClassConstraints));
         PRECONDITION(CheckPointer(pfHasCircularMethodConstraints));
     } CONTRACTL_END;
 
-    *pfHasCircularClassConstraints = FALSE;
+    // In this function we explicitly check for accessibility of method type parameter constraints as
+    // well as explicitly do a check for circularity among method type parameter constraints.
+    //
+    // For checking the variance of the constraints we rely on the fact that both DoAccessibilityCheckForConstraints
+    // and Bounded will call GetConstraints on the type variables, which will in turn call
+    // LoadConstraints, and LoadConstraints will do the variance checking using EEClass::CheckVarianceInSig
     *pfHasCircularMethodConstraints = FALSE;
 
-    // Force a load of the constraints on the type parameters
-    Instantiation classInst = GetClassInstantiation();
-    for (DWORD i = 0; i < classInst.GetNumArgs(); i++)
-    {
-        TypeVarTypeDesc* tyvar = classInst[i].AsGenericVariable();
-        _ASSERTE(tyvar != NULL);
-        tyvar->LoadConstraints(level);
-    }
-
     Instantiation methodInst = GetMethodInstantiation();
-    for (DWORD i = 0; i < methodInst.GetNumArgs(); i++)
-    {
-        TypeVarTypeDesc* tyvar = methodInst[i].AsGenericVariable();
-        _ASSERTE(tyvar != NULL);
-        tyvar->LoadConstraints(level);
-
-        VOID DoAccessibilityCheckForConstraints(MethodTable *pAskingMT, TypeVarTypeDesc *pTyVar, UINT resIDWhy);
-        DoAccessibilityCheckForConstraints(GetMethodTable(), tyvar, E_ACCESSDENIED);
-    }
-
-    // reject circular class constraints
-    for (DWORD i = 0; i < classInst.GetNumArgs(); i++)
-    {
-        TypeVarTypeDesc* tyvar = classInst[i].AsGenericVariable();
-        _ASSERTE(tyvar != NULL);
-        if(!Bounded(tyvar, classInst.GetNumArgs()))
-        {
-            *pfHasCircularClassConstraints = TRUE;
-        }
-    }
 
     // reject circular method constraints
     for (DWORD i = 0; i < methodInst.GetNumArgs(); i++)
     {
         TypeVarTypeDesc* tyvar = methodInst[i].AsGenericVariable();
         _ASSERTE(tyvar != NULL);
+        VOID DoAccessibilityCheckForConstraints(MethodTable *pAskingMT, TypeVarTypeDesc *pTyVar, UINT resIDWhy);
+        DoAccessibilityCheckForConstraints(GetMethodTable(), tyvar, E_ACCESSDENIED);
         if(!Bounded(tyvar, methodInst.GetNumArgs()))
         {
             *pfHasCircularMethodConstraints = TRUE;
@@ -1662,8 +1637,6 @@ BOOL MethodDesc::SatisfiesMethodConstraints(TypeHandle thParent, BOOL fThrowIfNo
         _ASSERTE(tyvar != NULL);
         _ASSERTE(TypeFromToken(tyvar->GetTypeOrMethodDef()) == mdtMethodDef);
 
-        tyvar->LoadConstraints(); //TODO: is this necessary for anything but the typical method?
-
         // Pass in the InstatiationContext so constraints can be correctly evaluated
         // if this is an instantiation where the type variable is in its open position
         if (!tyvar->SatisfiesConstraints(&typeContext,thArg, typicalInstMatchesMethodInst ? &instContext : NULL))