Remove some Linq usages from Routing (#47004)

eerhardt · web-flow · commit bb4153237f49 · 2023-03-08T09:05:05.000-06:00
* Remove some Linq usages from Routing

These Linq usages account for roughly 150KB of the 770KB
of NativeAOT app size attributed to Routing code. The largest
usages are the ones where we are using ValueTypes (like ValueTuple and
KeyValuePair) over internal types. The size savings helps even if the
app is using Linq because the generic instantiation will never be
brought back into the app.
diff --git a/src/Http/Routing/src/EndpointNameAddressScheme.cs b/src/Http/Routing/src/EndpointNameAddressScheme.cs
@@ -1,7 +1,6 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System.Linq;
 using System.Text;
 using Microsoft.AspNetCore.Http;
 
@@ -52,11 +51,17 @@ private static Dictionary<string, Endpoint[]> Initialize(IReadOnlyList<Endpoint>
                 entries[endpointName] = new[] { endpoint };
                 continue;
             }
-
-            // Ok this is a duplicate, because we have two endpoints with the same name. Bail out, because we
-            // are just going to throw, we don't need to finish collecting data.
-            hasDuplicates = true;
-            break;
+            else
+            {
+                // Ok this is a duplicate, because we have two endpoints with the same name. Collect all the data
+                // so we can throw an exception. The extra allocations here don't matter since this is an exceptional case.
+                hasDuplicates = true;
+
+                var newEntry = new Endpoint[existing.Length + 1];
+                Array.Copy(existing, newEntry, existing.Length);
+                newEntry[existing.Length] = endpoint;
+                entries[endpointName] = newEntry;
+            }
         }
 
         if (!hasDuplicates)
@@ -66,21 +71,20 @@ private static Dictionary<string, Endpoint[]> Initialize(IReadOnlyList<Endpoint>
         }
 
         // OK we need to report some duplicates.
-        var duplicates = endpoints
-            .GroupBy(GetEndpointName)
-            .Where(g => g.Key != null && g.Count() > 1);
-
         var builder = new StringBuilder();
         builder.AppendLine(Resources.DuplicateEndpointNameHeader);
 
-        foreach (var group in duplicates)
+        foreach (var group in entries)
         {
-            builder.AppendLine();
-            builder.AppendLine(Resources.FormatDuplicateEndpointNameEntry(group.Key));
-
-            foreach (var endpoint in group)
+            if (group.Key is not null && group.Value.Length > 1)
             {
-                builder.AppendLine(endpoint.DisplayName);
+                builder.AppendLine();
+                builder.AppendLine(Resources.FormatDuplicateEndpointNameEntry(group.Key));
+
+                foreach (var endpoint in group.Value)
+                {
+                    builder.AppendLine(endpoint.DisplayName);
+                }
             }
         }
 
diff --git a/src/Http/Routing/src/Matching/AcceptsMatcherPolicy.cs b/src/Http/Routing/src/Matching/AcceptsMatcherPolicy.cs
@@ -236,9 +236,14 @@ public IReadOnlyList<PolicyNodeEdge> GetEdges(IReadOnlyList<Endpoint> endpoints)
             edges.Add(string.Empty, anyEndpoints.ToList());
         }
 
-        return edges
-            .Select(kvp => new PolicyNodeEdge(kvp.Key, kvp.Value))
-            .ToArray();
+        var result = new PolicyNodeEdge[edges.Count];
+        var index = 0;
+        foreach (var kvp in edges)
+        {
+            result[index] = new PolicyNodeEdge(kvp.Key, kvp.Value);
+            index++;
+        }
+        return result;
     }
 
     private static Endpoint CreateRejectionEndpoint()
