From 26e5ea32741f1782037ce06d90aa5c63a1f54996 Mon Sep 17 00:00:00 2001 From: Gert Driesen Date: Mon, 22 Oct 2018 23:20:15 +0200 Subject: [PATCH] Improves performance and reduce allocations in DefaultEndpointDataSource (#882) --- .../DefaultEndpointDataSource.cs | 12 ++-- .../DefaultEndpointDataSourceTests.cs | 57 ++++++++++++++++++- 2 files changed, 64 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.AspNetCore.Routing/DefaultEndpointDataSource.cs b/src/Microsoft.AspNetCore.Routing/DefaultEndpointDataSource.cs index 94728feb8c..7586ee4a2e 100644 --- a/src/Microsoft.AspNetCore.Routing/DefaultEndpointDataSource.cs +++ b/src/Microsoft.AspNetCore.Routing/DefaultEndpointDataSource.cs @@ -14,15 +14,20 @@ namespace Microsoft.AspNetCore.Routing /// public sealed class DefaultEndpointDataSource : EndpointDataSource { - private readonly List _endpoints; + private readonly IReadOnlyList _endpoints; /// /// Initializes a new instance of the class. /// /// The instances that the data source will return. public DefaultEndpointDataSource(params Endpoint[] endpoints) - : this((IEnumerable) endpoints) { + if (endpoints == null) + { + throw new ArgumentNullException(nameof(endpoints)); + } + + _endpoints = (Endpoint[])endpoints.Clone(); } /// @@ -36,8 +41,7 @@ namespace Microsoft.AspNetCore.Routing throw new ArgumentNullException(nameof(endpoints)); } - _endpoints = new List(); - _endpoints.AddRange(endpoints); + _endpoints = new List(endpoints); } /// diff --git a/test/Microsoft.AspNetCore.Routing.Tests/DefaultEndpointDataSourceTests.cs b/test/Microsoft.AspNetCore.Routing.Tests/DefaultEndpointDataSourceTests.cs index 09278332df..033f32ab55 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/DefaultEndpointDataSourceTests.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/DefaultEndpointDataSourceTests.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Text; using Microsoft.AspNetCore.Http; using Xunit; @@ -26,6 +25,34 @@ namespace Microsoft.AspNetCore.Routing endpoint => Assert.Equal("2", endpoint.DisplayName)); } + [Fact] + public void Constructor_Params_ShouldMakeCopyOfEndpoints() + { + // Arrange + var endpoint1 = new Endpoint(TestConstants.EmptyRequestDelegate, EndpointMetadataCollection.Empty, "1"); + var endpoint2 = new Endpoint(TestConstants.EmptyRequestDelegate, EndpointMetadataCollection.Empty, "2"); + var endpoints = new[] { endpoint1, endpoint2 }; + + // Act + var dataSource = new DefaultEndpointDataSource(endpoints); + Array.Resize(ref endpoints, 1); + endpoints[0] = null; + + // Assert + Assert.Equal(2, dataSource.Endpoints.Count); + Assert.Contains(endpoint1, dataSource.Endpoints); + Assert.Contains(endpoint2, dataSource.Endpoints); + } + + [Fact] + public void Constructor_Params_ShouldThrowArgumentNullExceptionWhenEndpointsIsNull() + { + Endpoint[] endpoints = null; + + var actual = Assert.Throws(() => new DefaultEndpointDataSource(endpoints)); + Assert.Equal("endpoints", actual.ParamName); + } + [Fact] public void Constructor_Enumerable_EndpointsInitialized() { @@ -41,5 +68,33 @@ namespace Microsoft.AspNetCore.Routing endpoint => Assert.Equal("1", endpoint.DisplayName), endpoint => Assert.Equal("2", endpoint.DisplayName)); } + + [Fact] + public void Constructor_Enumerable_ShouldMakeCopyOfEndpoints() + { + // Arrange + var endpoint1 = new Endpoint(TestConstants.EmptyRequestDelegate, EndpointMetadataCollection.Empty, "1"); + var endpoint2 = new Endpoint(TestConstants.EmptyRequestDelegate, EndpointMetadataCollection.Empty, "2"); + var endpoints = new List { endpoint1, endpoint2 }; + + // Act + var dataSource = new DefaultEndpointDataSource((IEnumerable)endpoints); + endpoints.RemoveAt(0); + endpoints[0] = null; + + // Assert + Assert.Equal(2, dataSource.Endpoints.Count); + Assert.Contains(endpoint1, dataSource.Endpoints); + Assert.Contains(endpoint2, dataSource.Endpoints); + } + + [Fact] + public void Constructor_Enumerable_ShouldThrowArgumentNullExceptionWhenEndpointsIsNull() + { + IEnumerable endpoints = null; + + var actual = Assert.Throws(() => new DefaultEndpointDataSource(endpoints)); + Assert.Equal("endpoints", actual.ParamName); + } } } \ No newline at end of file