Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve DFA matcher builder startup perf with many constraints #46323

Merged
merged 7 commits into from
Jan 31, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using BenchmarkDotNet.Attributes;

namespace Microsoft.AspNetCore.Routing.Matching;

public class CreateMatcherRegexConstraintBenchmark : EndpointRoutingBenchmarkBase
{
[Params(true, false)]
public bool RegexSame { get; set; }

private const int EndpointCount = 1_000;

[GlobalSetup]
public void Setup()
{
Endpoints = new RouteEndpoint[EndpointCount];
for (var i = 0; i < Endpoints.Length; i++)
{
Endpoints[i] = RegexSame
? CreateEndpoint("/plaintext" + i + "/{param:regex(^\\d{{7}}|(SI[[PG]]|JPA|DEM)\\d{{4}})}")
: CreateEndpoint("/plaintext" + i + "/{param:regex(^" + i + "\\d{{7}}|(SI[[PG]]|JPA|DEM)\\d{{4}})}");
}

}

[Benchmark]
public void Build()
{
var builder = CreateDfaMatcherBuilder();
for (var i = 0; i < Endpoints.Length; i++)
{
builder.AddEndpoint(Endpoints[i]);
}

builder.Build();
}
}
3 changes: 2 additions & 1 deletion src/Http/Routing/src/Constraints/AlphaRouteConstraint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Text.RegularExpressions;
using Microsoft.AspNetCore.Routing.Matching;

namespace Microsoft.AspNetCore.Routing.Constraints;

