A new and exciting implementation of action selection

This is an improved implementation of the ActionSelector for conventional
routing. This will do fewer dictionary lookups than the decision tree, and
will avoid OrdinalIgnoreCase hashing in the common case.
This commit is contained in:
Ryan Nowak 2016-05-20 09:54:41 -07:00
parent 87bb1d0ff5
commit e2cb8e8ac8
9 changed files with 340 additions and 224 deletions

View File

@ -145,9 +145,6 @@ namespace Microsoft.Extensions.DependencyInjection
services.TryAddSingleton<IActionSelector, ActionSelector>();
services.TryAddSingleton<ActionConstraintCache>();
// Performs caching
services.TryAddSingleton<IActionSelectorDecisionTreeProvider, ActionSelectorDecisionTreeProvider>();
// Will be cached by the DefaultActionSelector
services.TryAddEnumerable(
ServiceDescriptor.Transient<IActionConstraintProvider, DefaultActionConstraintProvider>());

View File

@ -1,98 +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;
using System.Collections.Generic;
using System.Linq;
using Microsoft.AspNetCore.Mvc.Abstractions;
using Microsoft.AspNetCore.Mvc.Infrastructure;
using Microsoft.AspNetCore.Routing;
using Microsoft.AspNetCore.Routing.DecisionTree;
namespace Microsoft.AspNetCore.Mvc.Internal
{
/// <inheritdoc />
public class ActionSelectionDecisionTree : IActionSelectionDecisionTree
{
private readonly DecisionTreeNode<ActionDescriptor> _root;
/// <summary>
/// Creates a new <see cref="ActionSelectionDecisionTree"/>.
/// </summary>
/// <param name="actions">The <see cref="ActionDescriptorCollection"/>.</param>
public ActionSelectionDecisionTree(ActionDescriptorCollection actions)
{
Version = actions.Version;
var conventionalRoutedActions = actions.Items.Where(a => a.AttributeRouteInfo?.Template == null).ToArray();
_root = DecisionTreeBuilder<ActionDescriptor>.GenerateTree(
conventionalRoutedActions,
new ActionDescriptorClassifier());
}
/// <inheritdoc />
public int Version { get; private set; }
/// <inheritdoc />
public IReadOnlyList<ActionDescriptor> Select(IDictionary<string, object> routeValues)
{
var results = new List<ActionDescriptor>();
Walk(results, routeValues, _root);
return results;
}
private void Walk(
List<ActionDescriptor> results,
IDictionary<string, object> routeValues,
DecisionTreeNode<ActionDescriptor> node)
{
for (var i = 0; i < node.Matches.Count; i++)
{
results.Add(node.Matches[i]);
}
for (var i = 0; i < node.Criteria.Count; i++)
{
var criterion = node.Criteria[i];
var key = criterion.Key;
object value;
routeValues.TryGetValue(key, out value);
DecisionTreeNode<ActionDescriptor> branch;
if (criterion.Branches.TryGetValue(value ?? string.Empty, out branch))
{
Walk(results, routeValues, branch);
}
}
}
private class ActionDescriptorClassifier : IClassifier<ActionDescriptor>
{
public ActionDescriptorClassifier()
{
ValueComparer = new RouteValueEqualityComparer();
}
public IEqualityComparer<object> ValueComparer { get; private set; }
public IDictionary<string, DecisionCriterionValue> GetCriteria(ActionDescriptor item)
{
var results = new Dictionary<string, DecisionCriterionValue>(StringComparer.OrdinalIgnoreCase);
if (item.RouteValues != null)
{
foreach (var kvp in item.RouteValues)
{
// null and string.Empty are equivalent for route values, so just treat nulls as
// string.Empty.
results.Add(kvp.Key, new DecisionCriterionValue(kvp.Value ?? string.Empty));
}
}
return results;
}
}
}
}

View File

