Improve performance and reduce allocations in RouteValuesAddressScheme. (#879)

src/Microsoft.AspNetCore.Routing/RouteValuesAddressScheme.cs

@@ -16,7 +16,7 @@ namespace Microsoft.AspNetCore.Routing
     {
         private readonly CompositeEndpointDataSource _dataSource;
         private LinkGenerationDecisionTree _allMatchesLinkGenerationTree;
-        private IDictionary<string, List<OutboundMatchResult>> _namedMatchResults;
+        private Dictionary<string, List<OutboundMatchResult>> _namedMatchResults;
 
         public RouteValuesAddressScheme(CompositeEndpointDataSource dataSource)
         {
@@ -33,7 +33,7 @@ namespace Microsoft.AspNetCore.Routing
 
         public IEnumerable<Endpoint> FindEndpoints(RouteValuesAddress address)
         {
-            IEnumerable<OutboundMatchResult> matchResults = null;
+            IList<OutboundMatchResult> matchResults = null;
             if (string.IsNullOrEmpty(address.RouteName))
             {
                 matchResults = _allMatchesLinkGenerationTree.GetMatches(
@@ -45,14 +45,33 @@ namespace Microsoft.AspNetCore.Routing
                 matchResults = namedMatchResults;
             }
 
-            if (matchResults == null || !matchResults.Any())
+            if (matchResults != null)
             {
-                return Array.Empty<Endpoint>();
+                var matchCount = matchResults.Count;
+                if (matchCount > 0)
+                {
+                    if (matchResults.Count == 1)
+                    {
+                        // Special case having a single result to avoid creating iterator state machine
+                        return new[] { (RouteEndpoint)matchResults[0].Match.Entry.Data };
+                    }
+                    else
+                    {
+                        // Use separate method since one cannot have regular returns in an iterator method
+                        return GetEndpoints(matchResults, matchCount);
+                    }
+                }
             }
 
-            return matchResults
+            return Array.Empty<Endpoint>();
-                .Select(matchResult => matchResult.Match)
+        }
-                .Select(match => (RouteEndpoint)match.Entry.Data);
+
+        private static IEnumerable<Endpoint> GetEndpoints(IList<OutboundMatchResult> matchResults, int matchCount)
+        {
+            for (var i = 0; i < matchCount; i++)
+            {
+                yield return (RouteEndpoint)matchResults[i].Match.Entry.Data;
+            }
         }
 
         private void HandleChange()
@@ -73,7 +92,7 @@ namespace Microsoft.AspNetCore.Routing
             // as refresh of new endpoints happens within a lock and also these fields are not publicly accessible.
             var (allMatches, namedMatchResults) = GetOutboundMatches();
             _namedMatchResults = namedMatchResults;
-            _allMatchesLinkGenerationTree = new LinkGenerationDecisionTree(allMatches.ToArray());
+            _allMatchesLinkGenerationTree = new LinkGenerationDecisionTree(allMatches);
         }
 
         /// Decision tree is built using the 'required values' of actions.
@@ -93,21 +112,25 @@ namespace Microsoft.AspNetCore.Routing
         ///     requiredValues: new { controller = "Orders", action = "GetById" },
         ///   A call to GetLink("OrdersApi", new { id = "10" }) cannot generate url as neither the supplied values or
         ///   current ambient values do not satisfy the decision tree that is built based on the required values.
-        protected virtual (IEnumerable<OutboundMatch>, IDictionary<string, List<OutboundMatchResult>>) GetOutboundMatches()
+        protected virtual (List<OutboundMatch>, Dictionary<string, List<OutboundMatchResult>>) GetOutboundMatches()
         {
             var allOutboundMatches = new List<OutboundMatch>();
             var namedOutboundMatchResults = new Dictionary<string, List<OutboundMatchResult>>(
                 StringComparer.OrdinalIgnoreCase);
 
-            var endpoints = _dataSource.Endpoints.OfType<RouteEndpoint>();
+            foreach (var endpoint in _dataSource.Endpoints)
-            foreach (var endpoint in endpoints)
             {
+                if (!(endpoint is RouteEndpoint routeEndpoint))
+                {
+                    continue;
+                }
+
                 if (endpoint.Metadata.GetMetadata<ISuppressLinkGenerationMetadata>()?.SuppressLinkGeneration == true)
                 {
                     continue;
                 }
 
-                var entry = CreateOutboundRouteEntry(endpoint);
+                var entry = CreateOutboundRouteEntry(routeEndpoint);
 
                 var outboundMatch = new OutboundMatch() { Entry = entry };
                 allOutboundMatches.Add(outboundMatch);
@@ -117,8 +140,7 @@ namespace Microsoft.AspNetCore.Routing
                     continue;
                 }
 
-                List<OutboundMatchResult> matchResults;
+                if (!namedOutboundMatchResults.TryGetValue(entry.RouteName, out var matchResults))
-                if (!namedOutboundMatchResults.TryGetValue(entry.RouteName, out matchResults))
                 {
                     matchResults = new List<OutboundMatchResult>();
                     namedOutboundMatchResults.Add(entry.RouteName, matchResults);

test/Microsoft.AspNetCore.Routing.Tests/RouteValuesAddressSchemeTest.cs

@@ -139,6 +139,94 @@ namespace Microsoft.AspNetCore.Routing
                 });
         }
 
+        [Fact]
+        public void FindEndpoints_LookedUpByCriteria_NoMatch()
+        {
+            // Arrange
+            var endpoint1 = CreateEndpoint(
+                "api/orders/{id}/{name?}/{urgent=true}/{zipCode}",
+                defaults: new { zipCode = 3510 },
+                requiredValues: new { id = 7 },
+                routeName: "OrdersApi");
+            var endpoint2 = CreateEndpoint(
+                "api/orders/{id}/{name?}/{urgent=true}/{zipCode}",
+                defaults: new { id = 12 },
+                requiredValues: new { zipCode = 3510 },
+                routeName: "OrdersApi");
+            var addressScheme = CreateAddressScheme(endpoint1, endpoint2);
+
+            // Act
+            var foundEndpoints = addressScheme.FindEndpoints(
+                new RouteValuesAddress
+                {
+                    ExplicitValues = new RouteValueDictionary(new { id = 8 }),
+                    AmbientValues = new RouteValueDictionary(new { urgent = false }),
+                });
+
+            // Assert
+            Assert.Empty(foundEndpoints);
+        }
+
+        [Fact]
+        public void FindEndpoints_LookedUpByCriteria_OneMatch()
+        {
+            // Arrange
+            var endpoint1 = CreateEndpoint(
+                "api/orders/{id}/{name?}/{urgent=true}/{zipCode}",
+                defaults: new { zipCode = 3510 },
+                requiredValues: new { id = 7 },
+                routeName: "OrdersApi");
+            var endpoint2 = CreateEndpoint(
+                "api/orders/{id}/{name?}/{urgent=true}/{zipCode}",
+                defaults: new { id = 12 },
+                routeName: "OrdersApi");
+            var addressScheme = CreateAddressScheme(endpoint1, endpoint2);
+
+            // Act
+            var foundEndpoints = addressScheme.FindEndpoints(
+                new RouteValuesAddress
+                {
+                    ExplicitValues = new RouteValueDictionary(new { id = 13 }),
+                    AmbientValues = new RouteValueDictionary(new { zipCode = 3500 }),
+                });
+
+            // Assert
+            var actual = Assert.Single(foundEndpoints);
+            Assert.Same(endpoint2, actual);
+        }
+
+        [Fact]
+        public void FindEndpoints_LookedUpByCriteria_MultipleMatches()
+        {
+            // Arrange
+            var endpoint1 = CreateEndpoint(
+                "api/orders/{id}/{name?}/{urgent=true}/{zipCode}",
+                defaults: new { zipCode = 3510 },
+                requiredValues: new { id = 7 },
+                routeName: "OrdersApi");
+            var endpoint2 = CreateEndpoint(
+                "api/orders/{id}/{name?}/{urgent}/{zipCode}",
+                defaults: new { id = 12 },
+                routeName: "OrdersApi");
+            var endpoint3 = CreateEndpoint(
+                "api/orders/{id}/{name?}/{urgent=true}/{zipCode}",
+                defaults: new { id = 12 },
+                routeName: "OrdersApi");
+            var addressScheme = CreateAddressScheme(endpoint1, endpoint2, endpoint3);
+
+            // Act
+            var foundEndpoints = addressScheme.FindEndpoints(
+                new RouteValuesAddress
+                {
+                    ExplicitValues = new RouteValueDictionary(new { id = 7 }),
+                    AmbientValues = new RouteValueDictionary(new { zipCode = 3500 }),
+                });
+
+            // Assert
+            Assert.Contains(endpoint1, foundEndpoints);
+            Assert.Contains(endpoint1, foundEndpoints);
+        }
+
         [Fact]
         public void FindEndpoints_ReturnsEndpoint_WhenLookedUpByRouteName()
         {
@@ -270,7 +358,7 @@ namespace Microsoft.AspNetCore.Routing
 
             public IDictionary<string, List<OutboundMatchResult>> NamedMatches { get; private set; }
 
-            protected override (IEnumerable<OutboundMatch>, IDictionary<string, List<OutboundMatchResult>>) GetOutboundMatches()
+            protected override (List<OutboundMatch>, Dictionary<string, List<OutboundMatchResult>>) GetOutboundMatches()
             {
                 var matches = base.GetOutboundMatches();
                 AllMatches = matches.Item1;