@@ -258,10 +263,13 @@ public PolicyJumpTable BuildJumpTable(int exitDestination, IReadOnlyList<PolicyJ
 
         // Since our 'edges' can have wildcards, we do a sort based on how wildcard-ey they
         // are then then execute them in linear order.
-        var ordered = edges
-            .Select(e => (mediaType: CreateEdgeMediaType(ref e), destination: e.Destination))
-            .OrderBy(e => GetScore(e.mediaType))
-            .ToArray();
+        var ordered = new (ReadOnlyMediaTypeHeaderValue mediaType, int destination)[edges.Count];
+        for (var i = 0; i < edges.Count; i++)
+        {
+            var e = edges[i];
+            ordered[i] = (mediaType: CreateEdgeMediaType(ref e), destination: e.Destination);
+        }
+        Array.Sort(ordered, static (left, right) => GetScore(left.mediaType).CompareTo(GetScore(right.mediaType)));
 
         // If any edge matches all content types, then treat that as the 'exit'. This will
         // always happen because we insert a 415 endpoint.
diff --git a/src/Http/Routing/src/Matching/HostMatcherPolicy.cs b/src/Http/Routing/src/Matching/HostMatcherPolicy.cs
@@ -199,8 +199,8 @@ public IReadOnlyList<PolicyNodeEdge> GetEdges(IReadOnlyList<Endpoint> endpoints)
         for (var i = 0; i < endpoints.Count; i++)
         {
             var endpoint = endpoints[i];
-            var hosts = endpoint.Metadata.GetMetadata<IHostMetadata>()?.Hosts.Select(CreateEdgeKey).ToArray();
-            if (hosts == null || hosts.Length == 0)
+            var hosts = GetEdgeKeys(endpoint);
+            if (hosts is null || hosts.Length == 0)
             {
                 hosts = new[] { EdgeKey.WildcardEdgeKey };
             }
@@ -221,8 +221,8 @@ public IReadOnlyList<PolicyNodeEdge> GetEdges(IReadOnlyList<Endpoint> endpoints)
         {
             var endpoint = endpoints[i];
 
-            var endpointKeys = endpoint.Metadata.GetMetadata<IHostMetadata>()?.Hosts.Select(CreateEdgeKey).ToArray() ?? Array.Empty<EdgeKey>();
-            if (endpointKeys.Length == 0)
+            var endpointKeys = GetEdgeKeys(endpoint);
+            if (endpointKeys is null || endpointKeys.Length == 0)
             {
                 // OK this means that this endpoint matches *all* hosts.
                 // So, loop and add it to all states.
@@ -259,9 +259,28 @@ public IReadOnlyList<PolicyNodeEdge> GetEdges(IReadOnlyList<Endpoint> endpoints)
             }
         }
 
-        return edges
-            .Select(kvp => new PolicyNodeEdge(kvp.Key, kvp.Value))
-            .ToArray();
+        var result = new PolicyNodeEdge[edges.Count];
+        var index = 0;
+        foreach (var kvp in edges)
+        {
+            result[index] = new PolicyNodeEdge(kvp.Key, kvp.Value);
+            index++;
+        }
+        return result;
+    }
+
+    private static EdgeKey[]? GetEdgeKeys(Endpoint endpoint)
+    {
+        List<EdgeKey>? result = null;
+        var hostMetadata = endpoint.Metadata.GetMetadata<IHostMetadata>();
+        if (hostMetadata is not null)
+        {
+            foreach (var host in hostMetadata.Hosts)
+            {
+                (result ??= new()).Add(CreateEdgeKey(host));
+            }
+        }
+        return result?.ToArray();
     }
 
     /// <inheritdoc />
@@ -271,10 +290,13 @@ public PolicyJumpTable BuildJumpTable(int exitDestination, IReadOnlyList<PolicyJ
 
         // Since our 'edges' can have wildcards, we do a sort based on how wildcard-ey they
         // are then then execute them in linear order.
-        var ordered = edges
-            .Select(e => (host: (EdgeKey)e.State, destination: e.Destination))
-            .OrderBy(e => GetScore(e.host))
-            .ToArray();
+        var ordered = new (EdgeKey host, int destination)[edges.Count];
+        for (var i = 0; i < edges.Count; i++)
+        {
+            PolicyJumpTableEdge e = edges[i];
+            ordered[i] = (host: (EdgeKey)e.State, destination: e.Destination);
+        }
+        Array.Sort(ordered, static (left, right) => GetScore(left.host).CompareTo(GetScore(right.host)));
 
         return new HostPolicyJumpTable(exitDestination, ordered);
     }
diff --git a/src/Http/Routing/src/ParameterPolicyActivator.cs b/src/Http/Routing/src/ParameterPolicyActivator.cs
@@ -1,9 +1,9 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
-using System.Linq;
 using System.Reflection;
 using Microsoft.Extensions.DependencyInjection;
 
@@ -93,8 +93,8 @@ private static bool ResolveParameterPolicyTypeAndArgument(
     [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070", Justification = "We ensure the constructor is preserved when the constraint map is added.")]
     private static IParameterPolicy CreateParameterPolicy(IServiceProvider? serviceProvider, Type parameterPolicyType, string? argumentString)
     {
-        ConstructorInfo? activationConstructor = null;
-        object?[]? parameters = null;
+        ConstructorInfo? activationConstructor;
+        object?[]? parameters;
         var constructors = parameterPolicyType.GetConstructors();
 
         // If there is only one constructor and it has a single parameter, pass the argument string directly
@@ -113,30 +113,24 @@ private static IParameterPolicy CreateParameterPolicy(IServiceProvider? serviceP
             // arguments that can be resolved from DI
             //
             // For example, ctor(string, IService) will beat ctor(string)
-            var matchingConstructors = constructors
-                .Where(ci => GetNonConvertableParameterTypeCount(serviceProvider, ci.GetParameters()) == arguments.Length)
-                .OrderByDescending(ci => ci.GetParameters().Length)
-                .ToArray();
+            var matchingConstructors = GetMatchingConstructors(constructors, serviceProvider, arguments.Length);
 
-            if (matchingConstructors.Length == 0)
+            if (matchingConstructors.Count == 0)
             {
                 throw new RouteCreationException(
                             Resources.FormatDefaultInlineConstraintResolver_CouldNotFindCtor(
                                                    parameterPolicyType.Name, arguments.Length));
             }
             else
             {
-                // When there are multiple matching constructors, choose the one with the most service arguments
-                if (matchingConstructors.Length == 1
-                    || matchingConstructors[0].GetParameters().Length > matchingConstructors[1].GetParameters().Length)
+                if (matchingConstructors.Count == 1)
                 {
                     activationConstructor = matchingConstructors[0];
                 }
                 else
                 {
-                    throw new RouteCreationException(
-                                Resources.FormatDefaultInlineConstraintResolver_AmbiguousCtors(
-                                                       parameterPolicyType.Name, matchingConstructors[0].GetParameters().Length));
+                    // When there are multiple matching constructors, choose the one with the most service arguments
+                    activationConstructor = GetLongestConstructor(matchingConstructors, parameterPolicyType);
                 }
 
                 parameters = ConvertArguments(serviceProvider, activationConstructor.GetParameters(), arguments);
@@ -146,6 +140,52 @@ private static IParameterPolicy CreateParameterPolicy(IServiceProvider? serviceP
         return (IParameterPolicy)activationConstructor.Invoke(parameters);
     }
 
+    private static List<ConstructorInfo> GetMatchingConstructors(ConstructorInfo[] constructors, IServiceProvider? serviceProvider, int argumentsLength)
+    {
+        var result = new List<ConstructorInfo>();
+        foreach (var constructor in constructors)
+        {
+            if (GetNonConvertableParameterTypeCount(serviceProvider, constructor.GetParameters()) == argumentsLength)
+            {
+                result.Add(constructor);
+            }
+        }
+        return result;
+    }
+
+    private static ConstructorInfo GetLongestConstructor(List<ConstructorInfo> constructors, Type parameterPolicyType)
+    {
+        Debug.Assert(constructors.Count > 0);
+
+        var longestLength = -1;
+        ConstructorInfo? longest = null;
+        var multipleBestLengthFound = false;
+
+        foreach (var constructor in constructors)
+        {
+            var length = constructor.GetParameters().Length;
+            if (length > longestLength)
+            {
+                multipleBestLengthFound = false;
+                longestLength = length;
+                longest = constructor;
+            }
+            else if (longestLength == length)
+            {
+                multipleBestLengthFound = true;
+            }
+        }
+
+        if (multipleBestLengthFound)
+        {
+            throw new RouteCreationException(
+                        Resources.FormatDefaultInlineConstraintResolver_AmbiguousCtors(
+                                               parameterPolicyType.Name, longestLength));
+        }
+
+        return longest!;
+    }
+
     private static int GetNonConvertableParameterTypeCount(IServiceProvider? serviceProvider, ParameterInfo[] parameters)
     {
         if (serviceProvider == null)
diff --git a/src/Http/Routing/src/Template/RouteTemplate.cs b/src/Http/Routing/src/Template/RouteTemplate.cs
@@ -28,7 +28,13 @@ public RouteTemplate(RoutePattern other)
         // RequiredValues will be ignored. RouteTemplate doesn't support them.
 
         TemplateText = other.RawText;
-        Segments = new List<TemplateSegment>(other.PathSegments.Select(p => new TemplateSegment(p)));
+
+        Segments = new List<TemplateSegment>(other.PathSegments.Count);
+        foreach (var p in other.PathSegments)
+        {
+            Segments.Add(new TemplateSegment(p));
+        }
+
         Parameters = new List<TemplatePart>();
         for (var i = 0; i < Segments.Count; i++)
         {
diff --git a/src/Http/Routing/src/Template/TemplateBinder.cs b/src/Http/Routing/src/Template/TemplateBinder.cs
@@ -87,14 +87,7 @@ internal TemplateBinder(
         }
         _filters = filters.ToArray();
 
-        _constraints = parameterPolicies
-            ?.Where(p => p.policy is IRouteConstraint)
-            .Select(p => (p.parameterName, (IRouteConstraint)p.policy))
-            .ToArray() ?? Array.Empty<(string, IRouteConstraint)>();
-        _parameterTransformers = parameterPolicies
-            ?.Where(p => p.policy is IOutboundParameterTransformer)
-            .Select(p => (p.parameterName, (IOutboundParameterTransformer)p.policy))
-            .ToArray() ?? Array.Empty<(string, IOutboundParameterTransformer)>();
+        Initialize(parameterPolicies, out _constraints, out _parameterTransformers);
 
         _slots = AssignSlots(_pattern, _filters);
     }
@@ -126,18 +119,38 @@ internal TemplateBinder(
         }
         _filters = filters.ToArray();
 
-        _constraints = parameterPolicies
-            ?.Where(p => p.policy is IRouteConstraint)
-            .Select(p => (p.parameterName, (IRouteConstraint)p.policy))
-            .ToArray() ?? Array.Empty<(string, IRouteConstraint)>();
-        _parameterTransformers = parameterPolicies
-            ?.Where(p => p.policy is IOutboundParameterTransformer)
-            .Select(p => (p.parameterName, (IOutboundParameterTransformer)p.policy))
-            .ToArray() ?? Array.Empty<(string, IOutboundParameterTransformer)>();
+        Initialize(parameterPolicies, out _constraints, out _parameterTransformers);
 
         _slots = AssignSlots(_pattern, _filters);
     }
 
+    private static void Initialize(
+        IEnumerable<(string parameterName, IParameterPolicy policy)>? parameterPolicies,
+        out (string parameterName, IRouteConstraint constraint)[] constraints,
+        out (string parameterName, IOutboundParameterTransformer transformer)[] parameterTransformers)
+    {
+        List<(string parameterName, IRouteConstraint constraint)>? constraintList = null;
+        List<(string parameterName, IOutboundParameterTransformer transformer)>? parameterTransformerList = null;
+
+        if (parameterPolicies is not null)
+        {
+            foreach (var p in parameterPolicies)
+            {
+                if (p.policy is IRouteConstraint routeConstraint)
+                {
+                    (constraintList ??= new()).Add((p.parameterName, routeConstraint));
+                }
+                if (p.policy is IOutboundParameterTransformer transformer)
+                {
+                    (parameterTransformerList ??= new()).Add((p.parameterName, transformer));
+                }
+            }
+        }
+
+        constraints = constraintList?.ToArray() ?? Array.Empty<(string, IRouteConstraint)>();
+        parameterTransformers = parameterTransformerList?.ToArray() ?? Array.Empty<(string, IOutboundParameterTransformer)>();
+    }
+
     /// <summary>
     /// Generates the parameter values in the route.
     /// </summary>
diff --git a/src/Http/Routing/test/UnitTests/DefaultParameterPolicyFactoryTest.cs b/src/Http/Routing/test/UnitTests/DefaultParameterPolicyFactoryTest.cs
