From 2767e69bdd51e2a5795a20f2842d6f01b8012ba4 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Fri, 26 Apr 2019 19:28:44 -0700 Subject: [PATCH] Avoid allocations in more cases (#9788) * Avoid allocations in more cases Updates to DFA Matcher to avoid allocations in more cases. This makes the matcher more pay-for-play. - Avoid allocating an RVD while matching if possible - Avoid allocating the candidate set unless necessary First, avoid creating the RVD unless there are parameters or constraints. This means that the candidate set can contain null route value dictionaries. This seems fine because selectors are already low-level. The route values feature will allocate an RVD when accessed, so code in MVC or middleware won't even see a null RVD. Secondly, avoid creating the candidate set unless there are selectors. This will save an allocation most of the time since we won't need to run selectors is really common cases. The candidate set is needed because selectors mix mutability and async, so it needs to be a reference type. However the default case is that we *don't* need to run selectors. The impact of this is that we make a bunch of methods have an instance variant and a static variant that operates on the array. --- src/Http/HttpAbstractions.sln | 4 +- src/Http/Routing/src/Matching/CandidateSet.cs | 58 +++++++---- .../src/Matching/DefaultEndpointSelector.cs | 40 +++++--- src/Http/Routing/src/Matching/DfaMatcher.cs | 44 +++++---- .../test/UnitTests/Matching/DfaMatcherTest.cs | 98 ++++++++++++++++++- 5 files changed, 184 insertions(+), 60 deletions(-) diff --git a/src/Http/HttpAbstractions.sln b/src/Http/HttpAbstractions.sln index 07fdd02c67..f585ee6c2d 100644 --- a/src/Http/HttpAbstractions.sln +++ b/src/Http/HttpAbstractions.sln @@ -113,7 +113,7 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.HttpOv EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Server.IISIntegration", "..\Servers\IIS\IISIntegration\src\Microsoft.AspNetCore.Server.IISIntegration.csproj", "{1062FCDE-E145-40EC-B175-FDBCAA0C59A0}" EndProject -Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Microsoft.AspNetCore.WebUtilities.Performance", "WebUtilities\perf\Microsoft.AspNetCore.WebUtilities.Performance\Microsoft.AspNetCore.WebUtilities.Performance.csproj", "{21AC56E7-4E77-4B0E-B63E-C8E836E4D14E}" +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.WebUtilities.Performance", "WebUtilities\perf\Microsoft.AspNetCore.WebUtilities.Performance\Microsoft.AspNetCore.WebUtilities.Performance.csproj", "{21AC56E7-4E77-4B0E-B63E-C8E836E4D14E}" EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution @@ -642,7 +642,7 @@ Global {1A866315-5FD5-4F96-BFAC-1447E3CB4514} = {80A090C8-ED02-4DE3-875A-30DCCDBD84BA} {068A1DA0-C7DF-4E3C-9933-4E79A141EFF8} = {80A090C8-ED02-4DE3-875A-30DCCDBD84BA} {8C635944-51F0-4BB0-A89E-CA49A7D9BE7F} = {FB2DCA0F-EB9E-425B-ABBC-D543DBEC090F} - {1A74D674-5D19-4575-B443-8B7ED433EF2B} = {793FFE24-138A-4C3D-81AB-18D625E36230} + {1A74D674-5D19-4575-B443-8B7ED433EF2B} = {14A7B3DE-46C8-4245-B0BD-9AFF3795C163} {B8812D83-0F76-48F4-B716-C7356DB51E72} = {14A7B3DE-46C8-4245-B0BD-9AFF3795C163} {215E7408-A123-4B5F-B625-59ED22031109} = {DC519C5E-CA6E-48CA-BF35-B46305B83013} {8B64326C-A87F-4157-8337-22B5C4D7A4B7} = {DC519C5E-CA6E-48CA-BF35-B46305B83013} diff --git a/src/Http/Routing/src/Matching/CandidateSet.cs b/src/Http/Routing/src/Matching/CandidateSet.cs index 7720634e62..f90183897d 100644 --- a/src/Http/Routing/src/Matching/CandidateSet.cs +++ b/src/Http/Routing/src/Matching/CandidateSet.cs @@ -20,9 +20,7 @@ namespace Microsoft.AspNetCore.Routing.Matching /// public sealed class CandidateSet { - private const int BitVectorSize = 32; - - private CandidateState[] _candidates; + internal CandidateState[] Candidates; /// /// @@ -59,26 +57,32 @@ namespace Microsoft.AspNetCore.Routing.Matching throw new ArgumentException($"The provided {nameof(endpoints)}, {nameof(values)}, and {nameof(scores)} must have the same length."); } - _candidates = new CandidateState[endpoints.Length]; + Candidates = new CandidateState[endpoints.Length]; for (var i = 0; i < endpoints.Length; i++) { - _candidates[i] = new CandidateState(endpoints[i], values[i], scores[i]); + Candidates[i] = new CandidateState(endpoints[i], values[i], scores[i]); } } + // Used in tests. internal CandidateSet(Candidate[] candidates) { - _candidates = new CandidateState[candidates.Length]; + Candidates = new CandidateState[candidates.Length]; for (var i = 0; i < candidates.Length; i++) { - _candidates[i] = new CandidateState(candidates[i].Endpoint, candidates[i].Score); + Candidates[i] = new CandidateState(candidates[i].Endpoint, candidates[i].Score); } } + internal CandidateSet(CandidateState[] candidates) + { + Candidates = candidates; + } + /// /// Gets the count of candidates in the set. /// - public int Count => _candidates.Length; + public int Count => Candidates.Length; /// /// Gets the associated with the candidate @@ -103,7 +107,7 @@ namespace Microsoft.AspNetCore.Routing.Matching ThrowIndexArgumentOutOfRangeException(); } - return ref _candidates[index]; + return ref Candidates[index]; } } @@ -124,7 +128,12 @@ namespace Microsoft.AspNetCore.Routing.Matching ThrowIndexArgumentOutOfRangeException(); } - return _candidates[index].Score >= 0; + return IsValidCandidate(ref Candidates[index]); + } + + internal static bool IsValidCandidate(ref CandidateState candidate) + { + return candidate.Score >= 0; } /// @@ -142,8 +151,15 @@ namespace Microsoft.AspNetCore.Routing.Matching ThrowIndexArgumentOutOfRangeException(); } - ref var original = ref _candidates[index]; - _candidates[index] = new CandidateState(original.Endpoint, original.Values, original.Score >= 0 ^ value ? ~original.Score : original.Score); + ref var original = ref Candidates[index]; + SetValidity(ref original, value); + } + + internal static void SetValidity(ref CandidateState candidate, bool value) + { + var originalScore = candidate.Score; + var score = originalScore >= 0 ^ value ? ~originalScore : originalScore; + candidate = new CandidateState(candidate.Endpoint, candidate.Values, score); } /// @@ -168,7 +184,7 @@ namespace Microsoft.AspNetCore.Routing.Matching ThrowIndexArgumentOutOfRangeException(); } - _candidates[index] = new CandidateState(endpoint, values, _candidates[index].Score); + Candidates[index] = new CandidateState(endpoint, values, Candidates[index].Score); if (endpoint == null) { @@ -229,18 +245,18 @@ namespace Microsoft.AspNetCore.Routing.Matching break; case 1: - ReplaceEndpoint(index, endpoints[0], _candidates[index].Values); + ReplaceEndpoint(index, endpoints[0], Candidates[index].Values); break; default: var score = GetOriginalScore(index); - var values = _candidates[index].Values; + var values = Candidates[index].Values; // Adding candidates requires expanding the array and computing new score values for the new candidates. - var original = _candidates; + var original = Candidates; var candidates = new CandidateState[original.Length - 1 + endpoints.Count]; - _candidates = candidates; + Candidates = candidates; // Since the new endpoints have an unknown ordering relationship to each other, we need to: // - order them @@ -293,12 +309,12 @@ namespace Microsoft.AspNetCore.Routing.Matching scoreOffset++; } - _candidates[i + index] = new CandidateState(buffer[i], values, score + scoreOffset); + Candidates[i + index] = new CandidateState(buffer[i], values, score + scoreOffset); } for (var i = index + 1; i < original.Length; i++) { - _candidates[i + endpoints.Count - 1] = new CandidateState(original[i].Endpoint, original[i].Values, original[i].Score + scoreOffset); + Candidates[i + endpoints.Count - 1] = new CandidateState(original[i].Endpoint, original[i].Values, original[i].Score + scoreOffset); } break; @@ -311,7 +327,7 @@ namespace Microsoft.AspNetCore.Routing.Matching // This is the original score and used to determine if there are ambiguities. private int GetOriginalScore(int index) { - var score = _candidates[index].Score; + var score = Candidates[index].Score; return score >= 0 ? score : ~score; } @@ -320,7 +336,7 @@ namespace Microsoft.AspNetCore.Routing.Matching var score = GetOriginalScore(index); var count = 0; - var candidates = _candidates; + var candidates = Candidates; for (var i = 0; i < candidates.Length; i++) { if (GetOriginalScore(i) == score) diff --git a/src/Http/Routing/src/Matching/DefaultEndpointSelector.cs b/src/Http/Routing/src/Matching/DefaultEndpointSelector.cs index 113e0a4f01..657621b759 100644 --- a/src/Http/Routing/src/Matching/DefaultEndpointSelector.cs +++ b/src/Http/Routing/src/Matching/DefaultEndpointSelector.cs @@ -9,7 +9,7 @@ using Microsoft.AspNetCore.Http; namespace Microsoft.AspNetCore.Routing.Matching { - internal class DefaultEndpointSelector : EndpointSelector + internal sealed class DefaultEndpointSelector : EndpointSelector { public override Task SelectAsync( HttpContext httpContext, @@ -31,9 +31,18 @@ namespace Microsoft.AspNetCore.Routing.Matching throw new ArgumentNullException(nameof(candidateSet)); } + Select(httpContext, context, candidateSet.Candidates); + return Task.CompletedTask; + } + + internal static void Select( + HttpContext httpContext, + EndpointSelectorContext context, + CandidateState[] candidateState) + { // Fast path: We can specialize for trivial numbers of candidates since there can // be no ambiguities - switch (candidateSet.Count) + switch (candidateState.Length) { case 0: { @@ -43,9 +52,9 @@ namespace Microsoft.AspNetCore.Routing.Matching case 1: { - if (candidateSet.IsValidCandidate(0)) + ref var state = ref candidateState[0]; + if (CandidateSet.IsValidCandidate(ref state)) { - ref var state = ref candidateSet[0]; context.Endpoint = state.Endpoint; context.RouteValues = state.Values; } @@ -57,30 +66,28 @@ namespace Microsoft.AspNetCore.Routing.Matching { // Slow path: There's more than one candidate (to say nothing of validity) so we // have to process for ambiguities. - ProcessFinalCandidates(httpContext, context, candidateSet); + ProcessFinalCandidates(httpContext, context, candidateState); break; } } - - return Task.CompletedTask; } private static void ProcessFinalCandidates( HttpContext httpContext, EndpointSelectorContext context, - CandidateSet candidateSet) + CandidateState[] candidateState) { Endpoint endpoint = null; RouteValueDictionary values = null; int? foundScore = null; - for (var i = 0; i < candidateSet.Count; i++) + for (var i = 0; i < candidateState.Length; i++) { - if (!candidateSet.IsValidCandidate(i)) + ref var state = ref candidateState[i]; + if (!CandidateSet.IsValidCandidate(ref state)) { continue; } - ref var state = ref candidateSet[i]; if (foundScore == null) { // This is the first match we've seen - speculatively assign it. @@ -103,7 +110,7 @@ namespace Microsoft.AspNetCore.Routing.Matching // // Don't worry about the 'null == state.Score' case, it returns false. - ReportAmbiguity(candidateSet); + ReportAmbiguity(candidateState); // Unreachable, ReportAmbiguity always throws. throw new NotSupportedException(); @@ -117,16 +124,17 @@ namespace Microsoft.AspNetCore.Routing.Matching } } - private static void ReportAmbiguity(CandidateSet candidates) + private static void ReportAmbiguity(CandidateState[] candidateState) { // If we get here it's the result of an ambiguity - we're OK with this // being a littler slower and more allocatey. var matches = new List(); - for (var i = 0; i < candidates.Count; i++) + for (var i = 0; i < candidateState.Length; i++) { - if (candidates.IsValidCandidate(i)) + ref var state = ref candidateState[i]; + if (CandidateSet.IsValidCandidate(ref state)) { - matches.Add(candidates[i].Endpoint); + matches.Add(state.Endpoint); } } diff --git a/src/Http/Routing/src/Matching/DfaMatcher.cs b/src/Http/Routing/src/Matching/DfaMatcher.cs index 036351760a..441e53d556 100644 --- a/src/Http/Routing/src/Matching/DfaMatcher.cs +++ b/src/Http/Routing/src/Matching/DfaMatcher.cs @@ -101,7 +101,7 @@ namespace Microsoft.AspNetCore.Routing.Matching // set of endpoints before we call the EndpointSelector. // // `candidateSet` is the mutable state that we pass to the EndpointSelector. - var candidateSet = new CandidateSet(candidates); + var candidateState = new CandidateState[candidateCount]; for (var i = 0; i < candidateCount; i++) { @@ -111,17 +111,13 @@ namespace Microsoft.AspNetCore.Routing.Matching // candidate: readonly data about the endpoint and how to match // state: mutable storarge for our processing ref var candidate = ref candidates[i]; - ref var state = ref candidateSet[i]; + ref var state = ref candidateState[i]; + state = new CandidateState(candidate.Endpoint, candidate.Score); var flags = candidate.Flags; // First process all of the parameters and defaults. - RouteValueDictionary values; - if ((flags & Candidate.CandidateFlags.HasSlots) == 0) - { - values = new RouteValueDictionary(); - } - else + if ((flags & Candidate.CandidateFlags.HasSlots) != 0) { // The Slots array has the default values of the route values in it. // @@ -145,29 +141,29 @@ namespace Microsoft.AspNetCore.Routing.Matching ProcessCatchAll(slots, candidate.CatchAll, path, segments); } - values = RouteValueDictionary.FromArray(slots); + state.Values = RouteValueDictionary.FromArray(slots); } - state.Values = values; - // Now that we have the route values, we need to process complex segments. // Complex segments go through an old API that requires a fully-materialized // route value dictionary. var isMatch = true; if ((flags & Candidate.CandidateFlags.HasComplexSegments) != 0) { - if (!ProcessComplexSegments(candidate.Endpoint, candidate.ComplexSegments, path, segments, values)) + state.Values ??= new RouteValueDictionary(); + if (!ProcessComplexSegments(candidate.Endpoint, candidate.ComplexSegments, path, segments, state.Values)) { - candidateSet.SetValidity(i, false); + CandidateSet.SetValidity(ref state, false); isMatch = false; } } if ((flags & Candidate.CandidateFlags.HasConstraints) != 0) { - if (!ProcessConstraints(candidate.Endpoint, candidate.Constraints, httpContext, values)) + state.Values ??= new RouteValueDictionary(); + if (!ProcessConstraints(candidate.Endpoint, candidate.Constraints, httpContext, state.Values)) { - candidateSet.SetValidity(i, false); + CandidateSet.SetValidity(ref state, false); isMatch = false; } } @@ -185,13 +181,23 @@ namespace Microsoft.AspNetCore.Routing.Matching } } - if (policyCount == 0) + if (policyCount == 0 && _isDefaultEndpointSelector) { - // Perf: avoid a state machine if there are no polices - return _selector.SelectAsync(httpContext, context, candidateSet); + // Fast path that avoids allocating the candidate set. + // + // We can use this when there are no policies and we're using the default selector. + DefaultEndpointSelector.Select(httpContext, context, candidateState); + return Task.CompletedTask; + } + else if (policyCount == 0) + { + // Fast path that avoids a state machine. + // + // We can use this when there are no policies and a non-default selector. + return _selector.SelectAsync(httpContext, context, new CandidateSet(candidateState)); } - return SelectEndpointWithPoliciesAsync(httpContext, context, policies, candidateSet); + return SelectEndpointWithPoliciesAsync(httpContext, context, policies, new CandidateSet(candidateState)); } internal (Candidate[] candidates, IEndpointSelectorPolicy[] policies) FindCandidateSet( diff --git a/src/Http/Routing/test/UnitTests/Matching/DfaMatcherTest.cs b/src/Http/Routing/test/UnitTests/Matching/DfaMatcherTest.cs index e82f870043..52e6dfdfeb 100644 --- a/src/Http/Routing/test/UnitTests/Matching/DfaMatcherTest.cs +++ b/src/Http/Routing/test/UnitTests/Matching/DfaMatcherTest.cs @@ -7,6 +7,7 @@ using System.Linq; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.Routing.Constraints; using Microsoft.AspNetCore.Routing.Patterns; using Microsoft.AspNetCore.Routing.TestObjects; using Microsoft.Extensions.DependencyInjection; @@ -21,9 +22,9 @@ namespace Microsoft.AspNetCore.Routing.Matching // so we're reusing the services here. public class DfaMatcherTest { - private RouteEndpoint CreateEndpoint(string template, int order, object defaults = null, object requiredValues = null) + private RouteEndpoint CreateEndpoint(string template, int order, object defaults = null, object requiredValues = null, object policies = null) { - return EndpointFactory.CreateRouteEndpoint(template, defaults, requiredValues: requiredValues, order: order, displayName: template); + return EndpointFactory.CreateRouteEndpoint(template, defaults, policies, requiredValues, order, displayName: template); } private Matcher CreateDfaMatcher( @@ -399,6 +400,99 @@ namespace Microsoft.AspNetCore.Routing.Matching var endpoint1 = CreateEndpoint("/Teams", 0); var endpoint2 = CreateEndpoint("/Teams", 1); + var endpointSelector = new Mock(); + endpointSelector + .Setup(s => s.SelectAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Callback((c, f, cs) => + { + Assert.Equal(2, cs.Count); + + Assert.Same(endpoint1, cs[0].Endpoint); + Assert.True(cs.IsValidCandidate(0)); + Assert.Equal(0, cs[0].Score); + Assert.Null(cs[0].Values); + + Assert.Same(endpoint2, cs[1].Endpoint); + Assert.True(cs.IsValidCandidate(1)); + Assert.Equal(1, cs[1].Score); + Assert.Null(cs[1].Values); + + f.Endpoint = endpoint2; + }) + .Returns(Task.CompletedTask); + + var endpointDataSource = new DefaultEndpointDataSource(new List + { + endpoint1, + endpoint2 + }); + + var matcher = CreateDfaMatcher(endpointDataSource, endpointSelector: endpointSelector.Object); + + var (httpContext, context) = CreateContext(); + httpContext.Request.Path = "/Teams"; + + // Act + await matcher.MatchAsync(httpContext, context); + + // Assert + Assert.Equal(endpoint2, context.Endpoint); + } + + [Fact] + public async Task MatchAsync_MultipleMatches_EndpointSelectorCalled_AllocatesDictionaryForRouteParameter() + { + // Arrange + var endpoint1 = CreateEndpoint("/Teams/{x?}", 0); + var endpoint2 = CreateEndpoint("/Teams/{x?}", 1); + + var endpointSelector = new Mock(); + endpointSelector + .Setup(s => s.SelectAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Callback((c, f, cs) => + { + Assert.Equal(2, cs.Count); + + Assert.Same(endpoint1, cs[0].Endpoint); + Assert.True(cs.IsValidCandidate(0)); + Assert.Equal(0, cs[0].Score); + Assert.Empty(cs[0].Values); + + Assert.Same(endpoint2, cs[1].Endpoint); + Assert.True(cs.IsValidCandidate(1)); + Assert.Equal(1, cs[1].Score); + Assert.Empty(cs[1].Values); + + f.Endpoint = endpoint2; + }) + .Returns(Task.CompletedTask); + + var endpointDataSource = new DefaultEndpointDataSource(new List + { + endpoint1, + endpoint2 + }); + + var matcher = CreateDfaMatcher(endpointDataSource, endpointSelector: endpointSelector.Object); + + var (httpContext, context) = CreateContext(); + httpContext.Request.Path = "/Teams"; + + // Act + await matcher.MatchAsync(httpContext, context); + + // Assert + Assert.Equal(endpoint2, context.Endpoint); + } + + [Fact] + public async Task MatchAsync_MultipleMatches_EndpointSelectorCalled_AllocatesDictionaryForRouteConstraint() + { + // Arrange + var constraint = new OptionalRouteConstraint(new IntRouteConstraint()); + var endpoint1 = CreateEndpoint("/Teams", 0, policies: new { x = constraint, }); + var endpoint2 = CreateEndpoint("/Teams", 1, policies: new { x = constraint, }); + var endpointSelector = new Mock(); endpointSelector .Setup(s => s.SelectAsync(It.IsAny(), It.IsAny(), It.IsAny()))