Optimize defensive copies where possible

This commit is contained in:
Ryan Nowak 2015-12-07 17:49:37 -08:00
Родитель 48eb000a0e
Коммит 4b980c8afa
7 изменённых файлов: 392 добавлений и 95 удалений

Просмотреть файл

@ -12,6 +12,7 @@ namespace Microsoft.AspNetCore.Routing
public class RouteData
{
private RouteValueDictionary _dataTokens;
private IList<IRouter> _routers;
private RouteValueDictionary _values;
/// <summary>
@ -19,8 +20,7 @@ namespace Microsoft.AspNetCore.Routing
/// </summary>
public RouteData()
{
// Perf: Avoid allocating DataTokens and RouteValues unless needed.
Routers = new List<IRouter>();
// Perf: Avoid allocating collections unless needed.
}
/// <summary>
@ -34,9 +34,12 @@ namespace Microsoft.AspNetCore.Routing
throw new ArgumentNullException(nameof(other));
}
Routers = new List<IRouter>(other.Routers);
// Perf: Avoid allocating DataTokens and RouteValues unless we need to make a copy.
// Perf: Avoid allocating collections unless we need to make a copy.
if (other._routers != null)
{
_routers = new List<IRouter>(other.Routers);
}
if (other._dataTokens != null)
{
_dataTokens = new RouteValueDictionary(other._dataTokens);
@ -67,7 +70,18 @@ namespace Microsoft.AspNetCore.Routing
/// <summary>
/// Gets the list of <see cref="IRouter"/> instances on the current routing path.
/// </summary>
public List<IRouter> Routers { get; }
public IList<IRouter> Routers
{
get
{
if (_routers == null)
{
_routers = new List<IRouter>();
}
return _routers;
}
}
/// <summary>
/// Gets the set of values produced by routes on the current routing path.
@ -84,5 +98,160 @@ namespace Microsoft.AspNetCore.Routing
return _values;
}
}
/// <summary>
/// <para>
/// Creates a snapshot of the current state of the <see cref="RouteData"/> before appending
/// <paramref name="router"/> to <see cref="Routers"/>, merging <paramref name="values"/> into
/// <see cref="Values"/>, and merging <paramref name="dataTokens"/> into <see cref="DataTokens"/>.
/// </para>
/// <para>
/// Call <see cref="RouteDataSnapshot.Restore"/> to restore the state of this <see cref="RouteData"/>
/// to the state at the time of calling
/// <see cref="PushState(IRouter, RouteValueDictionary, RouteValueDictionary)"/>.
/// </para>
/// </summary>
/// <param name="router">
/// An <see cref="IRouter"/> to append to <see cref="Routers"/>. If <c>null</c>, then <see cref="Routers"/>
/// will not be changed.
/// </param>
/// <param name="values">
/// A <see cref="RouteValueDictionary"/> to merge into <see cref="Values"/>. If <c>null</c>, then
/// <see cref="Values"/> will not be changed.
/// </param>
/// <param name="dataTokens">
/// A <see cref="RouteValueDictionary"/> to merge into <see cref="DataTokens"/>. If <c>null</c>, then
/// <see cref="DataTokens"/> will not be changed.
/// </param>
/// <returns>A <see cref="RouteDataSnapshot"/> that captures the current state.</returns>
public RouteDataSnapshot PushState(IRouter router, RouteValueDictionary values, RouteValueDictionary dataTokens)
{
var snapshot = new RouteDataSnapshot(
this,
_dataTokens?.Count > 0 ? new RouteValueDictionary(_dataTokens) : null,
_routers?.Count > 0 ? new List<IRouter>(_routers) : null,
_values?.Count > 0 ? new RouteValueDictionary(_values) : null);
if (router != null)
{
Routers.Add(router);
}
if (values != null)
{
foreach (var kvp in values)
{
if (kvp.Value != null)
{
Values[kvp.Key] = kvp.Value;
}
}
}
if (dataTokens != null)
{
foreach (var kvp in dataTokens)
{
DataTokens[kvp.Key] = kvp.Value;
}
}
return snapshot;
}
/// <summary>
/// A snapshot of the state of a <see cref="RouteData"/> instance.
/// </summary>
public struct RouteDataSnapshot
{
private readonly RouteData _routeData;
private readonly RouteValueDictionary _dataTokens;
private readonly IList<IRouter> _routers;
private readonly RouteValueDictionary _values;
/// <summary>
/// Creates a new <see cref="RouteDataSnapshot"/> for <paramref name="routeData"/>.
/// </summary>
/// <param name="routeData">The <see cref="RouteData"/>.</param>
/// <param name="dataTokens">The data tokens.</param>
/// <param name="routers">The routers.</param>
/// <param name="values">The route values.</param>
public RouteDataSnapshot(
RouteData routeData,
RouteValueDictionary dataTokens,
IList<IRouter> routers,
RouteValueDictionary values)
{
if (routeData == null)
{
throw new ArgumentNullException(nameof(routeData));
}
_routeData = routeData;
_dataTokens = dataTokens;
_routers = routers;
_values = values;
}
/// <summary>
/// Restores the <see cref="RouteData"/> to the captured state.
/// </summary>
public void Restore()
{
if (_routeData._dataTokens == null && _dataTokens == null)
{
// Do nothing
}
else if (_dataTokens == null)
{
_routeData._dataTokens.Clear();
}
else
{
_routeData._dataTokens.Clear();
foreach (var kvp in _dataTokens)
{
_routeData._dataTokens.Add(kvp.Key, kvp.Value);
}
}
if (_routeData._routers == null && _routers == null)
{
// Do nothing
}
else if (_routers == null)
{
_routeData._routers.Clear();
}
else
{
_routeData._routers.Clear();
for (var i = 0; i < _routers.Count; i++)
{
_routeData._routers.Add(_routers[i]);
}
}
if (_routeData._values == null && _values == null)
{
// Do nothing
}
else if (_values == null)
{
_routeData._values.Clear();
}
else
{
_routeData._values.Clear();
foreach (var kvp in _values)
{
_routeData._values.Add(kvp.Key, kvp.Value);
}
}
}
}
}
}