@ -3,12 +3,15 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Threading;
using Microsoft.AspNetCore.Mvc.Abstractions;
using Microsoft.AspNetCore.Mvc.ActionConstraints;
using Microsoft.AspNetCore.Mvc.Core;
using Microsoft.AspNetCore.Mvc.Infrastructure;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.Internal;
using Microsoft.Extensions.Logging;
namespace Microsoft.AspNetCore.Mvc.Internal
@ -18,27 +21,51 @@ namespace Microsoft.AspNetCore.Mvc.Internal
/// </summary>
public class ActionSelector : IActionSelector
{
private readonly IActionSelectorDecisionTreeProvider _decisionTreeProvider;
private static readonly IReadOnlyList<ActionDescriptor> EmptyActions = Array.Empty<ActionDescriptor>();
private readonly IActionDescriptorCollectionProvider _actionDescriptorCollectionProvider;
private readonly ActionConstraintCache _actionConstraintCache;
private ILogger _logger;
private readonly ILogger _logger;
private Cache _cache;
/// <summary>
/// Creates a new <see cref="ActionSelector"/>.
/// </summary>
/// <param name="decisionTreeProvider">The <see cref="IActionSelectorDecisionTreeProvider"/>.</param>
/// <param name="actionDescriptorCollectionProvider">
/// The <see cref="IActionDescriptorCollectionProvider"/>.
/// </param>
/// <param name="actionConstraintCache">The <see cref="ActionConstraintCache"/> that
/// providers a set of <see cref="IActionConstraint"/> instances.</param>
/// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
public ActionSelector(
IActionSelectorDecisionTreeProvider decisionTreeProvider,
IActionDescriptorCollectionProvider actionDescriptorCollectionProvider,
ActionConstraintCache actionConstraintCache,
ILoggerFactory loggerFactory)
{
_decisionTreeProvider = decisionTreeProvider;
_actionDescriptorCollectionProvider = actionDescriptorCollectionProvider;
_logger = loggerFactory.CreateLogger<ActionSelector>();
_actionConstraintCache = actionConstraintCache;
}
private Cache Current
{
get
{
var actions = _actionDescriptorCollectionProvider.ActionDescriptors;
var cache = Volatile.Read(ref _cache);
if (cache != null && cache.Version == actions.Version)
{
return cache;
}
cache = new Cache(actions);
Volatile.Write(ref _cache, cache);
return cache;
}
}
public IReadOnlyList<ActionDescriptor> SelectCandidates(RouteContext context)
{
if (context == null)
@ -46,8 +73,31 @@ namespace Microsoft.AspNetCore.Mvc.Internal
throw new ArgumentNullException(nameof(context));
}
var tree = _decisionTreeProvider.DecisionTree;
return tree.Select(context.RouteData.Values);
var cache = Current;
// The Cache works based on a string[] of the route values in a pre-calculated order. This code extracts
// those values in the correct order.
var keys = cache.RouteKeys;
var values = new string[keys.Length];
for (var i = 0; i < keys.Length; i++)
{
context.RouteData.Values.TryGetValue(keys[i], out object value);
if (value != null)
{
values[i] = value as string ?? Convert.ToString(value);
}
}
if (cache.OrdinalEntries.TryGetValue(values, out var matchingRouteValues) ||
cache.OrdinalIgnoreCaseEntries.TryGetValue(values, out matchingRouteValues))
{
Debug.Assert(matchingRouteValues != null);
return matchingRouteValues;
}
_logger.NoActionsMatched(context.RouteData.Values);
return EmptyActions;
}
public ActionDescriptor SelectBestCandidate(RouteContext context, IReadOnlyList<ActionDescriptor> candidates)
@ -159,14 +209,14 @@ namespace Microsoft.AspNetCore.Mvc.Internal
}
}
// If we don't find a 'next' then there's nothing left to do.
// If we don't find a next then there's nothing left to do.
if (order == null)
{
return candidates;
}
// Since we have a constraint to process, bisect the set of actions into those with and without a
// constraint for the 'current order'.
// constraint for the current order.
var actionsWithConstraint = new List<ActionSelectorCandidate>();
var actionsWithoutConstraint = new List<ActionSelectorCandidate>();
@ -214,7 +264,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
}
}
// If we have matches with constraints, those are 'better' so try to keep processing those
// If we have matches with constraints, those are better so try to keep processing those
if (actionsWithConstraint.Count > 0)
{
var matches = EvaluateActionConstraintsCore(context, actionsWithConstraint, order);
@ -234,5 +284,168 @@ namespace Microsoft.AspNetCore.Mvc.Internal
return EvaluateActionConstraintsCore(context, actionsWithoutConstraint, order);
}
}
// The action selector cache stores a mapping of route-values -> action descriptors for each known set of
// of route-values. We actually build two of these mappings, one for case-sensitive (fast path) and one for
// case-insensitive (slow path).
//
// This is necessary because MVC routing/action-selection is always case-insensitive. So we're going to build
// a case-sensitive dictionary that will behave like the a case-insensitive dictionary when you hit one of the
// canonical entries. When you don't hit a case-sensitive match it will try the case-insensitive dictionary
// so you still get correct behaviors.
//
// The difference here is because while MVC is case-insensitive, doing a case-sensitive comparison is much
// faster. We also expect that most of the URLs we process are canonically-cased because they were generated
// by Url.Action or another routing api.
//
// This means that for a set of actions like:
// { controller = "Home", action = "Index" } -> HomeController::Index1()
// { controller = "Home", action = "index" } -> HomeController::Index2()
//
// Both of these actions match "Index" case-insensitively, but there exist two known canonical casings,
// so we will create an entry for "Index" and an entry for "index". Both of these entries match **both**
// actions.
private class Cache
{
public Cache(ActionDescriptorCollection actions)
{
// We need to store the version so the cache can be invalidated if the actions change.
Version = actions.Version;
// We need to build two maps for all of the route values.
OrdinalEntries = new Dictionary<string[], List<ActionDescriptor>>(StringArrayComparer.Ordinal);
OrdinalIgnoreCaseEntries = new Dictionary<string[], List<ActionDescriptor>>(StringArrayComparer.OrdinalIgnoreCase);
// We need to first identify of the keys that action selection will look at (in route data).
// We want to only consider conventionally routed actions here.
var routeKeys = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
for (var i = 0; i < actions.Items.Count; i++)
{
var action = actions.Items[i];
if (action.AttributeRouteInfo == null)
{
// This is a conventionally routed action - so make sure we include its keys in the set of
// known route value keys.
foreach (var kvp in action.RouteValues)
{
routeKeys.Add(kvp.Key);
}
}
}
// We need to hold on to an ordered set of keys for the route values. We'll use these later to
// extract the set of route values from an incoming request to compare against our maps of known
// route values.
RouteKeys = routeKeys.ToArray();
for (var i = 0; i < actions.Items.Count; i++)
{
var action = actions.Items[i];
if (action.AttributeRouteInfo != null)
{
// This only handles conventional routing. Ignore attribute routed actions.
continue;
}
// This is a conventionally routed action - so we need to extract the route values associated
// with this action (in order) so we can store them in our dictionaries.
var routeValues = new string[RouteKeys.Length];
for (var j = 0; j < RouteKeys.Length; j++)
{
action.RouteValues.TryGetValue(RouteKeys[j], out routeValues[j]);
}
if (!OrdinalIgnoreCaseEntries.TryGetValue(routeValues, out var entries))
{
entries = new List<ActionDescriptor>();
OrdinalIgnoreCaseEntries.Add(routeValues, entries);
}
entries.Add(action);
// We also want to add the same (as in reference equality) list of actions to the ordinal entries.
// We'll keep updating `entries` to include all of the actions in the same equivalence class -
// meaning, all conventionally routed actions for which the route values are equalignoring case.
//
// `entries` will appear in `OrdinalIgnoreCaseEntries` exactly once and in `OrdinalEntries` once
// for each variation of casing that we've seen.
if (!OrdinalEntries.ContainsKey(routeValues))
{
OrdinalEntries.Add(routeValues, entries);
}
}
}
public int Version { get; set; }
public string[] RouteKeys { get; set; }
public Dictionary<string[], List<ActionDescriptor>> OrdinalEntries { get; set; }
public Dictionary<string[], List<ActionDescriptor>> OrdinalIgnoreCaseEntries { get; set; }
}
private class StringArrayComparer : IEqualityComparer<string[]>
{
public static readonly StringArrayComparer Ordinal = new StringArrayComparer(StringComparer.Ordinal);
public static readonly StringArrayComparer OrdinalIgnoreCase = new StringArrayComparer(StringComparer.OrdinalIgnoreCase);
private readonly StringComparer _valueComparer;
private StringArrayComparer(StringComparer valueComparer)
{
_valueComparer = valueComparer;
}
public bool Equals(string[] x, string[] y)
{
if (object.ReferenceEquals(x, y))
{
return true;
}
if (x == null ^ y == null)
{
return false;
}
if (x.Length != y.Length)
{
return false;
}
for (var i = 0; i < x.Length; i++)
{
if (string.IsNullOrEmpty(x[i]) && string.IsNullOrEmpty(y[i]))
{
continue;
}
if (!_valueComparer.Equals(x[i], y[i]))
{
return false;
}
}
return true;
}
public int GetHashCode(string[] obj)
{
if (obj == null)
{
return 0;
}
var hash = new HashCodeCombiner();
for (var i = 0; i < obj.Length; i++)
{
hash.Add(obj[i], _valueComparer);
}
return hash.CombinedHash;
}
}
}
}