/// <summary>
/// Constrains a route parameter to contain only lowercase or uppercase letters A through Z in the English alphabet.
/// </summary>
public partial class AlphaRouteConstraint : RegexRouteConstraint
public partial class AlphaRouteConstraint : RegexRouteConstraint, ICachableParameterPolicy
{
/// <summary>
/// Initializes a new instance of the <see cref="AlphaRouteConstraint" /> class.
Expand Down
2 changes: 1 addition & 1 deletion src/Http/Routing/src/Constraints/BoolRouteConstraint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Routing.Constraints;
/// <summary>
/// Constrains a route parameter to represent only Boolean values.
/// </summary>
public class BoolRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy
public class BoolRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy, ICachableParameterPolicy
{
/// <inheritdoc />
public bool Match(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ namespace Microsoft.AspNetCore.Routing.Constraints;
/// For a sample on how to list all formats which are considered, please visit
/// http://msdn.microsoft.com/en-us/library/aszyst2c(v=vs.110).aspx
/// </remarks>
public class DateTimeRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy
public class DateTimeRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy, ICachableParameterPolicy
{
/// <inheritdoc />
public bool Match(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Routing.Constraints;
/// <summary>
/// Constrains a route parameter to represent only decimal values.
/// </summary>
public class DecimalRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy
public class DecimalRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy, ICachableParameterPolicy
{
/// <inheritdoc />
public bool Match(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Routing.Constraints;
/// <summary>
/// Constrains a route parameter to represent only 64-bit floating-point values.
/// </summary>
public class DoubleRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy
public class DoubleRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy, ICachableParameterPolicy
{
/// <inheritdoc />
public bool Match(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ namespace Microsoft.AspNetCore.Routing.Constraints;
/// </list>
/// </para>
/// </remarks>
public class FileNameRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy
public class FileNameRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy, ICachableParameterPolicy
{
/// <inheritdoc />
public bool Match(
Expand Down
2 changes: 1 addition & 1 deletion src/Http/Routing/src/Constraints/FloatRouteConstraint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Routing.Constraints;
/// <summary>
/// Constrains a route parameter to represent only 32-bit floating-point values.
/// </summary>
public class FloatRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy
public class FloatRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy, ICachableParameterPolicy
{
/// <inheritdoc />
public bool Match(
Expand Down
2 changes: 1 addition & 1 deletion src/Http/Routing/src/Constraints/GuidRouteConstraint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace Microsoft.AspNetCore.Routing.Constraints;
/// Matches values specified in any of the five formats "N", "D", "B", "P", or "X",
/// supported by Guid.ToString(string) and Guid.ToString(String, IFormatProvider) methods.
/// </summary>
public class GuidRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy
public class GuidRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy, ICachableParameterPolicy
{
/// <inheritdoc />
public bool Match(
Expand Down
2 changes: 1 addition & 1 deletion src/Http/Routing/src/Constraints/IntRouteConstraint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Routing.Constraints;
/// <summary>
/// Constrains a route parameter to represent only 32-bit integer values.
/// </summary>
public class IntRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy
public class IntRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy, ICachableParameterPolicy
{
/// <inheritdoc />
public bool Match(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Routing.Constraints;
/// <summary>
/// Constrains a route parameter to be a string of a given length or within a given range of lengths.
/// </summary>
public class LengthRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy
public class LengthRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy, ICachableParameterPolicy
{
/// <summary>
/// Initializes a new instance of the <see cref="LengthRouteConstraint" /> class that constrains
Expand Down
2 changes: 1 addition & 1 deletion src/Http/Routing/src/Constraints/LongRouteConstraint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Routing.Constraints;
/// <summary>
/// Constrains a route parameter to represent only 64-bit integer values.
/// </summary>
public class LongRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy
public class LongRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy, ICachableParameterPolicy
{
/// <inheritdoc />
public bool Match(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Routing.Constraints;
/// <summary>
/// Constrains a route parameter to be a string with a maximum length.
/// </summary>
public class MaxLengthRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy
public class MaxLengthRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy, ICachableParameterPolicy
{
/// <summary>
/// Initializes a new instance of the <see cref="MaxLengthRouteConstraint" /> class.
Expand Down
2 changes: 1 addition & 1 deletion src/Http/Routing/src/Constraints/MaxRouteConstraint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Routing.Constraints;
/// <summary>
/// Constrains a route parameter to be an integer with a maximum value.
/// </summary>
public class MaxRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy
public class MaxRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy, ICachableParameterPolicy
{
/// <summary>
/// Initializes a new instance of the <see cref="MaxRouteConstraint" /> class.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Routing.Constraints;
/// <summary>
/// Constrains a route parameter to be a string with a minimum length.
/// </summary>
public class MinLengthRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy
public class MinLengthRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy, ICachableParameterPolicy
{
/// <summary>
/// Initializes a new instance of the <see cref="MinLengthRouteConstraint" /> class.
Expand Down
2 changes: 1 addition & 1 deletion src/Http/Routing/src/Constraints/MinRouteConstraint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Routing.Constraints;
/// <summary>
/// Constrains a route parameter to be a long with a minimum value.
/// </summary>
public class MinRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy
public class MinRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy, ICachableParameterPolicy
{
/// <summary>
/// Initializes a new instance of the <see cref="MinRouteConstraint" /> class.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ namespace Microsoft.AspNetCore.Routing.Constraints;
/// </list>
/// </para>
/// </remarks>
public class NonFileNameRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy
public class NonFileNameRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy, ICachableParameterPolicy
{
/// <inheritdoc />
public bool Match(
Expand Down
2 changes: 1 addition & 1 deletion src/Http/Routing/src/Constraints/RangeRouteConstraint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Routing.Constraints;
/// <summary>
/// Constraints a route parameter to be an integer within a given range of values.
/// </summary>
public class RangeRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy
public class RangeRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy, ICachableParameterPolicy
{
/// <summary>
/// Initializes a new instance of the <see cref="RangeRouteConstraint" /> class.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@

using System.Diagnostics.CodeAnalysis;
using System.Text.RegularExpressions;
using Microsoft.AspNetCore.Routing.Matching;

namespace Microsoft.AspNetCore.Routing.Constraints;

/// <summary>
/// Represents a regex constraint which can be used as an inlineConstraint.
/// </summary>
public class RegexInlineRouteConstraint : RegexRouteConstraint
public class RegexInlineRouteConstraint : RegexRouteConstraint, ICachableParameterPolicy
{
/// <summary>
/// Initializes a new instance of the <see cref="RegexInlineRouteConstraint" /> class.
Expand Down
25 changes: 19 additions & 6 deletions src/Http/Routing/src/Constraints/RegexRouteConstraint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ namespace Microsoft.AspNetCore.Routing.Constraints;
public class RegexRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy
{
private static readonly TimeSpan RegexMatchTimeout = TimeSpan.FromSeconds(10);
private readonly string _regexPattern;
private Regex? _constraint;

/// <summary>
/// Constructor for a <see cref="RegexRouteConstraint"/> given a <paramref name="regex"/>.
Expand All @@ -24,7 +26,8 @@ public RegexRouteConstraint(Regex regex)
{
ArgumentNullException.ThrowIfNull(regex);

Constraint = regex;
_constraint = regex;
_regexPattern = regex.ToString();
}

/// <summary>
Expand All @@ -37,16 +40,26 @@ public RegexRouteConstraint(
{
ArgumentNullException.ThrowIfNull(regexPattern);

Constraint = new Regex(
regexPattern,
RegexOptions.CultureInvariant | RegexOptions.Compiled | RegexOptions.IgnoreCase,
RegexMatchTimeout);
_regexPattern = regexPattern;
}

/// <summary>
/// Gets the regular expression used in the route constraint.
/// </summary>
public Regex Constraint { get; private set; }
public Regex Constraint
{
get
{
// Create regex instance lazily to avoid compiling regexes at app startup. Delay creation until constraint is first evaluation.
// This is not thread safe. No side effect but multiple instances of a regex instance could be created from a burst of requests.
_constraint ??= new Regex(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess if a constraint is evaluated once, it's likely to be hit an arbitrary number of times over the life of the app?

If that wasn't the case one could imagine lazily instantiating as interpreted first, then counting until a threshold and swapping for compiled.

Copy link
Member

@davidfowl davidfowl Jan 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is similar to what DI does, but it could be something we do after this change. We would need to make sure this happened in a place that could properly measure the "hit count". I'm not sure where that is.

_regexPattern,
RegexOptions.CultureInvariant | RegexOptions.Compiled | RegexOptions.IgnoreCase,
RegexMatchTimeout);

return _constraint;
}
}

/// <inheritdoc />
public bool Match(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Routing.Constraints;
/// <summary>
/// Constrains a route parameter to contain only a specified string.
/// </summary>
public class StringRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy
public class StringRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy, ICachableParameterPolicy
{
private readonly string _value;

Expand Down
44 changes: 43 additions & 1 deletion src/Http/Routing/src/Matching/DfaMatcherBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Diagnostics;
using System.Linq;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Routing.Constraints;
using Microsoft.AspNetCore.Routing.Patterns;
using Microsoft.AspNetCore.Routing.Template;
using Microsoft.Extensions.Logging;
Expand Down Expand Up @@ -39,7 +40,8 @@ public DfaMatcherBuilder(
IEnumerable<MatcherPolicy> policies)
{
_loggerFactory = loggerFactory;
_parameterPolicyFactory = parameterPolicyFactory;
// DfaMatcherBuilder is a transient service. Each instance has its own cache of parameter policies.
_parameterPolicyFactory = new CachingParameterPolicyFactory(parameterPolicyFactory);
_selector = selector;

var (nodeBuilderPolicies, endpointComparerPolicies, endpointSelectorPolicies) = ExtractPolicies(policies.OrderBy(p => p.Order));
Expand Down Expand Up @@ -111,6 +113,46 @@ public DfaNode BuildDfaTree(bool includeLabel = false)
return root;
}

private sealed class CachingParameterPolicyFactory : ParameterPolicyFactory
{
private readonly ParameterPolicyFactory _inner;
private readonly Dictionary<string, IParameterPolicy> _cachedParameters;

public CachingParameterPolicyFactory(ParameterPolicyFactory inner)
{
_inner = inner;
_cachedParameters = new Dictionary<string, IParameterPolicy>();
JamesNK marked this conversation as resolved.
Show resolved Hide resolved
}

public override IParameterPolicy Create(RoutePatternParameterPart parameter, string inlineText)
{
// Blindly check the cache to see if it contains a match.
// Only cachable parameter policies are in the cache, so a match will only be available if the parameter policy key is configured to a cachable parameter policy.
if (_cachedParameters.TryGetValue(inlineText, out var parameterPolicy))
{
return _inner.Create(parameter, parameterPolicy);
}

parameterPolicy = _inner.Create(parameter, inlineText);

// The created parameter policy can be wrapped in an OptionalRouteConstraint if RoutePatternParameterPart.IsOptional is true.
var createdParameterPolicy = (parameterPolicy is OptionalRouteConstraint optionalRouteConstraint)
? optionalRouteConstraint.InnerConstraint
: parameterPolicy;
if (createdParameterPolicy is ICachableParameterPolicy)
{
_cachedParameters[inlineText] = createdParameterPolicy;
}

return parameterPolicy;
}

public override IParameterPolicy Create(RoutePatternParameterPart parameter, IParameterPolicy parameterPolicy)
{
return _inner.Create(parameter, parameterPolicy);
}
}

private sealed class DfaBuilderWorker
{
private List<DfaBuilderWorkerWorkItem> _previousWork;
Expand Down
11 changes: 11 additions & 0 deletions src/Http/Routing/src/Matching/ICachableParameterPolicy.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Microsoft.AspNetCore.Routing.Matching;

/// <summary>
/// A marker interface for parameter policies that can be shared.
/// </summary>
internal interface ICachableParameterPolicy
{
}
Loading