From 35163566a96642cc67ac330f578ab56814239994 Mon Sep 17 00:00:00 2001 From: "ASP.NET CI" Date: Wed, 5 Sep 2018 16:36:05 -0700 Subject: [PATCH 1/9] Update branding to 2.2.0-preview3 --- version.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.props b/version.props index 15637ba785..704cac087b 100644 --- a/version.props +++ b/version.props @@ -1,7 +1,7 @@ 2.2.0 - preview2 + preview3 $(VersionPrefix) $(VersionPrefix)-$(VersionSuffix)-final t000 From e5c520b4ca65978e7ae88194fbc354f62edfb908 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 6 Sep 2018 12:10:20 +1200 Subject: [PATCH 2/9] Add DfaMatcherBuilder benchmarks (#777) --- .../Matching/MatcherAzureBenchmark.cs | 3 +- ...=> MatcherAzureBenchmarkBase.generated.cs} | 10 ++-- .../Matching/MatcherBuilderAzureBenchmark.cs | 31 ++++++++++++ .../Matching/MatcherBuilderGithubBenchmark.cs | 31 ++++++++++++ .../MatcherBuilderMultipleEntryBenchmark.cs | 50 +++++++++++++++++++ .../Matching/MatcherGithubBenchmark.cs | 2 +- ...> MatcherGithubBenchmarkBase.generated.cs} | 10 ++-- tools/Swaggatherer/Template.cs | 8 +-- 8 files changed, 128 insertions(+), 17 deletions(-) rename benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/{MatcherAzureBenchmark.generated.cs => MatcherAzureBenchmarkBase.generated.cs} (99%) create mode 100644 benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherBuilderAzureBenchmark.cs create mode 100644 benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherBuilderGithubBenchmark.cs create mode 100644 benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherBuilderMultipleEntryBenchmark.cs rename benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/{MatcherGithubBenchmark.generated.cs => MatcherGithubBenchmarkBase.generated.cs} (99%) diff --git a/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherAzureBenchmark.cs b/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherAzureBenchmark.cs index b68255e3cd..a934316c1f 100644 --- a/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherAzureBenchmark.cs +++ b/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherAzureBenchmark.cs @@ -3,12 +3,11 @@ using System.Threading.Tasks; using BenchmarkDotNet.Attributes; -using Microsoft.AspNetCore.Http.Features; namespace Microsoft.AspNetCore.Routing.Matching { // Generated from https://github.com/Azure/azure-rest-api-specs - public partial class MatcherAzureBenchmark : EndpointRoutingBenchmarkBase + public class MatcherAzureBenchmark : MatcherAzureBenchmarkBase { private const int SampleCount = 100; diff --git a/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherAzureBenchmark.generated.cs b/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherAzureBenchmarkBase.generated.cs similarity index 99% rename from benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherAzureBenchmark.generated.cs rename to benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherAzureBenchmarkBase.generated.cs index ebb347ea51..1dc30336e3 100644 --- a/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherAzureBenchmark.generated.cs +++ b/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherAzureBenchmarkBase.generated.cs @@ -7,11 +7,11 @@ using Microsoft.AspNetCore.Http; namespace Microsoft.AspNetCore.Routing.Matching { // This code was generated by the Swaggatherer - public partial class MatcherAzureBenchmark : EndpointRoutingBenchmarkBase + public abstract partial class MatcherAzureBenchmarkBase : EndpointRoutingBenchmarkBase { - private const int EndpointCount = 5160; + private protected const int EndpointCount = 5160; - private void SetupEndpoints() + private protected void SetupEndpoints() { Endpoints = new RouteEndpoint[5160]; Endpoints[0] = CreateEndpoint("/account", "GET"); @@ -5176,7 +5176,7 @@ namespace Microsoft.AspNetCore.Routing.Matching Endpoints[5159] = CreateEndpoint("/{tenantID}/groups/{groupObjectId}/$links/members/{memberObjectId}", "DELETE"); } - private void SetupRequests() + private protected void SetupRequests() { Requests = new HttpContext[5160]; Requests[0] = new DefaultHttpContext(); @@ -25821,7 +25821,7 @@ namespace Microsoft.AspNetCore.Routing.Matching Requests[5159].Request.Path = "/67fb987d/groups/04d01a8c-6135/$links/members/71b75dbe-0076-"; } - private Matcher SetupMatcher(MatcherBuilder builder) + private protected Matcher SetupMatcher(MatcherBuilder builder) { builder.AddEndpoint(Endpoints[0]); builder.AddEndpoint(Endpoints[1]); diff --git a/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherBuilderAzureBenchmark.cs b/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherBuilderAzureBenchmark.cs new file mode 100644 index 0000000000..a839075c82 --- /dev/null +++ b/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherBuilderAzureBenchmark.cs @@ -0,0 +1,31 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using BenchmarkDotNet.Attributes; +using Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.AspNetCore.Routing.Matching +{ + // Generated from https://github.com/APIs-guru/openapi-directory + // Use https://editor2.swagger.io/ to convert from yaml to json- + public class MatcherBuilderAzureBenchmark : MatcherAzureBenchmarkBase + { + private IServiceProvider _services; + + [GlobalSetup] + public void Setup() + { + SetupEndpoints(); + + _services = CreateServices(); + } + + [Benchmark] + public void Dfa() + { + var builder = _services.GetRequiredService(); + SetupMatcher(builder); + } + } +} \ No newline at end of file diff --git a/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherBuilderGithubBenchmark.cs b/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherBuilderGithubBenchmark.cs new file mode 100644 index 0000000000..4e0244e46e --- /dev/null +++ b/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherBuilderGithubBenchmark.cs @@ -0,0 +1,31 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using BenchmarkDotNet.Attributes; +using Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.AspNetCore.Routing.Matching +{ + // Generated from https://github.com/APIs-guru/openapi-directory + // Use https://editor2.swagger.io/ to convert from yaml to json- + public class MatcherBuilderGithubBenchmark : MatcherGithubBenchmarkBase + { + private IServiceProvider _services; + + [GlobalSetup] + public void Setup() + { + SetupEndpoints(); + + _services = CreateServices(); + } + + [Benchmark] + public void Dfa() + { + var builder = _services.GetRequiredService(); + SetupMatcher(builder); + } + } +} \ No newline at end of file diff --git a/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherBuilderMultipleEntryBenchmark.cs b/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherBuilderMultipleEntryBenchmark.cs new file mode 100644 index 0000000000..002118caca --- /dev/null +++ b/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherBuilderMultipleEntryBenchmark.cs @@ -0,0 +1,50 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using BenchmarkDotNet.Attributes; +using Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.AspNetCore.Routing.Matching +{ + public partial class MatcherBuilderMultipleEntryBenchmark : EndpointRoutingBenchmarkBase + { + private IServiceProvider _services; + + [GlobalSetup] + public void Setup() + { + Endpoints = new RouteEndpoint[10]; + Endpoints[0] = CreateEndpoint("/product", "GET"); + Endpoints[1] = CreateEndpoint("/product/{id}", "GET"); + + Endpoints[2] = CreateEndpoint("/account", "GET"); + Endpoints[3] = CreateEndpoint("/account/{id}"); + Endpoints[4] = CreateEndpoint("/account/{id}", "POST"); + Endpoints[5] = CreateEndpoint("/account/{id}", "UPDATE"); + + Endpoints[6] = CreateEndpoint("/v2/account", "GET"); + Endpoints[7] = CreateEndpoint("/v2/account/{id}"); + Endpoints[8] = CreateEndpoint("/v2/account/{id}", "POST"); + Endpoints[9] = CreateEndpoint("/v2/account/{id}", "UPDATE"); + + _services = CreateServices(); + } + + private Matcher SetupMatcher(MatcherBuilder builder) + { + for (int i = 0; i < Endpoints.Length; i++) + { + builder.AddEndpoint(Endpoints[i]); + } + return builder.Build(); + } + + [Benchmark] + public void Dfa() + { + var builder = _services.GetRequiredService(); + SetupMatcher(builder); + } + } +} \ No newline at end of file diff --git a/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherGithubBenchmark.cs b/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherGithubBenchmark.cs index 0859fd7a72..3657b42c8b 100644 --- a/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherGithubBenchmark.cs +++ b/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherGithubBenchmark.cs @@ -9,7 +9,7 @@ namespace Microsoft.AspNetCore.Routing.Matching { // Generated from https://github.com/APIs-guru/openapi-directory // Use https://editor2.swagger.io/ to convert from yaml to json- - public partial class MatcherGithubBenchmark : EndpointRoutingBenchmarkBase + public class MatcherGithubBenchmark : MatcherGithubBenchmarkBase { private BarebonesMatcher _baseline; private Matcher _dfa; diff --git a/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherGithubBenchmark.generated.cs b/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherGithubBenchmarkBase.generated.cs similarity index 99% rename from benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherGithubBenchmark.generated.cs rename to benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherGithubBenchmarkBase.generated.cs index 01b2edd58f..3ac68c89e0 100644 --- a/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherGithubBenchmark.generated.cs +++ b/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherGithubBenchmarkBase.generated.cs @@ -7,11 +7,11 @@ using Microsoft.AspNetCore.Http; namespace Microsoft.AspNetCore.Routing.Matching { // This code was generated by the Swaggatherer - public partial class MatcherGithubBenchmark : EndpointRoutingBenchmarkBase + public partial class MatcherGithubBenchmarkBase : EndpointRoutingBenchmarkBase { - private const int EndpointCount = 243; + private protected const int EndpointCount = 243; - private void SetupEndpoints() + private protected void SetupEndpoints() { Endpoints = new RouteEndpoint[243]; Endpoints[0] = CreateEndpoint("/emojis", "GET"); @@ -259,7 +259,7 @@ namespace Microsoft.AspNetCore.Routing.Matching Endpoints[242] = CreateEndpoint("/repos/{owner}/{repo}/{archive_format}/{path}", "GET"); } - private void SetupRequests() + private protected void SetupRequests() { Requests = new HttpContext[243]; Requests[0] = new DefaultHttpContext(); @@ -1236,7 +1236,7 @@ namespace Microsoft.AspNetCore.Routing.Matching Requests[242].Request.Path = "/repos/21a74/f36c8/805b0492-b723-/680ad"; } - private Matcher SetupMatcher(MatcherBuilder builder) + private protected Matcher SetupMatcher(MatcherBuilder builder) { builder.AddEndpoint(Endpoints[0]); builder.AddEndpoint(Endpoints[1]); diff --git a/tools/Swaggatherer/Template.cs b/tools/Swaggatherer/Template.cs index c5c0c7b42e..8774c6f578 100644 --- a/tools/Swaggatherer/Template.cs +++ b/tools/Swaggatherer/Template.cs @@ -74,21 +74,21 @@ namespace Microsoft.AspNetCore.Routing // This code was generated by the Swaggatherer public partial class GeneratedBenchmark : EndpointRoutingBenchmarkBase {{ - private const int EndpointCount = {3}; + private protected const int EndpointCount = {3}; - private void SetupEndpoints() + private protected void SetupEndpoints() {{ Endpoints = new RouteEndpoint[{3}]; {0} }} - private void SetupRequests() + private protected void SetupRequests() {{ Requests = new HttpContext[{3}]; {1} }} - private Matcher SetupMatcher(MatcherBuilder builder) + private protected Matcher SetupMatcher(MatcherBuilder builder) {{ {2} return builder.Build(); From a777a4cdd54a1225fce1f3c57ff424740fef260d Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 6 Sep 2018 12:25:45 +1200 Subject: [PATCH 3/9] Reuse collections in DfaMatcherBuilder (#778) --- .../Matching/DfaMatcherBuilder.cs | 90 ++++++++++++++----- .../Matching/HttpMethodMatcherPolicy.cs | 2 +- 2 files changed, 68 insertions(+), 24 deletions(-) diff --git a/src/Microsoft.AspNetCore.Routing/Matching/DfaMatcherBuilder.cs b/src/Microsoft.AspNetCore.Routing/Matching/DfaMatcherBuilder.cs index 040d39a5e6..abe57ab1e7 100644 --- a/src/Microsoft.AspNetCore.Routing/Matching/DfaMatcherBuilder.cs +++ b/src/Microsoft.AspNetCore.Routing/Matching/DfaMatcherBuilder.cs @@ -19,6 +19,13 @@ namespace Microsoft.AspNetCore.Routing.Matching private readonly INodeBuilderPolicy[] _nodeBuilders; private readonly EndpointComparer _comparer; + // These collections are reused when building candidates + private readonly Dictionary _assignments; + private readonly List> _slots; + private readonly List<(string parameterName, int segmentIndex, int slotIndex)> _captures; + private readonly List<(RoutePatternPathSegment pathSegment, int segmentIndex)> _complexSegments; + private readonly List> _constraints; + public DfaMatcherBuilder( ParameterPolicyFactory parameterPolicyFactory, EndpointSelector selector, @@ -31,6 +38,12 @@ namespace Microsoft.AspNetCore.Routing.Matching // Taking care to use _policies, which has been sorted. _nodeBuilders = _policies.OfType().ToArray(); _comparer = new EndpointComparer(_policies.OfType().ToArray()); + + _assignments = new Dictionary(StringComparer.OrdinalIgnoreCase); + _slots = new List>(); + _captures = new List<(string parameterName, int segmentIndex, int slotIndex)>(); + _complexSegments = new List<(RoutePatternPathSegment pathSegment, int segmentIndex)>(); + _constraints = new List>(); } public override void AddEndpoint(RouteEndpoint endpoint) @@ -50,6 +63,7 @@ namespace Microsoft.AspNetCore.Routing.Matching // this list will hold the set of items we need to process at the current // stage. var work = new List<(RouteEndpoint endpoint, List parents)>(); + List<(RouteEndpoint endpoint, List parents)> previousWork = null; var root = new DfaNode() { PathDepth = 0, Label = "/" }; @@ -63,14 +77,26 @@ namespace Microsoft.AspNetCore.Routing.Matching work.Add((endpoint, new List() { root, })); } + var workCount = work.Count; // Now we process the entries a level at a time. for (var depth = 0; depth <= maxDepth; depth++) { // As we process items, collect the next set of items. - var nextWork = new List<(RouteEndpoint endpoint, List parents)>(); + List<(RouteEndpoint endpoint, List parents)> nextWork; + var nextWorkCount = 0; + if (previousWork == null) + { + nextWork = new List<(RouteEndpoint endpoint, List parents)>(); + } + else + { + // Reuse previous collection for the next collection + // Don't clear the list so nested lists can be reused + nextWork = previousWork; + } - for (var i = 0; i < work.Count; i++) + for (var i = 0; i < workCount; i++) { var (endpoint, parents) = work[i]; @@ -84,7 +110,23 @@ namespace Microsoft.AspNetCore.Routing.Matching } // Find the parents of this edge at the current depth - var nextParents = new List(); + List nextParents; + if (nextWorkCount < nextWork.Count) + { + nextParents = nextWork[nextWorkCount].parents; + nextParents.Clear(); + + nextWork[nextWorkCount] = (endpoint, nextParents); + } + else + { + nextParents = new List(); + + // Add to the next set of work now so the list will be reused + // even if there are no parents + nextWork.Add((endpoint, nextParents)); + } + var segment = GetCurrentSegment(endpoint, depth); if (segment == null) { @@ -178,12 +220,14 @@ namespace Microsoft.AspNetCore.Routing.Matching if (nextParents.Count > 0) { - nextWork.Add((endpoint, nextParents)); + nextWorkCount++; } } // Prepare the process the next stage. + previousWork = work; work = nextWork; + workCount = nextWorkCount; } // Build the trees of policy nodes (like HTTP methods). Post-order traversal @@ -370,20 +414,20 @@ namespace Microsoft.AspNetCore.Routing.Matching // internal for tests internal Candidate CreateCandidate(Endpoint endpoint, int score) { - var assignments = new Dictionary(StringComparer.OrdinalIgnoreCase); - var slots = new List>(); - var captures = new List<(string parameterName, int segmentIndex, int slotIndex)>(); - (string parameterName, int segmentIndex, int slotIndex) catchAll = default; + _assignments.Clear(); + _slots.Clear(); + _captures.Clear(); + _complexSegments.Clear(); + _constraints.Clear(); - var complexSegments = new List<(RoutePatternPathSegment pathSegment, int segmentIndex)>(); - var constraints = new List>(); + (string parameterName, int segmentIndex, int slotIndex) catchAll = default; if (endpoint is RouteEndpoint routeEndpoint) { foreach (var kvp in routeEndpoint.RoutePattern.Defaults) { - assignments.Add(kvp.Key, assignments.Count); - slots.Add(kvp); + _assignments.Add(kvp.Key, _assignments.Count); + _slots.Add(kvp); } for (var i = 0; i < routeEndpoint.RoutePattern.PathSegments.Count; i++) @@ -400,13 +444,13 @@ namespace Microsoft.AspNetCore.Routing.Matching continue; } - if (!assignments.TryGetValue(parameterPart.Name, out var slotIndex)) + if (!_assignments.TryGetValue(parameterPart.Name, out var slotIndex)) { - slotIndex = assignments.Count; - assignments.Add(parameterPart.Name, slotIndex); + slotIndex = _assignments.Count; + _assignments.Add(parameterPart.Name, slotIndex); var hasDefaultValue = parameterPart.Default != null || parameterPart.IsCatchAll; - slots.Add(hasDefaultValue ? new KeyValuePair(parameterPart.Name, parameterPart.Default) : default); + _slots.Add(hasDefaultValue ? new KeyValuePair(parameterPart.Name, parameterPart.Default) : default); } if (parameterPart.IsCatchAll) @@ -415,7 +459,7 @@ namespace Microsoft.AspNetCore.Routing.Matching } else { - captures.Add((parameterPart.Name, i, slotIndex)); + _captures.Add((parameterPart.Name, i, slotIndex)); } } @@ -427,7 +471,7 @@ namespace Microsoft.AspNetCore.Routing.Matching continue; } - complexSegments.Add((segment, i)); + _complexSegments.Add((segment, i)); } foreach (var kvp in routeEndpoint.RoutePattern.ParameterPolicies) @@ -440,7 +484,7 @@ namespace Microsoft.AspNetCore.Routing.Matching var parameterPolicy = _parameterPolicyFactory.Create(parameter, reference); if (parameterPolicy is IRouteConstraint routeConstraint) { - constraints.Add(new KeyValuePair(kvp.Key, routeConstraint)); + _constraints.Add(new KeyValuePair(kvp.Key, routeConstraint)); } } } @@ -449,11 +493,11 @@ namespace Microsoft.AspNetCore.Routing.Matching return new Candidate( endpoint, score, - slots.ToArray(), - captures.ToArray(), + _slots.ToArray(), + _captures.ToArray(), catchAll, - complexSegments.ToArray(), - constraints.ToArray()); + _complexSegments.ToArray(), + _constraints.ToArray()); } private int[] GetGroupLengths(DfaNode node) diff --git a/src/Microsoft.AspNetCore.Routing/Matching/HttpMethodMatcherPolicy.cs b/src/Microsoft.AspNetCore.Routing/Matching/HttpMethodMatcherPolicy.cs index 86706fed0d..d51c51c2ba 100644 --- a/src/Microsoft.AspNetCore.Routing/Matching/HttpMethodMatcherPolicy.cs +++ b/src/Microsoft.AspNetCore.Routing/Matching/HttpMethodMatcherPolicy.cs @@ -362,7 +362,7 @@ namespace Microsoft.AspNetCore.Routing.Matching public override int GetHashCode() { var hash = new HashCodeCombiner(); - hash.Add(IsCorsPreflightRequest); + hash.Add(IsCorsPreflightRequest ? 1 : 0); hash.Add(HttpMethod, StringComparer.Ordinal); return hash; } From 0f5d471dfd90214612d27726d2e2c771c0aa88cd Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 6 Sep 2018 12:58:57 +1200 Subject: [PATCH 4/9] Create collections on DfaNode as needed (#779) --- .../Internal/DfaGraphWriter.cs | 14 +- .../Matching/DfaMatcherBuilder.cs | 72 +++++--- .../Matching/DfaNode.cs | 76 ++++++-- .../Matching/DfaMatcherBuilderTest.cs | 166 ++++++++++++------ 4 files changed, 224 insertions(+), 104 deletions(-) diff --git a/src/Microsoft.AspNetCore.Routing/Internal/DfaGraphWriter.cs b/src/Microsoft.AspNetCore.Routing/Internal/DfaGraphWriter.cs index eb2d2f10ec..5a42cf493b 100644 --- a/src/Microsoft.AspNetCore.Routing/Internal/DfaGraphWriter.cs +++ b/src/Microsoft.AspNetCore.Routing/Internal/DfaGraphWriter.cs @@ -64,9 +64,12 @@ namespace Microsoft.AspNetCore.Routing.Internal // We can safely index into visited because this is a post-order traversal, // all of the children of this node are already in the dictionary. - foreach (var literal in node.Literals) + if (node.Literals != null) { - writer.WriteLine($"{label} -> {visited[literal.Value]} [label=\"/{literal.Key}\"]"); + foreach (var literal in node.Literals) + { + writer.WriteLine($"{label} -> {visited[literal.Value]} [label=\"/{literal.Key}\"]"); + } } if (node.Parameters != null) @@ -79,9 +82,12 @@ namespace Microsoft.AspNetCore.Routing.Internal writer.WriteLine($"{label} -> {visited[node.CatchAll]} [label=\"/**\"]"); } - foreach (var policy in node.PolicyEdges) + if (node.PolicyEdges != null) { - writer.WriteLine($"{label} -> {visited[policy.Value]} [label=\"{policy.Key}\"]"); + foreach (var policy in node.PolicyEdges) + { + writer.WriteLine($"{label} -> {visited[policy.Value]} [label=\"{policy.Key}\"]"); + } } writer.WriteLine($"{label} [label=\"{node.Label}\"]"); diff --git a/src/Microsoft.AspNetCore.Routing/Matching/DfaMatcherBuilder.cs b/src/Microsoft.AspNetCore.Routing/Matching/DfaMatcherBuilder.cs index abe57ab1e7..1573178b13 100644 --- a/src/Microsoft.AspNetCore.Routing/Matching/DfaMatcherBuilder.cs +++ b/src/Microsoft.AspNetCore.Routing/Matching/DfaMatcherBuilder.cs @@ -105,7 +105,7 @@ namespace Microsoft.AspNetCore.Routing.Matching for (var j = 0; j < parents.Count; j++) { var parent = parents[j]; - parent.Matches.Add(endpoint); + parent.AddMatch(endpoint); } } @@ -139,15 +139,17 @@ namespace Microsoft.AspNetCore.Routing.Matching var part = segment.Parts[0]; if (segment.IsSimple && part is RoutePatternLiteralPart literalPart) { + DfaNode next = null; var literal = literalPart.Content; - if (!parent.Literals.TryGetValue(literal, out var next)) + if (parent.Literals == null || + !parent.Literals.TryGetValue(literal, out next)) { next = new DfaNode() { PathDepth = parent.PathDepth + 1, Label = parent.Label + literal + "/", }; - parent.Literals.Add(literal, next); + parent.AddLiteral(literal, next); } nextParents.Add(next); @@ -157,7 +159,10 @@ namespace Microsoft.AspNetCore.Routing.Matching // A catch all should traverse all literal nodes as well as parameter nodes // we don't need to create the parameter node here because of ordering // all catchalls will be processed after all parameters. - nextParents.AddRange(parent.Literals.Values); + if (parent.Literals != null) + { + nextParents.AddRange(parent.Literals.Values); + } if (parent.Parameters != null) { nextParents.Add(parent.Parameters); @@ -181,7 +186,7 @@ namespace Microsoft.AspNetCore.Routing.Matching parent.CatchAll.CatchAll = parent.CatchAll; } - parent.CatchAll.Matches.Add(endpoint); + parent.CatchAll.AddMatch(endpoint); } else if (segment.IsSimple && part.IsParameter) { @@ -195,7 +200,10 @@ namespace Microsoft.AspNetCore.Routing.Matching } // A parameter should traverse all literal nodes as well as the parameter node - nextParents.AddRange(parent.Literals.Values); + if (parent.Literals != null) + { + nextParents.AddRange(parent.Literals.Values); + } nextParents.Add(parent.Parameters); } else @@ -213,7 +221,10 @@ namespace Microsoft.AspNetCore.Routing.Matching }; } - nextParents.AddRange(parent.Literals.Values); + if (parent.Literals != null) + { + nextParents.AddRange(parent.Literals.Values); + } nextParents.Add(parent.Parameters); } } @@ -311,7 +322,7 @@ namespace Microsoft.AspNetCore.Routing.Matching List states, List<(JumpTableBuilder pathBuilder, PolicyJumpTableBuilder policyBuilder)> tableBuilders) { - node.Matches.Sort(_comparer); + node.Matches?.Sort(_comparer); var stateIndex = states.Count; @@ -321,15 +332,18 @@ namespace Microsoft.AspNetCore.Routing.Matching var pathBuilder = new JumpTableBuilder(); tableBuilders.Add((pathBuilder, null)); - foreach (var kvp in node.Literals) + if (node.Literals != null) { - if (kvp.Key == null) + foreach (var kvp in node.Literals) { - continue; - } + if (kvp.Key == null) + { + continue; + } - var transition = Transition(kvp.Value); - pathBuilder.AddEntry(kvp.Key, transition); + var transition = Transition(kvp.Value); + pathBuilder.AddEntry(kvp.Key, transition); + } } if (node.Parameters != null && @@ -360,7 +374,7 @@ namespace Microsoft.AspNetCore.Routing.Matching pathBuilder.ExitDestination = pathBuilder.DefaultDestination; } - if (node.PolicyEdges.Count > 0) + if (node.PolicyEdges != null && node.PolicyEdges.Count > 0) { var policyBuilder = new PolicyJumpTableBuilder(node.NodeBuilder); tableBuilders[stateIndex] = (pathBuilder, policyBuilder); @@ -384,7 +398,7 @@ namespace Microsoft.AspNetCore.Routing.Matching // endpoint. internal Candidate[] CreateCandidates(IReadOnlyList endpoints) { - if (endpoints.Count == 0) + if (endpoints == null || endpoints.Count == 0) { return Array.Empty(); } @@ -502,7 +516,8 @@ namespace Microsoft.AspNetCore.Routing.Matching private int[] GetGroupLengths(DfaNode node) { - if (node.Matches.Count == 0) + var nodeMatches = node.Matches; + if (nodeMatches == null || nodeMatches.Count == 0) { return Array.Empty(); } @@ -510,16 +525,16 @@ namespace Microsoft.AspNetCore.Routing.Matching var groups = new List(); var length = 1; - var exemplar = node.Matches[0]; + var exemplar = nodeMatches[0]; - for (var i = 1; i < node.Matches.Count; i++) + for (var i = 1; i < nodeMatches.Count; i++) { - if (!_comparer.Equals(exemplar, node.Matches[i])) + if (!_comparer.Equals(exemplar, nodeMatches[i])) { groups.Add(length); length = 0; - exemplar = node.Matches[i]; + exemplar = nodeMatches[i]; } length++; @@ -561,7 +576,7 @@ namespace Microsoft.AspNetCore.Routing.Matching private void ApplyPolicies(DfaNode node) { - if (node.Matches.Count == 0) + if (node.Matches == null || node.Matches.Count == 0) { return; } @@ -578,7 +593,7 @@ namespace Microsoft.AspNetCore.Routing.Matching for (var j = 0; j < work.Count; j++) { var parent = work[j]; - if (!nodeBuilder.AppliesToNode(parent.Matches)) + if (!nodeBuilder.AppliesToNode(parent.Matches ?? (IReadOnlyList)Array.Empty())) { // This node-builder doesn't care about this node, so add it to the list // to be processed by the next node-builder. @@ -588,7 +603,7 @@ namespace Microsoft.AspNetCore.Routing.Matching // This node-builder does apply to this node, so we need to create new nodes for each edge, // and then attach them to the parent. - var edges = nodeBuilder.GetEdges(parent.Matches); + var edges = nodeBuilder.GetEdges(parent.Matches ?? (IReadOnlyList)Array.Empty()); for (var k = 0; k < edges.Count; k++) { var edge = edges[k]; @@ -598,17 +613,20 @@ namespace Microsoft.AspNetCore.Routing.Matching Label = parent.Label + " " + edge.State.ToString(), }; - next.Matches.AddRange(edge.Endpoints); + if (edge.Endpoints.Count > 0) + { + next.AddMatches(edge.Endpoints); + } nextWork.Add(next); - parent.PolicyEdges.Add(edge.State, next); + parent.AddPolicyEdge(edge.State, next); } // Associate the node-builder so we can build a jump table later. parent.NodeBuilder = nodeBuilder; // The parent no longer has matches, it's not considered a terminal node. - parent.Matches.Clear(); + parent.Matches?.Clear(); } work = nextWork; diff --git a/src/Microsoft.AspNetCore.Routing/Matching/DfaNode.cs b/src/Microsoft.AspNetCore.Routing/Matching/DfaNode.cs index e68c4909f7..f3f916b8e5 100644 --- a/src/Microsoft.AspNetCore.Routing/Matching/DfaNode.cs +++ b/src/Microsoft.AspNetCore.Routing/Matching/DfaNode.cs @@ -14,13 +14,6 @@ namespace Microsoft.AspNetCore.Routing.Matching [DebuggerDisplay("{DebuggerToString(),nq}")] internal class DfaNode { - public DfaNode() - { - Literals = new Dictionary(StringComparer.OrdinalIgnoreCase); - Matches = new List(); - PolicyEdges = new Dictionary(); - } - // The depth of the node. The depth indicates the number of segments // that must be processed to arrive at this node. // @@ -30,9 +23,9 @@ namespace Microsoft.AspNetCore.Routing.Matching // Just for diagnostics and debugging public string Label { get; set; } - public List Matches { get; } + public List Matches { get; private set; } - public Dictionary Literals { get; } + public Dictionary Literals { get; private set; } public DfaNode Parameters { get; set; } @@ -40,13 +33,58 @@ namespace Microsoft.AspNetCore.Routing.Matching public INodeBuilderPolicy NodeBuilder { get; set; } - public Dictionary PolicyEdges { get; } + public Dictionary PolicyEdges { get; private set; } + + public void AddPolicyEdge(object state, DfaNode node) + { + if (PolicyEdges == null) + { + PolicyEdges = new Dictionary(); + } + + PolicyEdges.Add(state, node); + } + + public void AddLiteral(string literal, DfaNode node) + { + if (Literals == null) + { + Literals = new Dictionary(StringComparer.OrdinalIgnoreCase); + } + + Literals.Add(literal, node); + } + + public void AddMatch(Endpoint endpoint) + { + if (Matches == null) + { + Matches = new List(); + } + + Matches.Add(endpoint); + } + + public void AddMatches(IEnumerable endpoints) + { + if (Matches == null) + { + Matches = new List(endpoints); + } + else + { + Matches.AddRange(endpoints); + } + } public void Visit(Action visitor) { - foreach (var kvp in Literals) + if (Literals != null) { - kvp.Value.Visit(visitor); + foreach (var kvp in Literals) + { + kvp.Value.Visit(visitor); + } } // Break cycles @@ -61,9 +99,12 @@ namespace Microsoft.AspNetCore.Routing.Matching CatchAll.Visit(visitor); } - foreach (var kvp in PolicyEdges) + if (PolicyEdges != null) { - kvp.Value.Visit(visitor); + foreach (var kvp in PolicyEdges) + { + kvp.Value.Visit(visitor); + } } visitor(this); @@ -76,9 +117,12 @@ namespace Microsoft.AspNetCore.Routing.Matching builder.Append(" d:"); builder.Append(PathDepth); builder.Append(" m:"); - builder.Append(Matches.Count); + builder.Append(Matches?.Count ?? 0); builder.Append(" c: "); - builder.Append(string.Join(", ", Literals.Select(kvp => $"{kvp.Key}->({FormatNode(kvp.Value)})"))); + if (Literals != null) + { + builder.Append(string.Join(", ", Literals.Select(kvp => $"{kvp.Key}->({FormatNode(kvp.Value)})"))); + } return builder.ToString(); // DfaNodes can be self-referential, don't traverse cycles. diff --git a/test/Microsoft.AspNetCore.Routing.Tests/Matching/DfaMatcherBuilderTest.cs b/test/Microsoft.AspNetCore.Routing.Tests/Matching/DfaMatcherBuilderTest.cs index a9071c7962..10c0aec62e 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/Matching/DfaMatcherBuilderTest.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/Matching/DfaMatcherBuilderTest.cs @@ -30,7 +30,7 @@ namespace Microsoft.AspNetCore.Routing.Matching // Assert Assert.Same(endpoint, Assert.Single(root.Matches)); Assert.Null(root.Parameters); - Assert.Empty(root.Literals); + Assert.Null(root.Literals); } [Fact] @@ -46,21 +46,21 @@ namespace Microsoft.AspNetCore.Routing.Matching var root = builder.BuildDfaTree(); // Assert - Assert.Empty(root.Matches); + Assert.Null(root.Matches); Assert.Null(root.Parameters); var next = Assert.Single(root.Literals); Assert.Equal("a", next.Key); var a = next.Value; - Assert.Empty(a.Matches); + Assert.Null(a.Matches); Assert.Null(a.Parameters); next = Assert.Single(a.Literals); Assert.Equal("b", next.Key); var b = next.Value; - Assert.Empty(b.Matches); + Assert.Null(b.Matches); Assert.Null(b.Parameters); next = Assert.Single(b.Literals); @@ -69,7 +69,7 @@ namespace Microsoft.AspNetCore.Routing.Matching var c = next.Value; Assert.Same(endpoint, Assert.Single(c.Matches)); Assert.Null(c.Parameters); - Assert.Empty(c.Literals); + Assert.Null(c.Literals); } [Fact] @@ -85,21 +85,21 @@ namespace Microsoft.AspNetCore.Routing.Matching var root = builder.BuildDfaTree(); // Assert - Assert.Empty(root.Matches); - Assert.Empty(root.Literals); + Assert.Null(root.Matches); + Assert.Null(root.Literals); var a = root.Parameters; - Assert.Empty(a.Matches); - Assert.Empty(a.Literals); + Assert.Null(a.Matches); + Assert.Null(a.Literals); var b = a.Parameters; - Assert.Empty(b.Matches); - Assert.Empty(b.Literals); + Assert.Null(b.Matches); + Assert.Null(b.Literals); var c = b.Parameters; Assert.Same(endpoint, Assert.Single(c.Matches)); Assert.Null(c.Parameters); - Assert.Empty(c.Literals); + Assert.Null(c.Literals); } [Fact] @@ -115,21 +115,21 @@ namespace Microsoft.AspNetCore.Routing.Matching var root = builder.BuildDfaTree(); // Assert - Assert.Empty(root.Matches); - Assert.Empty(root.Literals); + Assert.Null(root.Matches); + Assert.Null(root.Literals); var a = root.Parameters; // The catch all can match a path like '/a' Assert.Same(endpoint, Assert.Single(a.Matches)); - Assert.Empty(a.Literals); + Assert.Null(a.Literals); Assert.Null(a.Parameters); // Catch-all nodes include an extra transition that loops to process // extra segments. var catchAll = a.CatchAll; Assert.Same(endpoint, Assert.Single(catchAll.Matches)); - Assert.Empty(catchAll.Literals); + Assert.Null(catchAll.Literals); Assert.Same(catchAll, catchAll.Parameters); Assert.Same(catchAll, catchAll.CatchAll); } @@ -148,13 +148,13 @@ namespace Microsoft.AspNetCore.Routing.Matching // Assert Assert.Same(endpoint, Assert.Single(root.Matches)); - Assert.Empty(root.Literals); + Assert.Null(root.Literals); // Catch-all nodes include an extra transition that loops to process // extra segments. var catchAll = root.CatchAll; Assert.Same(endpoint, Assert.Single(catchAll.Matches)); - Assert.Empty(catchAll.Literals); + Assert.Null(catchAll.Literals); Assert.Same(catchAll, catchAll.Parameters); } @@ -174,19 +174,19 @@ namespace Microsoft.AspNetCore.Routing.Matching var root = builder.BuildDfaTree(); // Assert - Assert.Empty(root.Matches); + Assert.Null(root.Matches); Assert.Null(root.Parameters); var next = Assert.Single(root.Literals); Assert.Equal("a", next.Key); var a = next.Value; - Assert.Empty(a.Matches); + Assert.Null(a.Matches); Assert.Equal(2, a.Literals.Count); var b1 = a.Literals["b1"]; - Assert.Empty(b1.Matches); + Assert.Null(b1.Matches); Assert.Null(b1.Parameters); next = Assert.Single(b1.Literals); @@ -195,10 +195,10 @@ namespace Microsoft.AspNetCore.Routing.Matching var c1 = next.Value; Assert.Same(endpoint1, Assert.Single(c1.Matches)); Assert.Null(c1.Parameters); - Assert.Empty(c1.Literals); + Assert.Null(c1.Literals); var b2 = a.Literals["b2"]; - Assert.Empty(b2.Matches); + Assert.Null(b2.Matches); Assert.Null(b2.Parameters); next = Assert.Single(b2.Literals); @@ -207,7 +207,59 @@ namespace Microsoft.AspNetCore.Routing.Matching var c2 = next.Value; Assert.Same(endpoint2, Assert.Single(c2.Matches)); Assert.Null(c2.Parameters); - Assert.Empty(c2.Literals); + Assert.Null(c2.Literals); + } + + [Fact] + public void BuildDfaTree_MultipleEndpoint_LiteralDifferentCase() + { + // Arrange + var builder = CreateDfaMatcherBuilder(); + + var endpoint1 = CreateEndpoint("a/b1/c"); + builder.AddEndpoint(endpoint1); + + var endpoint2 = CreateEndpoint("A/b2/c"); + builder.AddEndpoint(endpoint2); + + // Act + var root = builder.BuildDfaTree(); + + // Assert + Assert.Null(root.Matches); + Assert.Null(root.Parameters); + + var next = Assert.Single(root.Literals); + Assert.Equal("a", next.Key); + + var a = next.Value; + Assert.Null(a.Matches); + + Assert.Equal(2, a.Literals.Count); + + var b1 = a.Literals["b1"]; + Assert.Null(b1.Matches); + Assert.Null(b1.Parameters); + + next = Assert.Single(b1.Literals); + Assert.Equal("c", next.Key); + + var c1 = next.Value; + Assert.Same(endpoint1, Assert.Single(c1.Matches)); + Assert.Null(c1.Parameters); + Assert.Null(c1.Literals); + + var b2 = a.Literals["b2"]; + Assert.Null(b2.Matches); + Assert.Null(b2.Parameters); + + next = Assert.Single(b2.Literals); + Assert.Equal("c", next.Key); + + var c2 = next.Value; + Assert.Same(endpoint2, Assert.Single(c2.Matches)); + Assert.Null(c2.Parameters); + Assert.Null(c2.Literals); } [Fact] @@ -226,20 +278,20 @@ namespace Microsoft.AspNetCore.Routing.Matching var root = builder.BuildDfaTree(); // Assert - Assert.Empty(root.Matches); + Assert.Null(root.Matches); Assert.Null(root.Parameters); var next = Assert.Single(root.Literals); Assert.Equal("a", next.Key); var a = next.Value; - Assert.Empty(a.Matches); + Assert.Null(a.Matches); next = Assert.Single(a.Literals); Assert.Equal("b", next.Key); var b = next.Value; - Assert.Empty(b.Matches); + Assert.Null(b.Matches); Assert.Null(b.Parameters); next = Assert.Single(b.Literals); @@ -251,10 +303,10 @@ namespace Microsoft.AspNetCore.Routing.Matching e => Assert.Same(endpoint1, e), e => Assert.Same(endpoint2, e)); Assert.Null(c1.Parameters); - Assert.Empty(c1.Literals); + Assert.Null(c1.Literals); var b2 = a.Parameters; - Assert.Empty(b2.Matches); + Assert.Null(b2.Matches); Assert.Null(b2.Parameters); next = Assert.Single(b2.Literals); @@ -263,7 +315,7 @@ namespace Microsoft.AspNetCore.Routing.Matching var c2 = next.Value; Assert.Same(endpoint2, Assert.Single(c2.Matches)); Assert.Null(c2.Parameters); - Assert.Empty(c2.Literals); + Assert.Null(c2.Literals); } [Fact] @@ -282,18 +334,18 @@ namespace Microsoft.AspNetCore.Routing.Matching var root = builder.BuildDfaTree(); // Assert - Assert.Empty(root.Matches); + Assert.Null(root.Matches); Assert.Null(root.Parameters); var next = Assert.Single(root.Literals); Assert.Equal("a", next.Key); var a = next.Value; - Assert.Empty(a.Matches); - Assert.Empty(a.Literals); + Assert.Null(a.Matches); + Assert.Null(a.Literals); var b = a.Parameters; - Assert.Empty(b.Matches); + Assert.Null(b.Matches); Assert.Null(b.Parameters); next = Assert.Single(b.Literals); @@ -305,7 +357,7 @@ namespace Microsoft.AspNetCore.Routing.Matching e => Assert.Same(endpoint1, e), e => Assert.Same(endpoint2, e)); Assert.Null(c.Parameters); - Assert.Empty(c.Literals); + Assert.Null(c.Literals); } [Fact] @@ -324,7 +376,7 @@ namespace Microsoft.AspNetCore.Routing.Matching var root = builder.BuildDfaTree(); // Assert - Assert.Empty(root.Matches); + Assert.Null(root.Matches); Assert.Null(root.Parameters); var next = Assert.Single(root.Literals); @@ -349,7 +401,7 @@ namespace Microsoft.AspNetCore.Routing.Matching e => Assert.Same(endpoint1, e), e => Assert.Same(endpoint2, e)); Assert.Null(c1.Parameters); - Assert.Empty(c1.Literals); + Assert.Null(c1.Literals); var catchAll = a.CatchAll; Assert.Same(endpoint2, Assert.Single(catchAll.Matches)); @@ -373,7 +425,7 @@ namespace Microsoft.AspNetCore.Routing.Matching var root = builder.BuildDfaTree(); // Assert - Assert.Empty(root.Matches); + Assert.Null(root.Matches); Assert.Null(root.Parameters); var next = Assert.Single(root.Literals); @@ -381,7 +433,7 @@ namespace Microsoft.AspNetCore.Routing.Matching var a = next.Value; Assert.Same(endpoint2, Assert.Single(a.Matches)); - Assert.Empty(a.Literals); + Assert.Null(a.Literals); var b1 = a.Parameters; Assert.Same(endpoint2, Assert.Single(a.Matches)); @@ -396,7 +448,7 @@ namespace Microsoft.AspNetCore.Routing.Matching e => Assert.Same(endpoint1, e), e => Assert.Same(endpoint2, e)); Assert.Null(c1.Parameters); - Assert.Empty(c1.Literals); + Assert.Null(c1.Literals); var catchAll = a.CatchAll; Assert.Same(endpoint2, Assert.Single(catchAll.Matches)); @@ -417,7 +469,7 @@ namespace Microsoft.AspNetCore.Routing.Matching var root = builder.BuildDfaTree(); // Assert - Assert.Empty(root.Matches); + Assert.Null(root.Matches); Assert.Null(root.Parameters); var next = Assert.Single(root.Literals); @@ -440,7 +492,7 @@ namespace Microsoft.AspNetCore.Routing.Matching var test2_true = test1_0.PolicyEdges[true]; Assert.Same(endpoint1, Assert.Single(test2_true.Matches)); Assert.Null(test2_true.NodeBuilder); - Assert.Empty(test2_true.PolicyEdges); + Assert.Null(test2_true.PolicyEdges); } [Fact] @@ -462,7 +514,7 @@ namespace Microsoft.AspNetCore.Routing.Matching var root = builder.BuildDfaTree(); // Assert - Assert.Empty(root.Matches); + Assert.Null(root.Matches); Assert.Null(root.Parameters); var next = Assert.Single(root.Literals); @@ -486,7 +538,7 @@ namespace Microsoft.AspNetCore.Routing.Matching var test2_true = test1_0.PolicyEdges[true]; Assert.Same(endpoint1, Assert.Single(test2_true.Matches)); Assert.Null(test2_true.NodeBuilder); - Assert.Empty(test2_true.PolicyEdges); + Assert.Null(test2_true.PolicyEdges); var test1_1 = a.PolicyEdges[1]; Assert.Empty(test1_1.Matches); @@ -499,12 +551,12 @@ namespace Microsoft.AspNetCore.Routing.Matching test2_true = test1_1.PolicyEdges[true]; Assert.Same(endpoint2, Assert.Single(test2_true.Matches)); Assert.Null(test2_true.NodeBuilder); - Assert.Empty(test2_true.PolicyEdges); + Assert.Null(test2_true.PolicyEdges); var test2_false = test1_1.PolicyEdges[false]; Assert.Same(endpoint3, Assert.Single(test2_false.Matches)); Assert.Null(test2_false.NodeBuilder); - Assert.Empty(test2_false.PolicyEdges); + Assert.Null(test2_false.PolicyEdges); } [Fact] @@ -526,7 +578,7 @@ namespace Microsoft.AspNetCore.Routing.Matching var root = builder.BuildDfaTree(); // Assert - Assert.Empty(root.Matches); + Assert.Null(root.Matches); Assert.Null(root.Parameters); var next = Assert.Single(root.Literals); @@ -543,12 +595,12 @@ namespace Microsoft.AspNetCore.Routing.Matching var test2_true = a.PolicyEdges[true]; Assert.Equal(new[] { endpoint1, endpoint2, }, test2_true.Matches); Assert.Null(test2_true.NodeBuilder); - Assert.Empty(test2_true.PolicyEdges); + Assert.Null(test2_true.PolicyEdges); var test2_false = a.PolicyEdges[false]; Assert.Equal(new[] { endpoint3, }, test2_false.Matches); Assert.Null(test2_false.NodeBuilder); - Assert.Empty(test2_false.PolicyEdges); + Assert.Null(test2_false.PolicyEdges); } [Fact] @@ -570,7 +622,7 @@ namespace Microsoft.AspNetCore.Routing.Matching var root = builder.BuildDfaTree(); // Assert - Assert.Empty(root.Matches); + Assert.Null(root.Matches); Assert.Null(root.Parameters); var next = Assert.Single(root.Literals); @@ -587,12 +639,12 @@ namespace Microsoft.AspNetCore.Routing.Matching var test1_0 = a.PolicyEdges[0]; Assert.Equal(new[] { endpoint1, }, test1_0.Matches); Assert.Null(test1_0.NodeBuilder); - Assert.Empty(test1_0.PolicyEdges); + Assert.Null(test1_0.PolicyEdges); var test1_1 = a.PolicyEdges[1]; Assert.Equal(new[] { endpoint2, endpoint3, }, test1_1.Matches); Assert.Null(test1_1.NodeBuilder); - Assert.Empty(test1_1.PolicyEdges); + Assert.Null(test1_1.PolicyEdges); } [Fact] @@ -614,7 +666,7 @@ namespace Microsoft.AspNetCore.Routing.Matching var root = builder.BuildDfaTree(); // Assert - Assert.Empty(root.Matches); + Assert.Null(root.Matches); Assert.Null(root.Parameters); var next = Assert.Single(root.Literals); @@ -632,12 +684,12 @@ namespace Microsoft.AspNetCore.Routing.Matching var test1_0 = a.PolicyEdges[0]; Assert.Equal(new[] { endpoint1, }, test1_0.Matches); Assert.Null(test1_0.NodeBuilder); - Assert.Empty(test1_0.PolicyEdges); + Assert.Null(test1_0.PolicyEdges); var test1_1 = a.PolicyEdges[1]; Assert.Equal(new[] { endpoint2, endpoint3, }, test1_1.Matches); Assert.Null(test1_1.NodeBuilder); - Assert.Empty(test1_1.PolicyEdges); + Assert.Null(test1_1.PolicyEdges); var nonRouteEndpoint = a.PolicyEdges[int.MaxValue]; Assert.Equal("MaxValueEndpoint", Assert.Single(nonRouteEndpoint.Matches).DisplayName); @@ -662,7 +714,7 @@ namespace Microsoft.AspNetCore.Routing.Matching var root = builder.BuildDfaTree(); // Assert - Assert.Empty(root.Matches); + Assert.Null(root.Matches); Assert.Null(root.Parameters); var next = Assert.Single(root.Literals); @@ -671,7 +723,7 @@ namespace Microsoft.AspNetCore.Routing.Matching var a = next.Value; Assert.Equal(new[] { endpoint1, endpoint2, endpoint3, }, a.Matches); Assert.Null(a.NodeBuilder); - Assert.Empty(a.PolicyEdges); + Assert.Null(a.PolicyEdges); } [Fact] From abc378d3dc55f2070d852d055d66748159d3294d Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 6 Sep 2018 13:10:17 +1200 Subject: [PATCH 5/9] Avoid resizing large struct arrays (#767) --- .../Matching/DfaMatcherBuilder.cs | 53 ++++++++++++------- 1 file changed, 35 insertions(+), 18 deletions(-) diff --git a/src/Microsoft.AspNetCore.Routing/Matching/DfaMatcherBuilder.cs b/src/Microsoft.AspNetCore.Routing/Matching/DfaMatcherBuilder.cs index 1573178b13..5e06a36a5c 100644 --- a/src/Microsoft.AspNetCore.Routing/Matching/DfaMatcherBuilder.cs +++ b/src/Microsoft.AspNetCore.Routing/Matching/DfaMatcherBuilder.cs @@ -26,6 +26,8 @@ namespace Microsoft.AspNetCore.Routing.Matching private readonly List<(RoutePatternPathSegment pathSegment, int segmentIndex)> _complexSegments; private readonly List> _constraints; + private int _stateIndex; + public DfaMatcherBuilder( ParameterPolicyFactory parameterPolicyFactory, EndpointSelector selector, @@ -273,22 +275,29 @@ namespace Microsoft.AspNetCore.Routing.Matching { var root = BuildDfaTree(); + // State count is the number of nodes plus an exit state + var stateCount = 1; var maxSegmentCount = 0; - root.Visit((node) => maxSegmentCount = Math.Max(maxSegmentCount, node.PathDepth)); + root.Visit((node) => + { + stateCount++; + maxSegmentCount = Math.Max(maxSegmentCount, node.PathDepth); + }); + _stateIndex = 0; // The max segment count is the maximum path-node-depth +1. We need // the +1 to capture any additional content after the 'last' segment. maxSegmentCount++; - var states = new List(); - var tableBuilders = new List<(JumpTableBuilder pathBuilder, PolicyJumpTableBuilder policyBuilder)>(); + var states = new DfaState[stateCount]; + var tableBuilders = new (JumpTableBuilder pathBuilder, PolicyJumpTableBuilder policyBuilder)[stateCount]; AddNode(root, states, tableBuilders); - var exit = states.Count; - states.Add(new DfaState(Array.Empty(), null, null)); - tableBuilders.Add((new JumpTableBuilder() { DefaultDestination = exit, ExitDestination = exit, }, null)); + var exit = stateCount - 1; + states[exit] = new DfaState(Array.Empty(), null, null); + tableBuilders[exit] = (new JumpTableBuilder() { DefaultDestination = exit, ExitDestination = exit, }, null); - for (var i = 0; i < tableBuilders.Count; i++) + for (var i = 0; i < tableBuilders.Length; i++) { if (tableBuilders[i].pathBuilder?.DefaultDestination == JumpTableBuilder.InvalidDestination) { @@ -306,31 +315,31 @@ namespace Microsoft.AspNetCore.Routing.Matching } } - for (var i = 0; i < states.Count; i++) + for (var i = 0; i < states.Length; i++) { states[i] = new DfaState( - states[i].Candidates, + states[i].Candidates, tableBuilders[i].pathBuilder?.Build(), tableBuilders[i].policyBuilder?.Build()); } - return new DfaMatcher(_selector, states.ToArray(), maxSegmentCount); + return new DfaMatcher(_selector, states, maxSegmentCount); } private int AddNode( DfaNode node, - List states, - List<(JumpTableBuilder pathBuilder, PolicyJumpTableBuilder policyBuilder)> tableBuilders) + DfaState[] states, + (JumpTableBuilder pathBuilder, PolicyJumpTableBuilder policyBuilder)[] tableBuilders) { node.Matches?.Sort(_comparer); - var stateIndex = states.Count; + var currentStateIndex = _stateIndex; var candidates = CreateCandidates(node.Matches); - states.Add(new DfaState(candidates, null, null)); + states[currentStateIndex] = new DfaState(candidates, null, null); var pathBuilder = new JumpTableBuilder(); - tableBuilders.Add((pathBuilder, null)); + tableBuilders[currentStateIndex] = (pathBuilder, null); if (node.Literals != null) { @@ -377,7 +386,7 @@ namespace Microsoft.AspNetCore.Routing.Matching if (node.PolicyEdges != null && node.PolicyEdges.Count > 0) { var policyBuilder = new PolicyJumpTableBuilder(node.NodeBuilder); - tableBuilders[stateIndex] = (pathBuilder, policyBuilder); + tableBuilders[currentStateIndex] = (pathBuilder, policyBuilder); foreach (var kvp in node.PolicyEdges) { @@ -385,12 +394,20 @@ namespace Microsoft.AspNetCore.Routing.Matching } } - return stateIndex; + return currentStateIndex; int Transition(DfaNode next) { // Break cycles - return ReferenceEquals(node, next) ? stateIndex : AddNode(next, states, tableBuilders); + if (ReferenceEquals(node, next)) + { + return _stateIndex; + } + else + { + _stateIndex++; + return AddNode(next, states, tableBuilders); + } } } From fe8c6332245f2aeb67d35f488671e95b465befd1 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 6 Sep 2018 13:25:35 +1200 Subject: [PATCH 6/9] Optimize builder and HTTP policy allocations (#768) --- .../Matching/DfaMatcherBuilder.cs | 96 ++++++++----------- .../Matching/HttpMethodMatcherPolicy.cs | 24 ++++- .../Matching/JumpTableBuilder.cs | 50 ++++------ .../Matching/PolicyJumpTableBuilder.cs | 32 ------- 4 files changed, 79 insertions(+), 123 deletions(-) delete mode 100644 src/Microsoft.AspNetCore.Routing/Matching/PolicyJumpTableBuilder.cs diff --git a/src/Microsoft.AspNetCore.Routing/Matching/DfaMatcherBuilder.cs b/src/Microsoft.AspNetCore.Routing/Matching/DfaMatcherBuilder.cs index 5e06a36a5c..3592918a9a 100644 --- a/src/Microsoft.AspNetCore.Routing/Matching/DfaMatcherBuilder.cs +++ b/src/Microsoft.AspNetCore.Routing/Matching/DfaMatcherBuilder.cs @@ -64,7 +64,7 @@ namespace Microsoft.AspNetCore.Routing.Matching // Since we're doing a BFS we will process each 'level' of the tree in stages // this list will hold the set of items we need to process at the current // stage. - var work = new List<(RouteEndpoint endpoint, List parents)>(); + var work = new List<(RouteEndpoint endpoint, List parents)>(_endpoints.Count); List<(RouteEndpoint endpoint, List parents)> previousWork = null; var root = new DfaNode() { PathDepth = 0, Label = "/" }; @@ -290,38 +290,13 @@ namespace Microsoft.AspNetCore.Routing.Matching maxSegmentCount++; var states = new DfaState[stateCount]; - var tableBuilders = new (JumpTableBuilder pathBuilder, PolicyJumpTableBuilder policyBuilder)[stateCount]; - AddNode(root, states, tableBuilders); + var exitDestination = stateCount - 1; + AddNode(root, states, exitDestination); - var exit = stateCount - 1; - states[exit] = new DfaState(Array.Empty(), null, null); - tableBuilders[exit] = (new JumpTableBuilder() { DefaultDestination = exit, ExitDestination = exit, }, null); - - for (var i = 0; i < tableBuilders.Length; i++) - { - if (tableBuilders[i].pathBuilder?.DefaultDestination == JumpTableBuilder.InvalidDestination) - { - tableBuilders[i].pathBuilder.DefaultDestination = exit; - } - - if (tableBuilders[i].pathBuilder?.ExitDestination == JumpTableBuilder.InvalidDestination) - { - tableBuilders[i].pathBuilder.ExitDestination = exit; - } - - if (tableBuilders[i].policyBuilder?.ExitDestination == JumpTableBuilder.InvalidDestination) - { - tableBuilders[i].policyBuilder.ExitDestination = exit; - } - } - - for (var i = 0; i < states.Length; i++) - { - states[i] = new DfaState( - states[i].Candidates, - tableBuilders[i].pathBuilder?.Build(), - tableBuilders[i].policyBuilder?.Build()); - } + states[exitDestination] = new DfaState( + Array.Empty(), + JumpTableBuilder.Build(exitDestination, exitDestination, null), + null); return new DfaMatcher(_selector, states, maxSegmentCount); } @@ -329,29 +304,26 @@ namespace Microsoft.AspNetCore.Routing.Matching private int AddNode( DfaNode node, DfaState[] states, - (JumpTableBuilder pathBuilder, PolicyJumpTableBuilder policyBuilder)[] tableBuilders) + int exitDestination) { node.Matches?.Sort(_comparer); var currentStateIndex = _stateIndex; - var candidates = CreateCandidates(node.Matches); - states[currentStateIndex] = new DfaState(candidates, null, null); - - var pathBuilder = new JumpTableBuilder(); - tableBuilders[currentStateIndex] = (pathBuilder, null); + var currentDefaultDestination = exitDestination; + var currentExitDestination = exitDestination; + (string text, int destination)[] pathEntries = null; + PolicyJumpTableEdge[] policyEntries = null; if (node.Literals != null) { + pathEntries = new (string text, int destination)[node.Literals.Count]; + + var index = 0; foreach (var kvp in node.Literals) { - if (kvp.Key == null) - { - continue; - } - var transition = Transition(kvp.Value); - pathBuilder.AddEntry(kvp.Key, transition); + pathEntries[index++] = (kvp.Key, transition); } } @@ -361,39 +333,43 @@ namespace Microsoft.AspNetCore.Routing.Matching { // This node has a single transition to but it should accept zero-width segments // this can happen when a node only has catchall parameters. - pathBuilder.DefaultDestination = Transition(node.Parameters); - pathBuilder.ExitDestination = pathBuilder.DefaultDestination; + currentExitDestination = currentDefaultDestination = Transition(node.Parameters); } else if (node.Parameters != null && node.CatchAll != null) { // This node has a separate transition for zero-width segments // this can happen when a node has both parameters and catchall parameters. - pathBuilder.DefaultDestination = Transition(node.Parameters); - pathBuilder.ExitDestination = Transition(node.CatchAll); + currentDefaultDestination = Transition(node.Parameters); + currentExitDestination = Transition(node.CatchAll); } else if (node.Parameters != null) { // This node has paramters but no catchall. - pathBuilder.DefaultDestination = Transition(node.Parameters); + currentDefaultDestination = Transition(node.Parameters); } else if (node.CatchAll != null) { // This node has a catchall but no parameters - pathBuilder.DefaultDestination = Transition(node.CatchAll); - pathBuilder.ExitDestination = pathBuilder.DefaultDestination; + currentExitDestination = currentDefaultDestination = Transition(node.CatchAll); } if (node.PolicyEdges != null && node.PolicyEdges.Count > 0) { - var policyBuilder = new PolicyJumpTableBuilder(node.NodeBuilder); - tableBuilders[currentStateIndex] = (pathBuilder, policyBuilder); + policyEntries = new PolicyJumpTableEdge[node.PolicyEdges.Count]; + var index = 0; foreach (var kvp in node.PolicyEdges) { - policyBuilder.AddEntry(kvp.Key, Transition(kvp.Value)); + policyEntries[index++] = new PolicyJumpTableEdge(kvp.Key, Transition(kvp.Value)); } } + var candidates = CreateCandidates(node.Matches); + states[currentStateIndex] = new DfaState( + candidates, + JumpTableBuilder.Build(currentDefaultDestination, currentExitDestination, pathEntries), + BuildPolicy(currentExitDestination, node.NodeBuilder, policyEntries)); + return currentStateIndex; int Transition(DfaNode next) @@ -406,11 +382,21 @@ namespace Microsoft.AspNetCore.Routing.Matching else { _stateIndex++; - return AddNode(next, states, tableBuilders); + return AddNode(next, states, exitDestination); } } } + private static PolicyJumpTable BuildPolicy(int exitDestination, INodeBuilderPolicy nodeBuilder, PolicyJumpTableEdge[] policyEntries) + { + if (policyEntries == null) + { + return null; + } + + return nodeBuilder.BuildJumpTable(exitDestination, policyEntries); + } + // Builds an array of candidates for a node, assigns a 'score' for each // endpoint. internal Candidate[] CreateCandidates(IReadOnlyList endpoints) diff --git a/src/Microsoft.AspNetCore.Routing/Matching/HttpMethodMatcherPolicy.cs b/src/Microsoft.AspNetCore.Routing/Matching/HttpMethodMatcherPolicy.cs index d51c51c2ba..652a290230 100644 --- a/src/Microsoft.AspNetCore.Routing/Matching/HttpMethodMatcherPolicy.cs +++ b/src/Microsoft.AspNetCore.Routing/Matching/HttpMethodMatcherPolicy.cs @@ -80,7 +80,7 @@ namespace Microsoft.AspNetCore.Routing.Matching // // For now we're just building up the set of keys. We don't add any endpoints // to lists now because we don't want ordering problems. - var allHttpMethods = new HashSet(StringComparer.OrdinalIgnoreCase); + var allHttpMethods = new List(); var edges = new Dictionary>(); for (var i = 0; i < endpoints.Count; i++) { @@ -120,11 +120,16 @@ namespace Microsoft.AspNetCore.Routing.Matching // Also if it's not the *any* method key, then track it. if (!string.Equals(AnyMethod, httpMethod, StringComparison.OrdinalIgnoreCase)) { - allHttpMethods.Add(httpMethod); + if (!ContainsHttpMethod(allHttpMethods, httpMethod)) + { + allHttpMethods.Add(httpMethod); + } } } } + allHttpMethods.Sort(StringComparer.OrdinalIgnoreCase); + // Now in a second loop, add endpoints to these lists. We've enumerated all of // the states, so we want to see which states this endpoint matches. for (var i = 0; i < endpoints.Count; i++) @@ -185,7 +190,7 @@ namespace Microsoft.AspNetCore.Routing.Matching if (!edges.TryGetValue(new EdgeKey(AnyMethod, false), out var matches)) { // Methods sorted for testability. - var endpoint = CreateRejectionEndpoint(allHttpMethods.OrderBy(m => m)); + var endpoint = CreateRejectionEndpoint(allHttpMethods); matches = new List() { endpoint, }; edges[new EdgeKey(AnyMethod, false)] = matches; } @@ -261,6 +266,19 @@ namespace Microsoft.AspNetCore.Routing.Matching Http405EndpointDisplayName); } + private static bool ContainsHttpMethod(List httpMethods, string httpMethod) + { + for (var i = 0; i < httpMethods.Count; i++) + { + if (StringComparer.OrdinalIgnoreCase.Equals(httpMethods[i], httpMethod)) + { + return true; + } + } + + return false; + } + private class HttpMethodPolicyJumpTable : PolicyJumpTable { private readonly int _exitDestination; diff --git a/src/Microsoft.AspNetCore.Routing/Matching/JumpTableBuilder.cs b/src/Microsoft.AspNetCore.Routing/Matching/JumpTableBuilder.cs index dcd5e3abc3..69939ffeeb 100644 --- a/src/Microsoft.AspNetCore.Routing/Matching/JumpTableBuilder.cs +++ b/src/Microsoft.AspNetCore.Routing/Matching/JumpTableBuilder.cs @@ -2,39 +2,24 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Collections.Generic; namespace Microsoft.AspNetCore.Routing.Matching { - internal class JumpTableBuilder + internal static class JumpTableBuilder { public static readonly int InvalidDestination = -1; - private readonly List<(string text, int destination)> _entries = new List<(string text, int destination)>(); - - // The destination state when none of the text entries match. - public int DefaultDestination { get; set; } = InvalidDestination; - - // The destination state for a zero-length segment. This is a special - // case because parameters don't match a zero-length segment. - public int ExitDestination { get; set; } = InvalidDestination; - - public void AddEntry(string text, int destination) + public static JumpTable Build(int defaultDestination, int exitDestination, (string text, int destination)[] pathEntries) { - _entries.Add((text, destination)); - } - - public JumpTable Build() - { - if (DefaultDestination == InvalidDestination) + if (defaultDestination == InvalidDestination) { - var message = $"{nameof(DefaultDestination)} is not set. Please report this as a bug."; + var message = $"{nameof(defaultDestination)} is not set. Please report this as a bug."; throw new InvalidOperationException(message); } - if (ExitDestination == InvalidDestination) + if (exitDestination == InvalidDestination) { - var message = $"{nameof(ExitDestination)} is not set. Please report this as a bug."; + var message = $"{nameof(exitDestination)} is not set. Please report this as a bug."; throw new InvalidOperationException(message); } @@ -49,16 +34,16 @@ namespace Microsoft.AspNetCore.Routing.Matching // We have an optimized fast path for zero entries since we don't have to // do any string comparisons. - if (_entries.Count == 0) + if (pathEntries == null || pathEntries.Length == 0) { - return new ZeroEntryJumpTable(DefaultDestination, ExitDestination); + return new ZeroEntryJumpTable(defaultDestination, exitDestination); } // The IL Emit jump table is not faster for a single entry - if (_entries.Count == 1) + if (pathEntries.Length == 1) { - var entry = _entries[0]; - return new SingleEntryJumpTable(DefaultDestination, ExitDestination, entry.text, entry.destination); + var entry = pathEntries[0]; + return new SingleEntryJumpTable(defaultDestination, exitDestination, entry.text, entry.destination); } // We choose a hard upper bound of 100 as the limit for when we switch to a dictionary @@ -72,9 +57,9 @@ namespace Microsoft.AspNetCore.Routing.Matching // Additionally if we're on 32bit, the scalability is worse, so switch to the dictionary at 50 // entries. var threshold = IntPtr.Size == 8 ? 100 : 50; - if (_entries.Count >= threshold) + if (pathEntries.Length >= threshold) { - return new DictionaryJumpTable(DefaultDestination, ExitDestination, _entries.ToArray()); + return new DictionaryJumpTable(defaultDestination, exitDestination, pathEntries); } // If we have more than a single string, the IL emit strategy is the fastest - but we need to decide @@ -82,18 +67,17 @@ namespace Microsoft.AspNetCore.Routing.Matching JumpTable fallback; // Based on our testing a linear search is still faster than a dictionary at ten entries. - if (_entries.Count <= 10) + if (pathEntries.Length <= 10) { - fallback = new LinearSearchJumpTable(DefaultDestination, ExitDestination, _entries.ToArray()); + fallback = new LinearSearchJumpTable(defaultDestination, exitDestination, pathEntries); } else { - fallback = new DictionaryJumpTable(DefaultDestination, ExitDestination, _entries.ToArray()); + fallback = new DictionaryJumpTable(defaultDestination, exitDestination, pathEntries); } #if IL_EMIT - - return new ILEmitTrieJumpTable(DefaultDestination, ExitDestination, _entries.ToArray(), vectorize: null, fallback); + return new ILEmitTrieJumpTable(defaultDestination, exitDestination, pathEntries, vectorize: null, fallback); #else return fallback; #endif diff --git a/src/Microsoft.AspNetCore.Routing/Matching/PolicyJumpTableBuilder.cs b/src/Microsoft.AspNetCore.Routing/Matching/PolicyJumpTableBuilder.cs deleted file mode 100644 index f533a9813c..0000000000 --- a/src/Microsoft.AspNetCore.Routing/Matching/PolicyJumpTableBuilder.cs +++ /dev/null @@ -1,32 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System.Collections.Generic; - -namespace Microsoft.AspNetCore.Routing.Matching -{ - internal class PolicyJumpTableBuilder - { - private readonly INodeBuilderPolicy _nodeBuilder; - private readonly List _entries; - - public PolicyJumpTableBuilder(INodeBuilderPolicy nodeBuilder) - { - _nodeBuilder = nodeBuilder; - _entries = new List(); - } - - // The destination state for a non-match. - public int ExitDestination { get; set; } = JumpTableBuilder.InvalidDestination; - - public void AddEntry(object state, int destination) - { - _entries.Add(new PolicyJumpTableEdge(state, destination)); - } - - public PolicyJumpTable Build() - { - return _nodeBuilder.BuildJumpTable(ExitDestination, _entries); - } - } -} From dbebdbecd60ce1479d27da610d1b9b2247c04007 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 6 Sep 2018 13:38:17 +1200 Subject: [PATCH 7/9] Add flag for including label with DfaNodes (#769) --- .../Internal/DfaGraphWriter.cs | 2 +- .../Matching/DfaMatcherBuilder.cs | 23 ++++++++++++------- .../Matching/HttpMethodMatcherPolicy.cs | 11 ++++++--- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/Microsoft.AspNetCore.Routing/Internal/DfaGraphWriter.cs b/src/Microsoft.AspNetCore.Routing/Internal/DfaGraphWriter.cs index 5a42cf493b..ed1c3f2d9f 100644 --- a/src/Microsoft.AspNetCore.Routing/Internal/DfaGraphWriter.cs +++ b/src/Microsoft.AspNetCore.Routing/Internal/DfaGraphWriter.cs @@ -47,7 +47,7 @@ namespace Microsoft.AspNetCore.Routing.Internal // Assign each node a sequential index. var visited = new Dictionary(); - var tree = builder.BuildDfaTree(); + var tree = builder.BuildDfaTree(includeLabel: true); writer.WriteLine("digraph DFA {"); tree.Visit(WriteNode); diff --git a/src/Microsoft.AspNetCore.Routing/Matching/DfaMatcherBuilder.cs b/src/Microsoft.AspNetCore.Routing/Matching/DfaMatcherBuilder.cs index 3592918a9a..7cc748ddf4 100644 --- a/src/Microsoft.AspNetCore.Routing/Matching/DfaMatcherBuilder.cs +++ b/src/Microsoft.AspNetCore.Routing/Matching/DfaMatcherBuilder.cs @@ -53,7 +53,7 @@ namespace Microsoft.AspNetCore.Routing.Matching _endpoints.Add(endpoint); } - public DfaNode BuildDfaTree() + public DfaNode BuildDfaTree(bool includeLabel = false) { // We build the tree by doing a BFS over the list of entries. This is important // because a 'parameter' node can also traverse the same paths that literal nodes @@ -67,7 +67,7 @@ namespace Microsoft.AspNetCore.Routing.Matching var work = new List<(RouteEndpoint endpoint, List parents)>(_endpoints.Count); List<(RouteEndpoint endpoint, List parents)> previousWork = null; - var root = new DfaNode() { PathDepth = 0, Label = "/" }; + var root = new DfaNode() { PathDepth = 0, Label = includeLabel ? "/" : null }; // To prepare for this we need to compute the max depth, as well as // a seed list of items to process (entry, root). @@ -149,7 +149,7 @@ namespace Microsoft.AspNetCore.Routing.Matching next = new DfaNode() { PathDepth = parent.PathDepth + 1, - Label = parent.Label + literal + "/", + Label = includeLabel ? parent.Label + literal + "/" : null, }; parent.AddLiteral(literal, next); } @@ -180,7 +180,7 @@ namespace Microsoft.AspNetCore.Routing.Matching parent.CatchAll = new DfaNode() { PathDepth = parent.PathDepth + 1, - Label = parent.Label + "{*...}/", + Label = includeLabel ? parent.Label + "{*...}/" : null, }; // The catchall node just loops. @@ -197,7 +197,7 @@ namespace Microsoft.AspNetCore.Routing.Matching parent.Parameters = new DfaNode() { PathDepth = parent.PathDepth + 1, - Label = parent.Label + "{...}/", + Label = includeLabel ? parent.Label + "{...}/" : null, }; } @@ -219,7 +219,7 @@ namespace Microsoft.AspNetCore.Routing.Matching parent.Parameters = new DfaNode() { PathDepth = parent.PathDepth + 1, - Label = parent.Label + "{...}/", + Label = includeLabel ? parent.Label + "{...}/" : null, }; } @@ -273,7 +273,13 @@ namespace Microsoft.AspNetCore.Routing.Matching public override Matcher Build() { - var root = BuildDfaTree(); +#if DEBUG + var includeLabel = true; +#else + var includeLabel = false; +#endif + + var root = BuildDfaTree(includeLabel); // State count is the number of nodes plus an exit state var stateCount = 1; @@ -613,7 +619,8 @@ namespace Microsoft.AspNetCore.Routing.Matching var next = new DfaNode() { - Label = parent.Label + " " + edge.State.ToString(), + // If parent label is null then labels are not being included + Label = (parent.Label != null) ? parent.Label + " " + edge.State.ToString() : null, }; if (edge.Endpoints.Count > 0) diff --git a/src/Microsoft.AspNetCore.Routing/Matching/HttpMethodMatcherPolicy.cs b/src/Microsoft.AspNetCore.Routing/Matching/HttpMethodMatcherPolicy.cs index 652a290230..f6149531d4 100644 --- a/src/Microsoft.AspNetCore.Routing/Matching/HttpMethodMatcherPolicy.cs +++ b/src/Microsoft.AspNetCore.Routing/Matching/HttpMethodMatcherPolicy.cs @@ -195,9 +195,14 @@ namespace Microsoft.AspNetCore.Routing.Matching edges[new EdgeKey(AnyMethod, false)] = matches; } - return edges - .Select(kvp => new PolicyNodeEdge(kvp.Key, kvp.Value)) - .ToArray(); + var policyNodeEdges = new PolicyNodeEdge[edges.Count]; + var index = 0; + foreach (var kvp in edges) + { + policyNodeEdges[index++] = new PolicyNodeEdge(kvp.Key, kvp.Value); + } + + return policyNodeEdges; (IReadOnlyList httpMethods, bool acceptCorsPreflight) GetHttpMethods(Endpoint e) { From 1c74973c6ad04d12c7e6eca05fe3b4f8b403f5e3 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 6 Sep 2018 13:47:34 +1200 Subject: [PATCH 8/9] Minor matcher builder allocation optimization (#771) --- .../Matching/DfaMatcherBuilder.cs | 14 +++++++++- .../Matching/HttpMethodMatcherPolicy.cs | 28 +++++++++++++------ 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/src/Microsoft.AspNetCore.Routing/Matching/DfaMatcherBuilder.cs b/src/Microsoft.AspNetCore.Routing/Matching/DfaMatcherBuilder.cs index 7cc748ddf4..28b5cdf353 100644 --- a/src/Microsoft.AspNetCore.Routing/Matching/DfaMatcherBuilder.cs +++ b/src/Microsoft.AspNetCore.Routing/Matching/DfaMatcherBuilder.cs @@ -592,12 +592,23 @@ namespace Microsoft.AspNetCore.Routing.Matching // Start with the current node as the root. var work = new List() { node, }; + List previousWork = null; for (var i = 0; i < _nodeBuilders.Length; i++) { var nodeBuilder = _nodeBuilders[i]; // Build a list of each - var nextWork = new List(); + List nextWork; + if (previousWork == null) + { + nextWork = new List(); + } + else + { + // Reuse previous collection for the next collection + previousWork.Clear(); + nextWork = previousWork; + } for (var j = 0; j < work.Count; j++) { @@ -639,6 +650,7 @@ namespace Microsoft.AspNetCore.Routing.Matching parent.Matches?.Clear(); } + previousWork = work; work = nextWork; } } diff --git a/src/Microsoft.AspNetCore.Routing/Matching/HttpMethodMatcherPolicy.cs b/src/Microsoft.AspNetCore.Routing/Matching/HttpMethodMatcherPolicy.cs index f6149531d4..8a5beb5bd9 100644 --- a/src/Microsoft.AspNetCore.Routing/Matching/HttpMethodMatcherPolicy.cs +++ b/src/Microsoft.AspNetCore.Routing/Matching/HttpMethodMatcherPolicy.cs @@ -56,7 +56,7 @@ namespace Microsoft.AspNetCore.Routing.Matching for (var i = 0; i < endpoints.Count; i++) { - if (endpoints[i].Metadata.GetMetadata()?.HttpMethods.Any() == true) + if (endpoints[i].Metadata.GetMetadata()?.HttpMethods.Count > 0) { return true; } @@ -219,31 +219,41 @@ namespace Microsoft.AspNetCore.Routing.Matching /// public PolicyJumpTable BuildJumpTable(int exitDestination, IReadOnlyList edges) { - var destinations = new Dictionary(StringComparer.OrdinalIgnoreCase); - var corsPreflightDestinations = new Dictionary(StringComparer.OrdinalIgnoreCase); + Dictionary destinations = null; + Dictionary corsPreflightDestinations = null; for (var i = 0; i < edges.Count; i++) { // We create this data, so it's safe to cast it. var key = (EdgeKey)edges[i].State; if (key.IsCorsPreflightRequest) { + if (corsPreflightDestinations == null) + { + corsPreflightDestinations = new Dictionary(StringComparer.OrdinalIgnoreCase); + } + corsPreflightDestinations.Add(key.HttpMethod, edges[i].Destination); } else { + if (destinations == null) + { + destinations = new Dictionary(StringComparer.OrdinalIgnoreCase); + } + destinations.Add(key.HttpMethod, edges[i].Destination); } } int corsPreflightExitDestination = exitDestination; - if (corsPreflightDestinations.TryGetValue(AnyMethod, out var matchesAnyVerb)) + if (corsPreflightDestinations != null && corsPreflightDestinations.TryGetValue(AnyMethod, out var matchesAnyVerb)) { // If we have endpoints that match any HTTP method, use that as the exit. corsPreflightExitDestination = matchesAnyVerb; corsPreflightDestinations.Remove(AnyMethod); } - if (destinations.TryGetValue(AnyMethod, out matchesAnyVerb)) + if (destinations != null && destinations.TryGetValue(AnyMethod, out matchesAnyVerb)) { // If we have endpoints that match any HTTP method, use that as the exit. exitDestination = matchesAnyVerb; @@ -304,7 +314,7 @@ namespace Microsoft.AspNetCore.Routing.Matching _corsPreflightExitDestination = corsPreflightExitDestination; _corsPreflightDestinations = corsPreflightDestinations; - _supportsCorsPreflight = _corsPreflightDestinations.Count > 0; + _supportsCorsPreflight = _corsPreflightDestinations != null && _corsPreflightDestinations.Count > 0; } public override int GetDestination(HttpContext httpContext) @@ -318,12 +328,14 @@ namespace Microsoft.AspNetCore.Routing.Matching httpContext.Request.Headers.TryGetValue(AccessControlRequestMethod, out var accessControlRequestMethod) && !StringValues.IsNullOrEmpty(accessControlRequestMethod)) { - return _corsPreflightDestinations.TryGetValue(accessControlRequestMethod, out destination) + return _corsPreflightDestinations != null && + _corsPreflightDestinations.TryGetValue(accessControlRequestMethod, out destination) ? destination : _corsPreflightExitDestination; } - return _destinations.TryGetValue(httpMethod, out destination) ? destination : _exitDestination; + return _destinations != null && + _destinations.TryGetValue(httpMethod, out destination) ? destination : _exitDestination; } } From dcfb63a7684bfd1ad1a24804b08015abe68e836d Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 6 Sep 2018 14:00:52 +1200 Subject: [PATCH 9/9] Avoid RoutePattern allocating empty dictionaries (#772) --- .../Matching/RouteEndpointAzureBenchmark.cs | 16 ++++++ .../Patterns/RoutePatternFactory.cs | 55 +++++++++++++++---- 2 files changed, 59 insertions(+), 12 deletions(-) create mode 100644 benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/RouteEndpointAzureBenchmark.cs diff --git a/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/RouteEndpointAzureBenchmark.cs b/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/RouteEndpointAzureBenchmark.cs new file mode 100644 index 0000000000..5d3af48562 --- /dev/null +++ b/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/RouteEndpointAzureBenchmark.cs @@ -0,0 +1,16 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using BenchmarkDotNet.Attributes; + +namespace Microsoft.AspNetCore.Routing.Matching +{ + public class RouteEndpointAzureBenchmark : MatcherAzureBenchmarkBase + { + [Benchmark] + public void CreateEndpoints() + { + SetupEndpoints(); + } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Routing/Patterns/RoutePatternFactory.cs b/src/Microsoft.AspNetCore.Routing/Patterns/RoutePatternFactory.cs index 51b869c58c..bbc1e50aca 100644 --- a/src/Microsoft.AspNetCore.Routing/Patterns/RoutePatternFactory.cs +++ b/src/Microsoft.AspNetCore.Routing/Patterns/RoutePatternFactory.cs @@ -2,7 +2,9 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections; using System.Collections.Generic; +using System.Collections.ObjectModel; using System.Linq; using Microsoft.AspNetCore.Routing.Constraints; using Microsoft.AspNetCore.Routing.Matching; @@ -16,6 +18,12 @@ namespace Microsoft.AspNetCore.Routing.Patterns /// public static class RoutePatternFactory { + private static readonly IReadOnlyDictionary EmptyDefaultsDictionary = + new ReadOnlyDictionary(new Dictionary()); + + private static readonly IReadOnlyDictionary> EmptyPoliciesDictionary = + new ReadOnlyDictionary>(new Dictionary>()); + /// /// Creates a from its string representation. /// @@ -242,8 +250,8 @@ namespace Microsoft.AspNetCore.Routing.Patterns private static RoutePattern PatternCore( string rawText, - IDictionary defaults, - IDictionary parameterPolicies, + RouteValueDictionary defaults, + RouteValueDictionary parameterPolicies, IEnumerable segments) { // We want to merge the segment data with the 'out of line' defaults and parameter policies. @@ -257,18 +265,22 @@ namespace Microsoft.AspNetCore.Routing.Patterns // It's important that these two views of the data are consistent. We don't want // values specified out of line to have a different behavior. - var updatedDefaults = new Dictionary(StringComparer.OrdinalIgnoreCase); - if (defaults != null) + Dictionary updatedDefaults = null; + if (defaults != null && defaults.Count > 0) { + updatedDefaults = new Dictionary(defaults.Count, StringComparer.OrdinalIgnoreCase); + foreach (var kvp in defaults) { updatedDefaults.Add(kvp.Key, kvp.Value); } } - var updatedParameterPolicies = new Dictionary>(StringComparer.OrdinalIgnoreCase); - if (parameterPolicies != null) + Dictionary> updatedParameterPolicies = null; + if (parameterPolicies != null && parameterPolicies.Count > 0) { + updatedParameterPolicies = new Dictionary>(parameterPolicies.Count, StringComparer.OrdinalIgnoreCase); + foreach (var kvp in parameterPolicies) { updatedParameterPolicies.Add(kvp.Key, new List() @@ -280,7 +292,7 @@ namespace Microsoft.AspNetCore.Routing.Patterns } } - var parameters = new List(); + List parameters = null; var updatedSegments = segments.ToArray(); for (var i = 0; i < updatedSegments.Length; i++) { @@ -291,6 +303,11 @@ namespace Microsoft.AspNetCore.Routing.Patterns { if (segment.Parts[j] is RoutePatternParameterPart parameter) { + if (parameters == null) + { + parameters = new List(); + } + parameters.Add(parameter); } } @@ -298,9 +315,11 @@ namespace Microsoft.AspNetCore.Routing.Patterns return new RoutePattern( rawText, - updatedDefaults, - updatedParameterPolicies.ToDictionary(kvp => kvp.Key, kvp => (IReadOnlyList)kvp.Value.ToArray()), - parameters, + updatedDefaults ?? EmptyDefaultsDictionary, + updatedParameterPolicies != null + ? updatedParameterPolicies.ToDictionary(kvp => kvp.Key, kvp => (IReadOnlyList)kvp.Value.ToArray()) + : EmptyPoliciesDictionary, + (IReadOnlyList)parameters ?? Array.Empty(), updatedSegments); RoutePatternPathSegment VisitSegment(RoutePatternPathSegment segment) @@ -341,7 +360,7 @@ namespace Microsoft.AspNetCore.Routing.Patterns var parameter = (RoutePatternParameterPart)part; var @default = parameter.Default; - if (updatedDefaults.TryGetValue(parameter.Name, out var newDefault)) + if (updatedDefaults != null && updatedDefaults.TryGetValue(parameter.Name, out var newDefault)) { if (parameter.Default != null && !Equals(newDefault, parameter.Default)) { @@ -360,12 +379,23 @@ namespace Microsoft.AspNetCore.Routing.Patterns if (parameter.Default != null) { + if (updatedDefaults == null) + { + updatedDefaults = new Dictionary(StringComparer.OrdinalIgnoreCase); + } + updatedDefaults[parameter.Name] = parameter.Default; } - if (!updatedParameterPolicies.TryGetValue(parameter.Name, out var parameterConstraints) && + List parameterConstraints = null; + if ((updatedParameterPolicies == null || !updatedParameterPolicies.TryGetValue(parameter.Name, out parameterConstraints)) && parameter.ParameterPolicies.Count > 0) { + if (updatedParameterPolicies == null) + { + updatedParameterPolicies = new Dictionary>(StringComparer.OrdinalIgnoreCase); + } + parameterConstraints = new List(); updatedParameterPolicies.Add(parameter.Name, parameterConstraints); } @@ -391,6 +421,7 @@ namespace Microsoft.AspNetCore.Routing.Patterns parameter.EncodeSlashes); } } + /// /// Creates a from the provided collection /// of parts.