View File

@ -1,51 +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;
using Microsoft.AspNetCore.Mvc.Core;
using Microsoft.AspNetCore.Mvc.Infrastructure;
namespace Microsoft.AspNetCore.Mvc.Internal
{
/// <inheritdoc />
public class ActionSelectorDecisionTreeProvider : IActionSelectorDecisionTreeProvider
{
private readonly IActionDescriptorCollectionProvider _actionDescriptorCollectionProvider;
private ActionSelectionDecisionTree _decisionTree;
/// <summary>
/// Creates a new <see cref="ActionSelectorDecisionTreeProvider"/>.
/// </summary>
/// <param name="actionDescriptorCollectionProvider">
/// The <see cref="IActionDescriptorCollectionProvider"/>.
/// </param>
public ActionSelectorDecisionTreeProvider(
IActionDescriptorCollectionProvider actionDescriptorCollectionProvider)
{
_actionDescriptorCollectionProvider = actionDescriptorCollectionProvider;
}
/// <inheritdoc />
public IActionSelectionDecisionTree DecisionTree
{
get
{
var descriptors = _actionDescriptorCollectionProvider.ActionDescriptors;
if (descriptors == null)
{
throw new InvalidOperationException(
Resources.FormatPropertyOfTypeCannotBeNull(
"ActionDescriptors",
_actionDescriptorCollectionProvider.GetType()));
}
if (_decisionTree == null || descriptors.Version != _decisionTree.Version)
{
_decisionTree = new ActionSelectionDecisionTree(descriptors);
}
return _decisionTree;
}
}
}
}

