Optimized/fixed TypeLocator.GetGenericInterfaceImplementation (#857)

This method was identified as a hot path in profiling JsonApiDotNetCoreExample tests.

Type.GetGenericTypeDefinition() on the incoming parameter was called inside the loop, which is unneeded. And it compared generic type parameters before looking at the type itself, potentially evaluating too much.

This change makes it run 30% faster. This change also improves input validation and fixes the bug where only the first type parameter was matched against found types.

Author: Bart Koelman

src/JsonApiDotNetCore/Configuration/ServiceDiscoveryFacade.cs

@@ -185,10 +185,10 @@ private void AddResourceDefinitions(Assembly assembly, ResourceDescriptor resour
         private void RegisterImplementations(Assembly assembly, Type interfaceType, ResourceDescriptor resourceDescriptor)
         {
             var genericArguments = interfaceType.GetTypeInfo().GenericTypeParameters.Length == 2 ? new[] { resourceDescriptor.ResourceType, resourceDescriptor.IdType } : new[] { resourceDescriptor.ResourceType };
-            var (implementation, registrationInterface) = TypeLocator.GetGenericInterfaceImplementation(assembly, interfaceType, genericArguments);
-
-            if (implementation != null)
+            var result = TypeLocator.GetGenericInterfaceImplementation(assembly, interfaceType, genericArguments);
+            if (result != null)
             {
+                var (implementation, registrationInterface) = result.Value;
                 _services.AddScoped(registrationInterface, implementation);
             }
         }

src/JsonApiDotNetCore/Configuration/TypeLocator.cs

@@ -38,41 +38,52 @@ public static ResourceDescriptor TryGetResourceDescriptor(Type type)
         }
 
         /// <summary>
-        /// Gets all implementations of the generic interface.
+        /// Gets all implementations of a generic interface.
         /// </summary>
-        /// <param name="assembly">The assembly to search.</param>
-        /// <param name="openGenericInterface">The open generic type, e.g. `typeof(IResourceService&lt;&gt;)`.</param>
-        /// <param name="genericInterfaceArguments">Parameters to the generic type.</param>
+        /// <param name="assembly">The assembly to search in.</param>
+        /// <param name="openGenericInterface">The open generic interface.</param>
+        /// <param name="interfaceGenericTypeArguments">Generic type parameters to construct the generic interface.</param>
         /// <example>
         /// <code><![CDATA[
-        /// GetGenericInterfaceImplementation(assembly, typeof(IResourceService<>), typeof(Article), typeof(Guid));
+        /// GetGenericInterfaceImplementation(assembly, typeof(IResourceService<,>), typeof(Article), typeof(Guid));
         /// ]]></code>
         /// </example>
-        public static (Type implementation, Type registrationInterface) GetGenericInterfaceImplementation(Assembly assembly, Type openGenericInterface, params Type[] genericInterfaceArguments)
+        public static (Type implementation, Type registrationInterface)? GetGenericInterfaceImplementation(Assembly assembly, Type openGenericInterface, params Type[] interfaceGenericTypeArguments)
         {
             if (assembly == null) throw new ArgumentNullException(nameof(assembly));
             if (openGenericInterface == null) throw new ArgumentNullException(nameof(openGenericInterface));
-            if (genericInterfaceArguments == null) throw new ArgumentNullException(nameof(genericInterfaceArguments));
-            if (genericInterfaceArguments.Length == 0) throw new ArgumentException("No arguments supplied for the generic interface.", nameof(genericInterfaceArguments));
-            if (!openGenericInterface.IsGenericType) throw new ArgumentException("Requested type is not a generic type.", nameof(openGenericInterface));
+            if (interfaceGenericTypeArguments == null) throw new ArgumentNullException(nameof(interfaceGenericTypeArguments));
 
-            foreach (var type in assembly.GetTypes())
+            if (!openGenericInterface.IsInterface || !openGenericInterface.IsGenericType ||
+                openGenericInterface != openGenericInterface.GetGenericTypeDefinition())
+            {
+                throw new ArgumentException($"Specified type '{openGenericInterface.FullName}' is not an open generic interface.", nameof(openGenericInterface));
+            }
+
+            if (interfaceGenericTypeArguments.Length != openGenericInterface.GetGenericArguments().Length)
             {
-                var interfaces = type.GetInterfaces();
-                foreach (var @interface in interfaces)
+                throw new ArgumentException(
+                    $"Interface '{openGenericInterface.FullName}' requires {openGenericInterface.GetGenericArguments().Length} type parameters instead of {interfaceGenericTypeArguments.Length}.",
+                    nameof(interfaceGenericTypeArguments));
+            }
+
+            foreach (var nextType in assembly.GetTypes())
+            {
+                foreach (var nextGenericInterface in nextType.GetInterfaces().Where(x => x.IsGenericType))
                 {
-                    if (@interface.IsGenericType)
+                    var nextOpenGenericInterface = nextGenericInterface.GetGenericTypeDefinition();
+                    if (nextOpenGenericInterface == openGenericInterface)
                     {
-                        var genericTypeDefinition = @interface.GetGenericTypeDefinition();
-                        if (@interface.GetGenericArguments().First() == genericInterfaceArguments.First() &&genericTypeDefinition == openGenericInterface.GetGenericTypeDefinition())
+                        var nextGenericArguments = nextGenericInterface.GetGenericArguments();
+                        if (nextGenericArguments.Length == interfaceGenericTypeArguments.Length && nextGenericArguments.SequenceEqual(interfaceGenericTypeArguments))
                         {
-                            return (type, genericTypeDefinition.MakeGenericType(genericInterfaceArguments));
+                            return (nextType, nextOpenGenericInterface.MakeGenericType(interfaceGenericTypeArguments));    
                         }
                     }
                 }
             }
 
-            return (null, null);
+            return null;
         }
 
         /// <summary>

test/UnitTests/Graph/TypeLocator_Tests.cs

@@ -19,15 +19,16 @@ public void GetGenericInterfaceImplementation_Gets_Implementation()
             var expectedInterface = typeof(IGenericInterface<int>);
 
             // Act
-            var (implementation, registrationInterface) = TypeLocator.GetGenericInterfaceImplementation(
+            var result = TypeLocator.GetGenericInterfaceImplementation(
                 assembly,
                 openGeneric,
                 genericArg
             );
 
             // Assert
-            Assert.Equal(expectedImplementation, implementation);
-            Assert.Equal(expectedInterface, registrationInterface);
+            Assert.NotNull(result);
+            Assert.Equal(expectedImplementation, result.Value.implementation);
+            Assert.Equal(expectedInterface, result.Value.registrationInterface);
         }
 
         [Fact]