Remove dictionary alloc in routing
This changes TemplateMatcher to mutate RouteData.Values directly instead of creating a new dictionary and then merging in values. This is one the biggest single costs in routing in terms of both allocations and execution time. So Match now becomes TryMatch. This will dirty the state of the RVD, so the caller needs to snapshot it before calling into it (handled inside the TreeRouter or RouteCollection). Some subtle changes were needed to how/when values are added to be compatible with the existing tests. The general idea is that we add null values for non-parameter defaults or catchalls, but only if they don't trounce an existing value. This logic used to live in MergeValues but now it's in TryMatch since TryMatch might be working from existing data. Also fixed the .sln to avoid building a package that we use as shared source.
This commit is contained in:
parent
e337756cce
commit
a51c78da06
|
|
@ -1,6 +1,6 @@
|
|||
Microsoft Visual Studio Solution File, Format Version 12.00
|
||||
# Visual Studio 14
|
||||
VisualStudioVersion = 14.0.24711.0
|
||||
VisualStudioVersion = 14.0.25029.0
|
||||
MinimumVisualStudioVersion = 10.0.40219.1
|
||||
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{0E966C37-7334-4D96-AAF6-9F49FBD166E3}"
|
||||
EndProject
|
||||
|
|
@ -70,7 +70,6 @@ Global
|
|||
{ABD5AA59-6000-4A3D-A54F-4B636F725AE8}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
|
||||
{ABD5AA59-6000-4A3D-A54F-4B636F725AE8}.Debug|Any CPU.Build.0 = Debug|Any CPU
|
||||
{ABD5AA59-6000-4A3D-A54F-4B636F725AE8}.Debug|Mixed Platforms.ActiveCfg = Debug|Any CPU
|
||||
{ABD5AA59-6000-4A3D-A54F-4B636F725AE8}.Debug|Mixed Platforms.Build.0 = Debug|Any CPU
|
||||
{ABD5AA59-6000-4A3D-A54F-4B636F725AE8}.Debug|x86.ActiveCfg = Debug|Any CPU
|
||||
{ABD5AA59-6000-4A3D-A54F-4B636F725AE8}.Debug|x86.Build.0 = Debug|Any CPU
|
||||
{ABD5AA59-6000-4A3D-A54F-4B636F725AE8}.Release|Any CPU.ActiveCfg = Release|Any CPU
|
||||
|
|
|
|||
|
|
@ -82,9 +82,8 @@ namespace Microsoft.AspNetCore.Routing
|
|||
EnsureLoggers(context.HttpContext);
|
||||
|
||||
var requestPath = context.HttpContext.Request.Path;
|
||||
var values = _matcher.Match(requestPath);
|
||||
|
||||
if (values == null)
|
||||
if (!_matcher.TryMatch(requestPath, context.RouteData.Values))
|
||||
{
|
||||
// If we got back a null value set, that means the URI did not match
|
||||
return TaskCache.CompletedTask;
|
||||
|
|
@ -97,11 +96,6 @@ namespace Microsoft.AspNetCore.Routing
|
|||
MergeValues(context.RouteData.DataTokens, DataTokens);
|
||||
}
|
||||
|
||||
if (values.Count > 0)
|
||||
{
|
||||
MergeValues(context.RouteData.Values, values);
|
||||
}
|
||||
|
||||
if (!RouteConstraintMatcher.Match(
|
||||
Constraints,
|
||||
context.RouteData.Values,
|
||||
|
|
|
|||
|
|
@ -63,8 +63,13 @@ namespace Microsoft.AspNetCore.Routing.Template
|
|||
|
||||
public RouteTemplate Template { get; }
|
||||
|
||||
public RouteValueDictionary Match(PathString path)
|
||||
public bool TryMatch(PathString path, RouteValueDictionary values)
|
||||
{
|
||||
if (values == null)
|
||||
{
|
||||
throw new ArgumentNullException(nameof(values));
|
||||
}
|
||||
|
||||
var i = 0;
|
||||
var pathTokenizer = new PathTokenizer(path);
|
||||
|
||||
|
|
@ -84,7 +89,7 @@ namespace Microsoft.AspNetCore.Routing.Template
|
|||
{
|
||||
// If pathSegment is null, then we're out of route segments. All we can match is the empty
|
||||
// string.
|
||||
return null;
|
||||
return false;
|
||||
}
|
||||
else if (routeSegment.IsSimple && routeSegment.Parts[0].IsLiteral)
|
||||
{
|
||||
|
|
@ -92,7 +97,7 @@ namespace Microsoft.AspNetCore.Routing.Template
|
|||
var part = routeSegment.Parts[0];
|
||||
if (!requestSegment.Equals(part.Text, StringComparison.OrdinalIgnoreCase))
|
||||
{
|
||||
return null;
|
||||
return false;
|
||||
}
|
||||
}
|
||||
else if (routeSegment.IsSimple && routeSegment.Parts[0].IsCatchAll)
|
||||
|
|
@ -111,7 +116,7 @@ namespace Microsoft.AspNetCore.Routing.Template
|
|||
!part.IsOptional)
|
||||
{
|
||||
// There's no value for this parameter, the route can't match.
|
||||
return null;
|
||||
return false;
|
||||
}
|
||||
}
|
||||
else
|
||||
|
|
@ -134,14 +139,14 @@ namespace Microsoft.AspNetCore.Routing.Template
|
|||
{
|
||||
// If the segment is a complex segment, it MUST contain literals, and we've parsed the full
|
||||
// path so far, so it can't match.
|
||||
return null;
|
||||
return false;
|
||||
}
|
||||
|
||||
var part = routeSegment.Parts[0];
|
||||
if (part.IsLiteral)
|
||||
{
|
||||
// If the segment is a simple literal - which need the URL to provide a value, so we don't match.
|
||||
return null;
|
||||
return false;
|
||||
}
|
||||
|
||||
if (part.IsCatchAll)
|
||||
|
|
@ -158,12 +163,11 @@ namespace Microsoft.AspNetCore.Routing.Template
|
|||
if (!_hasDefaultValue[i] && !part.IsOptional)
|
||||
{
|
||||
// There's no default for this (non-optional) parameter so it can't match.
|
||||
return null;
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
// At this point we've very likely got a match, so start capturing values for real.
|
||||
var values = new RouteValueDictionary();
|
||||
|
||||
i = 0;
|
||||
foreach (var requestSegment in pathTokenizer)
|
||||
|
|
@ -177,12 +181,12 @@ namespace Microsoft.AspNetCore.Routing.Template
|
|||
var captured = requestSegment.Buffer.Substring(requestSegment.Offset);
|
||||
if (captured.Length > 0)
|
||||
{
|
||||
values.Add(part.Name, captured);
|
||||
values[part.Name] = captured;
|
||||
}
|
||||
else
|
||||
{
|
||||
// It's ok for a catch-all to produce a null value, so we don't check _hasDefaultValue.
|
||||
values.Add(part.Name, _defaultValues[i]);
|
||||
values[part.Name] =_defaultValues[i];
|
||||
}
|
||||
|
||||
// A catch-all has to be the last part, so we're done.
|
||||
|
|
@ -195,13 +199,13 @@ namespace Microsoft.AspNetCore.Routing.Template
|
|||
var part = routeSegment.Parts[0];
|
||||
if (requestSegment.Length > 0)
|
||||
{
|
||||
values.Add(part.Name, requestSegment.ToString());
|
||||
values[part.Name] = requestSegment.ToString();
|
||||
}
|
||||
else
|
||||
{
|
||||
if (_hasDefaultValue[i])
|
||||
{
|
||||
values.Add(part.Name, _defaultValues[i]);
|
||||
values[part.Name] = _defaultValues[i];
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -209,7 +213,7 @@ namespace Microsoft.AspNetCore.Routing.Template
|
|||
{
|
||||
if (!MatchComplexSegment(routeSegment, requestSegment.ToString(), Defaults, values))
|
||||
{
|
||||
return null;
|
||||
return false;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -228,7 +232,12 @@ namespace Microsoft.AspNetCore.Routing.Template
|
|||
// It's ok for a catch-all to produce a null value
|
||||
if (_hasDefaultValue[i] || part.IsCatchAll)
|
||||
{
|
||||
values.Add(part.Name, _defaultValues[i]);
|
||||
// Don't trounce an existing value with a null.
|
||||
var defaultValue = _defaultValues[i];
|
||||
if (defaultValue != null || !values.ContainsKey(part.Name))
|
||||
{
|
||||
values[part.Name] = _defaultValues[i];
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -241,7 +250,7 @@ namespace Microsoft.AspNetCore.Routing.Template
|
|||
}
|
||||
}
|
||||
|
||||
return values;
|
||||
return true;
|
||||
}
|
||||
|
||||
private bool MatchComplexSegment(
|
||||
|
|
|
|||
|
|
@ -166,36 +166,37 @@ namespace Microsoft.AspNetCore.Routing.Tree
|
|||
|
||||
var treeEnumerator = new TreeEnumerator(root, tokenizer);
|
||||
|
||||
// Create a snapshot before processing the route. We'll restore this snapshot before running each
|
||||
// to restore the state. This is likely an "empty" snapshot, which doesn't allocate.
|
||||
var snapshot = context.RouteData.PushState(router: null, values: null, dataTokens: null);
|
||||
|
||||
while (treeEnumerator.MoveNext())
|
||||
{
|
||||
var node = treeEnumerator.Current;
|
||||
foreach (var item in node.Matches)
|
||||
{
|
||||
var values = item.TemplateMatcher.Match(context.HttpContext.Request.Path);
|
||||
if (values == null)
|
||||
if (!item.TemplateMatcher.TryMatch(context.HttpContext.Request.Path, context.RouteData.Values))
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
var match = new TemplateMatch(item, values);
|
||||
var snapshot = context.RouteData.PushState(match.Entry.Target, match.Values, dataTokens: null);
|
||||
|
||||
try
|
||||
{
|
||||
if (!RouteConstraintMatcher.Match(
|
||||
match.Entry.Constraints,
|
||||
context.RouteData.Values,
|
||||
context.HttpContext,
|
||||
this,
|
||||
RouteDirection.IncomingRequest,
|
||||
_constraintLogger))
|
||||
item.Constraints,
|
||||
context.RouteData.Values,
|
||||
context.HttpContext,
|
||||
this,
|
||||
RouteDirection.IncomingRequest,
|
||||
_constraintLogger))
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
_logger.MatchedRoute(match.Entry.RouteName, match.Entry.RouteTemplate.TemplateText);
|
||||
_logger.MatchedRoute(item.RouteName, item.RouteTemplate.TemplateText);
|
||||
|
||||
await match.Entry.Target.RouteAsync(context);
|
||||
context.RouteData.Routers.Add(item.Target);
|
||||
await item.Target.RouteAsync(context);
|
||||
if (context.Handler != null)
|
||||
{
|
||||
return;
|
||||
|
|
@ -315,54 +316,6 @@ namespace Microsoft.AspNetCore.Routing.Tree
|
|||
}
|
||||
}
|
||||
|
||||
private struct TemplateMatch : IEquatable<TemplateMatch>
|
||||
{
|
||||
public TemplateMatch(TreeRouteMatchingEntry entry, RouteValueDictionary values)
|
||||
{
|
||||
Entry = entry;
|
||||
Values = values;
|
||||
}
|
||||
|
||||
public TreeRouteMatchingEntry Entry { get; }
|
||||
|
||||
public RouteValueDictionary Values { get; }
|
||||
|
||||
public override bool Equals(object obj)
|
||||
{
|
||||
if (obj is TemplateMatch)
|
||||
{
|
||||
return Equals((TemplateMatch)obj);
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
public bool Equals(TemplateMatch other)
|
||||
{
|
||||
return
|
||||
object.ReferenceEquals(Entry, other.Entry) &&
|
||||
object.ReferenceEquals(Values, other.Values);
|
||||
}
|
||||
|
||||
public override int GetHashCode()
|
||||
{
|
||||
var hash = new HashCodeCombiner();
|
||||
hash.Add(Entry);
|
||||
hash.Add(Values);
|
||||
return hash.CombinedHash;
|
||||
}
|
||||
|
||||
public static bool operator ==(TemplateMatch left, TemplateMatch right)
|
||||
{
|
||||
return left.Equals(right);
|
||||
}
|
||||
|
||||
public static bool operator !=(TemplateMatch left, TemplateMatch right)
|
||||
{
|
||||
return !left.Equals(right);
|
||||
}
|
||||
}
|
||||
|
||||
private VirtualPathData GetVirtualPathForNamedRoute(VirtualPathContext context)
|
||||
{
|
||||
TreeRouteLinkGenerationEntry entry;
|
||||
|
|
|
|||
File diff suppressed because it is too large
Load Diff
|
|
@ -1533,7 +1533,6 @@ namespace Microsoft.AspNetCore.Routing.Tree
|
|||
// Assert
|
||||
Assert.NotEqual(nestedValues, context.RouteData.Values);
|
||||
|
||||
// The new routedata is a copy
|
||||
Assert.Equal("Index", context.RouteData.Values["action"]);
|
||||
Assert.Equal("Index", nestedValues["action"]);
|
||||
Assert.DoesNotContain(context.RouteData.Values, kvp => kvp.Key == "test_route_group");
|
||||
|
|
|
|||
Loading…
Reference in New Issue