View File

@ -1,20 +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.
namespace Microsoft.AspNetCore.Mvc.Internal
{
/// <summary>
/// Stores an <see cref="ActionSelectionDecisionTree"/> for the current value of
/// <see cref="Infrastructure.IActionDescriptorCollectionProvider.ActionDescriptors"/>.
/// </summary>
public interface IActionSelectorDecisionTreeProvider
{
/// <summary>
/// Gets the <see cref="Infrastructure.IActionDescriptorCollectionProvider"/>.
/// </summary>
IActionSelectionDecisionTree DecisionTree
{
get;
}
}
}

View File

@ -27,7 +27,6 @@ Microsoft.AspNetCore.Mvc.RouteAttribute</Description>
<PackageReference Include="Microsoft.AspNetCore.Http" Version="$(AspNetCoreVersion)" />
<PackageReference Include="Microsoft.AspNetCore.ResponseCaching.Abstractions" Version="$(AspNetCoreVersion)" />
<PackageReference Include="Microsoft.AspNetCore.Routing" Version="$(AspNetCoreVersion)" />
<PackageReference Include="Microsoft.AspNetCore.Routing.DecisionTree.Sources" Version="$(AspNetCoreVersion)" PrivateAssets="All" />
<PackageReference Include="Microsoft.Extensions.ClosedGenericMatcher.Sources" Version="$(AspNetCoreVersion)" PrivateAssets="All" />
<PackageReference Include="Microsoft.Extensions.DependencyModel" Version="$(DependencyModelVersion)" />
<PackageReference Include="Microsoft.Extensions.FileProviders.Abstractions" Version="$(AspNetCoreVersion)" />