Просмотреть файл

@ -19,6 +19,12 @@ namespace Microsoft.AspNetCore.Routing
{
private TemplateMatcher _matcher;
private TemplateBinder _binder;
private readonly IReadOnlyDictionary<string, IRouteConstraint> _constraints;
private readonly RouteValueDictionary _dataTokens;
private readonly RouteValueDictionary _defaults;
private readonly IRouter _target;
private readonly RouteTemplate _parsedTemplate;
private readonly string _routeTemplate;
private ILogger _logger;
private ILogger _constraintLogger;

Просмотреть файл

@ -54,19 +54,18 @@ namespace Microsoft.AspNetCore.Routing
public async virtual Task RouteAsync(RouteContext context)
{
// Perf: We want to avoid allocating a new RouteData for each route we need to process.
// We can do this by snapshotting the state at the beginning and then restoring it
// for each router we execute.
var snapshot = context.RouteData.PushState(null, values: null, dataTokens: null);
for (var i = 0; i < Count; i++)
{
var route = this[i];
var oldRouteData = context.RouteData;
var newRouteData = new RouteData(oldRouteData);
newRouteData.Routers.Add(route);
context.RouteData.Routers.Add(route);
try
{
context.RouteData = newRouteData;
await route.RouteAsync(context);
if (context.Handler != null)
@ -78,7 +77,7 @@ namespace Microsoft.AspNetCore.Routing
{
if (context.Handler == null)
{
context.RouteData = oldRouteData;
snapshot.Restore();
}
}
}

Просмотреть файл

@ -173,31 +173,23 @@ namespace Microsoft.AspNetCore.Routing.Tree
}
var match = new TemplateMatch(item, values);
var oldRouteData = context.RouteData;
var newRouteData = new RouteData(oldRouteData);
newRouteData.Routers.Add(match.Entry.Target);
MergeValues(newRouteData.Values, match.Values);
if (!RouteConstraintMatcher.Match(
match.Entry.Constraints,
newRouteData.Values,
context.HttpContext,
this,
RouteDirection.IncomingRequest,
_constraintLogger))
{
continue;
}
_logger.MatchedRoute(match.Entry.RouteName, match.Entry.RouteTemplate.TemplateText);
context.RouteData = newRouteData;
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))
{
continue;
}
_logger.MatchedRoute(match.Entry.RouteName, match.Entry.RouteTemplate.TemplateText);
await match.Entry.Target.RouteAsync(context);
if (context.Handler != null)
{
@ -209,7 +201,7 @@ namespace Microsoft.AspNetCore.Routing.Tree
if (context.Handler == null)
{
// Restore the original values to prevent polluting the route data.
context.RouteData = oldRouteData;
snapshot.Restore();
}
}
}
@ -308,22 +300,6 @@ namespace Microsoft.AspNetCore.Routing.Tree
}
}
private static void MergeValues(
RouteValueDictionary destination,
RouteValueDictionary values)
{
foreach (var kvp in values)
{
if (kvp.Value != null)
{
// This will replace the original value for the specified key.
// Values from the matched route will take preference over previous
// data in the route context.
destination[kvp.Key] = kvp.Value;
}
}
}
private struct TemplateMatch : IEquatable<TemplateMatch>
{
public TemplateMatch(TreeRouteMatchingEntry entry, RouteValueDictionary values)

Просмотреть файл

@ -0,0 +1,157 @@
// 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 Moq;
using Xunit;
namespace Microsoft.AspNetCore.Routing
{
public class RouteDataTest
{
[Fact]
public void RouteData_DefaultPropertyValues()
{
// Arrange & Act
var routeData = new RouteData();
// Assert
Assert.Empty(routeData.DataTokens);
Assert.Empty(routeData.Routers);
Assert.Empty(routeData.Values);
}
[Fact]
public void RouteData_CopyConstructor()
{
// Arrange & Act
var original = new RouteData();
original.DataTokens.Add("data", "token");
original.Routers.Add(Mock.Of<IRouter>());
original.Values.Add("route", "value");
var routeData = new RouteData(original);
// Assert
Assert.NotSame(routeData.DataTokens, original.DataTokens);
Assert.Equal(routeData.DataTokens, original.DataTokens);
Assert.NotSame(routeData.Routers, original.Routers);
Assert.Equal(routeData.Routers, original.Routers);
Assert.NotSame(routeData.Values, original.Values);
Assert.Equal(routeData.Values, original.Values);
}
[Fact]
public void RouteData_PushStateAndRestore_NullValues()
{
// Arrange
var routeData = new RouteData();
// Act
var snapshot = routeData.PushState(null, null, null);
var copy = new RouteData(routeData);
snapshot.Restore();
// Assert
Assert.Equal(routeData.DataTokens, copy.DataTokens);
Assert.Equal(routeData.Routers, copy.Routers);
Assert.Equal(routeData.Values, copy.Values);
}
[Fact]
public void RouteData_PushStateAndRestore_EmptyValues()
{
// Arrange
var routeData = new RouteData();
// Act
var snapshot = routeData.PushState(null, new RouteValueDictionary(), new RouteValueDictionary());
var copy = new RouteData(routeData);
snapshot.Restore();
// Assert
Assert.Equal(routeData.DataTokens, copy.DataTokens);
Assert.Equal(routeData.Routers, copy.Routers);
Assert.Equal(routeData.Values, copy.Values);
}
// This is an important semantic for catchall parameters. A null route value shouldn't be
// merged.
[Fact]
public void RouteData_PushStateAndRestore_NullRouteValueNotSet()
{
// Arrange
var original = new RouteData();
original.Values.Add("bleh", "16");
var routeData = new RouteData(original);
// Act
var snapshot = routeData.PushState(
null,
new RouteValueDictionary(new { bleh = (string)null }),
new RouteValueDictionary());
snapshot.Restore();
// Assert
Assert.Equal(routeData.Values, original.Values);
}
[Fact]
public void RouteData_PushStateAndThenModify()
{
// Arrange
var routeData = new RouteData();
// Act
var snapshot = routeData.PushState(null, null, null);
routeData.DataTokens.Add("data", "token");
routeData.Routers.Add(Mock.Of<IRouter>());
routeData.Values.Add("route", "value");
var copy = new RouteData(routeData);
snapshot.Restore();
// Assert
Assert.Empty(routeData.DataTokens);
Assert.NotEqual(routeData.DataTokens, copy.DataTokens);
Assert.Empty(routeData.Routers);
Assert.NotEqual(routeData.Routers, copy.Routers);
Assert.Empty(routeData.Values);
Assert.NotEqual(routeData.Values, copy.Values);
}
[Fact]
public void RouteData_PushStateAndThenModify_WithInitialData()
{
// Arrange
var original = new RouteData();
original.DataTokens.Add("data", "token1");
original.Routers.Add(Mock.Of<IRouter>());
original.Values.Add("route", "value1");
var routeData = new RouteData(original);
// Act
var snapshot = routeData.PushState(
Mock.Of<IRouter>(),
new RouteValueDictionary(new { route = "value2" }),
new RouteValueDictionary(new { data = "token2" }));
routeData.DataTokens.Add("data2", "token");
routeData.Routers.Add(Mock.Of<IRouter>());
routeData.Values.Add("route2", "value");
var copy = new RouteData(routeData);
snapshot.Restore();
// Assert
Assert.Equal(original.DataTokens, routeData.DataTokens);
Assert.NotEqual(routeData.DataTokens, copy.DataTokens);
Assert.Equal(original.Routers, routeData.Routers);
Assert.NotEqual(routeData.Routers, copy.Routers);
Assert.Equal(original.Values, routeData.Values);
Assert.NotEqual(routeData.Values, copy.Values);
}
}
}

Просмотреть файл

@ -155,11 +155,9 @@ namespace Microsoft.AspNetCore.Routing
Assert.Equal("USA", context.RouteData.Values["country"]);
Assert.True(context.RouteData.Values.ContainsKey("id"));
Assert.Equal("5", context.RouteData.Values["id"]);
Assert.Same(originalRouteDataValues, context.RouteData.Values);
Assert.Equal("Contoso", context.RouteData.DataTokens["company"]);
Assert.Equal("Friday", context.RouteData.DataTokens["today"]);
Assert.Same(originalDataTokens, context.RouteData.DataTokens);
}
[Fact]