View File

@ -1,29 +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;
using Microsoft.AspNetCore.Mvc.Abstractions;
namespace Microsoft.AspNetCore.Mvc.Internal
{
/// <summary>
/// A data structure that retrieves a list of <see cref="ActionDescriptor"/> matches based on the values
/// supplied for the current request by <see cref="Microsoft.AspNetCore.Routing.RouteData.Values"/>.
/// </summary>
public interface IActionSelectionDecisionTree
{
/// <summary>
/// Gets the version. The same as the value of
/// <see cref="Infrastructure.ActionDescriptorCollection.Version"/>.
/// </summary>
int Version { get; }
/// <summary>
/// Retrieves a set of <see cref="ActionDescriptor"/> based on the route values supplied by
/// <paramref name="routeValues"/>/
/// </summary>
/// <param name="routeValues">The route values for the current request.</param>
/// <returns>A set of <see cref="ActionDescriptor"/> matching the route values.</returns>
IReadOnlyList<ActionDescriptor> Select(IDictionary<string, object> routeValues);
}
}

View File

@ -22,9 +22,7 @@ using Xunit;
namespace Microsoft.AspNetCore.Mvc.Infrastructure
{
// Most of the in-depth testing for SelectCandidates is part of the descision tree tests.
// This is just basic coverage of the API in common scenarios.
public class DefaultActionSelectorTests
public class ActionSelectorTest
{
[Fact]
public void SelectCandidates_SingleMatch()
@ -173,6 +171,110 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure
Assert.Empty(candidates);
}
// In this context `CaseSensitiveMatch` means that the input route values exactly match one of the action
// descriptor's route values in terms of casing. This is important because we optimize for this case
// in the implementation.
[Fact]
public void SelectCandidates_Match_CaseSensitiveMatch_IncludesAllCaseInsensitiveMatches()
{
var actions = new ActionDescriptor[]
{
new ActionDescriptor()
{
DisplayName = "A1",
RouteValues = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
{
{ "controller", "Home" },
{ "action", "Index" }
},
},
new ActionDescriptor()
{
DisplayName = "A2",
RouteValues = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
{
{ "controller", "home" },
{ "action", "Index" }
},
},
new ActionDescriptor() // This won't match the request
{
DisplayName = "A3",
RouteValues = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
{
{ "controller", "Home" },
{ "action", "About" }
},
},
};
var expected = actions.Take(2).ToArray();
var selector = CreateSelector(actions);
var routeContext = CreateRouteContext("GET");
routeContext.RouteData.Values.Add("controller", "Home");
routeContext.RouteData.Values.Add("action", "Index");
// Act
var candidates = selector.SelectCandidates(routeContext);
// Assert
Assert.Equal(expected, candidates);
}
// In this context `CaseInsensitiveMatch` means that the input route values do not match any action
// descriptor's route values in terms of casing. This is important because we optimize for the case
// where the casing matches - the non-matching-casing path is handled a bit differently.
[Fact]
public void SelectCandidates_Match_CaseInsensitiveMatch_IncludesAllCaseInsensitiveMatches()
{
var actions = new ActionDescriptor[]
{
new ActionDescriptor()
{
DisplayName = "A1",
RouteValues = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
{
{ "controller", "Home" },
{ "action", "Index" }
},
},
new ActionDescriptor()
{
DisplayName = "A2",
RouteValues = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
{
{ "controller", "home" },
{ "action", "Index" }
},
},
new ActionDescriptor() // This won't match the request
{
DisplayName = "A3",
RouteValues = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
{
{ "controller", "Home" },
{ "action", "About" }
},
},
};
var expected = actions.Take(2).ToArray();
var selector = CreateSelector(actions);
var routeContext = CreateRouteContext("GET");
routeContext.RouteData.Values.Add("controller", "HOME");
routeContext.RouteData.Values.Add("action", "iNDex");
// Act
var candidates = selector.SelectCandidates(routeContext);
// Assert
Assert.Equal(expected, candidates);
}
[Fact]
public void SelectBestCandidate_AmbiguousActions_LogIsCorrect()
{
@ -661,15 +763,14 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure
var actionDescriptorCollectionProvider = new ActionDescriptorCollectionProvider(
new[] { actionDescriptorProvider },
Enumerable.Empty<IActionDescriptorChangeProvider>());
var decisionTreeProvider = new ActionSelectorDecisionTreeProvider(actionDescriptorCollectionProvider);
var actionConstraintProviders = new[]
{
new DefaultActionConstraintProvider(),
};
var actionSelector = new ActionSelector(
decisionTreeProvider,
actionDescriptorCollectionProvider,
GetActionConstraintCache(actionConstraintProviders),
NullLoggerFactory.Instance);
@ -679,7 +780,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure
private ControllerActionDescriptorProvider GetActionDescriptorProvider()
{
var controllerTypes = typeof(DefaultActionSelectorTests)
var controllerTypes = typeof(ActionSelectorTest)
.GetNestedTypes(BindingFlags.NonPublic)
.Select(t => t.GetTypeInfo())
.ToList();
@ -753,9 +854,8 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure
var actionProvider = new Mock<IActionDescriptorCollectionProvider>(MockBehavior.Strict);
actionProvider
.Setup(p => p.ActionDescriptors).Returns(new ActionDescriptorCollection(actions, 0));
var decisionTreeProvider = new ActionSelectorDecisionTreeProvider(actionProvider.Object);
.Setup(p => p.ActionDescriptors)
.Returns(new ActionDescriptorCollection(actions, 0));
var actionConstraintProviders = new IActionConstraintProvider[] {
new DefaultActionConstraintProvider(),
@ -763,7 +863,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure
};
return new ActionSelector(
decisionTreeProvider,
actionProvider.Object,
GetActionConstraintCache(actionConstraintProviders),
loggerFactory);
}

View File

@ -208,6 +208,11 @@ namespace Microsoft.AspNetCore.Mvc.Performance
private static void Verify(IReadOnlyList<ActionDescriptor> expected, IReadOnlyList<ActionDescriptor> actual)
{
if (expected.Count == 0 && actual == null)
{
return;
}
if (expected.Count != actual.Count)
{
throw new InvalidOperationException("The count is different.");
@ -227,7 +232,7 @@ namespace Microsoft.AspNetCore.Mvc.Performance
var actionCollection = new MockActionDescriptorCollectionProvider(actions);
return new ActionSelector(
new ActionSelectorDecisionTreeProvider(actionCollection),
actionCollection,
new ActionConstraintCache(actionCollection, Enumerable.Empty<IActionConstraintProvider>()),
NullLoggerFactory.Instance);
}