Просмотреть файл

@ -1343,18 +1343,18 @@ namespace Microsoft.AspNetCore.Routing.Tree
}
[Fact]
public async Task TreeRouter_CreatesNewRouteData()
public async Task TreeRouter_SnapshotsRouteData()
{
// Arrange
RouteData nestedRouteData = null;
RouteValueDictionary nestedValues = null;
var next = new Mock<IRouter>();
next
.Setup(r => r.RouteAsync(It.IsAny<RouteContext>()))
.Callback<RouteContext>(c =>
{
nestedRouteData = c.RouteData;
c.Handler = NullHandler;
nestedValues = new RouteValueDictionary(c.RouteData.Values);
c.Handler = null; // Not a match
})
.Returns(Task.FromResult(true))
.Verifiable();
@ -1364,23 +1364,17 @@ namespace Microsoft.AspNetCore.Routing.Tree
var context = CreateRouteContext("/api/Store");
var originalRouteData = context.RouteData;
originalRouteData.Values.Add("action", "Index");
var routeData = context.RouteData;
routeData.Values.Add("action", "Index");
var originalValues = new RouteValueDictionary(context.RouteData.Values);
// Act
await route.RouteAsync(context);
// Assert
Assert.NotSame(originalRouteData, context.RouteData);
Assert.NotSame(originalRouteData, nestedRouteData);
Assert.Same(nestedRouteData, context.RouteData);
// The new routedata is a copy
Assert.Equal("Index", context.RouteData.Values["action"]);
Assert.Single(context.RouteData.Values, kvp => kvp.Key == "test_route_group");
Assert.Equal(1, context.RouteData.Routers.Count);
Assert.Equal(next.Object.GetType(), context.RouteData.Routers[0].GetType());
Assert.Equal(originalValues, context.RouteData.Values);
Assert.NotEqual(nestedValues, context.RouteData.Values);
}
[Fact]
@ -1436,16 +1430,19 @@ namespace Microsoft.AspNetCore.Routing.Tree
}
[Fact]
public async Task TreeRouter_CreatesNewRouteData_ResetsWhenNotMatched()
public async Task TreeRouter_SnapshotsRouteData_ResetsWhenNotMatched()
{
// Arrange
RouteData nestedRouteData = null;
RouteValueDictionary nestedValues = null;
List<IRouter> nestedRouters = null;
var next = new Mock<IRouter>();
next
.Setup(r => r.RouteAsync(It.IsAny<RouteContext>()))
.Callback<RouteContext>(c =>
{
nestedRouteData = c.RouteData;
nestedValues = new RouteValueDictionary(c.RouteData.Values);
nestedRouters = new List<IRouter>(c.RouteData.Routers);
})
.Returns(Task.FromResult(true))
.Verifiable();
@ -1455,40 +1452,39 @@ namespace Microsoft.AspNetCore.Routing.Tree
var context = CreateRouteContext("/api/Store");
var originalRouteData = context.RouteData;
originalRouteData.Values.Add("action", "Index");
context.RouteData.Values.Add("action", "Index");
// Act
await route.RouteAsync(context);
// Assert
Assert.Same(originalRouteData, context.RouteData);
Assert.NotSame(originalRouteData, nestedRouteData);
Assert.NotSame(nestedRouteData, context.RouteData);
Assert.NotEqual(nestedValues, context.RouteData.Values);
// The new routedata is a copy
Assert.Equal("Index", context.RouteData.Values["action"]);
Assert.Equal("Index", nestedRouteData.Values["action"]);
Assert.Equal("Index", nestedValues["action"]);
Assert.DoesNotContain(context.RouteData.Values, kvp => kvp.Key == "test_route_group");
Assert.Single(nestedRouteData.Values, kvp => kvp.Key == "test_route_group");
Assert.Single(nestedValues, kvp => kvp.Key == "test_route_group");
Assert.Empty(context.RouteData.Routers);
Assert.Equal(1, nestedRouteData.Routers.Count);
Assert.Equal(next.Object.GetType(), nestedRouteData.Routers[0].GetType());
Assert.Equal(1, nestedRouters.Count);
Assert.Equal(next.Object.GetType(), nestedRouters[0].GetType());
}
[Fact]
public async Task TreeRouter_CreatesNewRouteData_ResetsWhenThrows()
{
// Arrange
RouteData nestedRouteData = null;
RouteValueDictionary nestedValues = null;
List<IRouter> nestedRouters = null;
var next = new Mock<IRouter>();
next
.Setup(r => r.RouteAsync(It.IsAny<RouteContext>()))
.Callback<RouteContext>(c =>
{
nestedRouteData = c.RouteData;
nestedValues = new RouteValueDictionary(c.RouteData.Values);
nestedRouters = new List<IRouter>(c.RouteData.Routers);
})
.Throws(new Exception());
@ -1496,28 +1492,24 @@ namespace Microsoft.AspNetCore.Routing.Tree
var route = CreateAttributeRoute(next.Object, entry);
var context = CreateRouteContext("/api/Store");
var originalRouteData = context.RouteData;
originalRouteData.Values.Add("action", "Index");
context.RouteData.Values.Add("action", "Index");
// Act
await Assert.ThrowsAsync<Exception>(() => route.RouteAsync(context));
// Assert
Assert.Same(originalRouteData, context.RouteData);
Assert.NotSame(originalRouteData, nestedRouteData);
Assert.NotSame(nestedRouteData, context.RouteData);
Assert.NotEqual(nestedValues, context.RouteData.Values);
// The new routedata is a copy
Assert.Equal("Index", context.RouteData.Values["action"]);
Assert.Equal("Index", nestedRouteData.Values["action"]);
Assert.Equal("Index", nestedValues["action"]);
Assert.DoesNotContain(context.RouteData.Values, kvp => kvp.Key == "test_route_group");
Assert.Single(nestedRouteData.Values, kvp => kvp.Key == "test_route_group");
Assert.Single(nestedValues, kvp => kvp.Key == "test_route_group");
Assert.Empty(context.RouteData.Routers);
Assert.Equal(1, nestedRouteData.Routers.Count);
Assert.Equal(next.Object.GetType(), nestedRouteData.Routers[0].GetType());
Assert.Equal(1, nestedRouters.Count);
Assert.Equal(next.Object.GetType(), nestedRouters[0].GetType());
}
private static RouteContext CreateRouteContext(string requestPath)