Skip to content

Commit

Permalink
Reduce allocations during HTML generation
Browse files Browse the repository at this point in the history
- aspnet#3918
- precompute size of `StringBuilder` in `ExpressionHelper`
- reduce `string` allocations in `ViewDataEvaluator`
 - also get rid of `Enumeration` state machines
- reduce the size of a few objects; use more expression-valued properties
 - e.g. don't store `_modelType` in `ModelExplorer`
- add `EmptyArray<TElement>`; make empty arrays consistently `static`
- avoid `string.Split()` in HTML and tag helpers

nits:
- make `ExpressionHelperTest` tests more stringent
- correct `Message` for an `ArgumentNullException`
- remove excess `using`s in classes I touched (but often ended up leaving otherwise unchanged)
- improve doc comments
- remove `ToString()` call on a `string`
- avoid encoding `string.Empty`
- fix test file name
- remove useless variables
- correct spelling
- improve whitespace
  • Loading branch information
dougbu committed Oct 3, 2016
1 parent 92682b7 commit 4cca6b0
Show file tree
Hide file tree
Showing 55 changed files with 350 additions and 271 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ public ModelStateEntry this[string key]
}
}

// Flag that indiciates if TooManyModelErrorException has already been added to this dictionary.
// Flag that indicates if TooManyModelErrorException has already been added to this dictionary.
private bool HasRecordedMaxModelError { get; set; }

/// <summary>
Expand Down Expand Up @@ -625,7 +625,7 @@ private void AddModelErrorCore(string key, Exception exception)
}

/// <summary>
/// Removes all keys and values from ths instance of <see cref="ModelStateDictionary"/>.
/// Removes all keys and values from this instance of <see cref="ModelStateDictionary"/>.
/// </summary>
public void Clear()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
public abstract class ModelStateEntry
{
private ModelErrorCollection _errors;

/// <summary>
/// Gets the raw value from the request associated with this entry.
/// </summary>
Expand Down
3 changes: 2 additions & 1 deletion src/Microsoft.AspNetCore.Mvc.Core/BindAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using System.Linq;
using Microsoft.AspNetCore.Mvc.Internal;
#if NETSTANDARD1_6
using System.Reflection;
#endif
Expand Down Expand Up @@ -83,7 +84,7 @@ private static IEnumerable<string> SplitString(string original)
{
if (string.IsNullOrEmpty(original))
{
return new string[0];
return EmptyArray<string>.Instance;
}

var split = original.Split(',').Select(piece => piece.Trim()).Where(piece => !string.IsNullOrEmpty(piece));
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.AspNetCore.Mvc.Core/ChallengeResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public class ChallengeResult : ActionResult
/// Initializes a new instance of <see cref="ChallengeResult"/>.
/// </summary>
public ChallengeResult()
: this(new string[] { })
: this(EmptyArray<string>.Instance)
{
}

Expand Down Expand Up @@ -51,7 +51,7 @@ public ChallengeResult(IList<string> authenticationSchemes)
/// <param name="properties"><see cref="AuthenticationProperties"/> used to perform the authentication
/// challenge.</param>
public ChallengeResult(AuthenticationProperties properties)
: this(new string[] { }, properties)
: this(EmptyArray<string>.Instance, properties)
{
}

Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.AspNetCore.Mvc.Core/ConsumesAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public ConsumesAttribute(string contentType, params string[] otherContentTypes)
// The value used is a non default value so that it avoids getting mixed with other action constraints
// with default order.
/// <inheritdoc />
int IActionConstraint.Order { get; } = ConsumesActionConstraintOrder;
int IActionConstraint.Order => ConsumesActionConstraintOrder;

/// <summary>
/// Gets or sets the supported request content types. Used to select an action when there would otherwise be
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public MiddlewareFilterAttribute(Type configurationType)
public int Order { get; set; }

/// <inheritdoc />
public bool IsReusable { get; } = true;
public bool IsReusable => true;

/// <inheritdoc />
public IFilterMetadata CreateInstance(IServiceProvider serviceProvider)
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.AspNetCore.Mvc.Core/ForbidResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public class ForbidResult : ActionResult
/// Initializes a new instance of <see cref="ForbidResult"/>.
/// </summary>
public ForbidResult()
: this(new string[] { })
: this(EmptyArray<string>.Instance)
{
}

Expand Down Expand Up @@ -51,7 +51,7 @@ public ForbidResult(IList<string> authenticationSchemes)
/// <param name="properties"><see cref="AuthenticationProperties"/> used to perform the authentication
/// challenge.</param>
public ForbidResult(AuthenticationProperties properties)
: this(new string[] { }, properties)
: this(EmptyArray<string>.Instance, properties)
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System;
using System.Collections.Generic;
using Microsoft.AspNetCore.Mvc.Core;
using Microsoft.Extensions.Primitives;
using Microsoft.Net.Http.Headers;

namespace Microsoft.AspNetCore.Mvc.Formatters
Expand All @@ -18,7 +17,7 @@ public class FormatterMappings
new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);

/// <summary>
/// Sets mapping for the format to specified media type.
/// Sets mapping for the format to specified media type.
/// If the format already exists, the media type will be overwritten with the new value.
/// </summary>
/// <param name="format">The format value.</param>
Expand All @@ -39,7 +38,7 @@ public void SetMediaTypeMappingForFormat(string format, string contentType)
}

/// <summary>
/// Sets mapping for the format to specified media type.
/// Sets mapping for the format to specified media type.
/// If the format already exists, the media type will be overwritten with the new value.
/// </summary>
/// <param name="format">The format value.</param>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
{
public class ClientValidatorCache
{
private readonly IReadOnlyList<IClientModelValidator> EmptyArray = new IClientModelValidator[0];

private readonly ConcurrentDictionary<ModelMetadata, CacheEntry> _cacheEntries = new ConcurrentDictionary<ModelMetadata, CacheEntry>();

public IReadOnlyList<IClientModelValidator> GetValidators(ModelMetadata metadata, IClientModelValidatorProvider validatorProvider)
Expand Down Expand Up @@ -106,7 +104,7 @@ private IReadOnlyList<IClientModelValidator> ExtractValidators(List<ClientValida

if (count == 0)
{
return EmptyArray;
return EmptyArray<IClientModelValidator>.Instance;
}

var validators = new IClientModelValidator[count];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using Microsoft.AspNetCore.Mvc.Abstractions;
using Microsoft.AspNetCore.Mvc.Filters;
Expand All @@ -13,8 +12,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
{
public class ControllerActionInvokerCache
{
private readonly IFilterMetadata[] EmptyFilterArray = new IFilterMetadata[0];

private readonly IActionDescriptorCollectionProvider _collectionProvider;
private readonly IFilterProvider[] _filterProviders;

Expand Down Expand Up @@ -134,7 +131,7 @@ private IFilterMetadata[] GetFilters(ActionContext actionContext, List<FilterIte

if (count == 0)
{
return EmptyFilterArray;
return EmptyArray<IFilterMetadata>.Instance;
}
else
{
Expand Down
19 changes: 19 additions & 0 deletions src/Microsoft.AspNetCore.Mvc.Core/Internal/EmptyArray.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// 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.

#if !NET451
using System;
#endif

namespace Microsoft.AspNetCore.Mvc.Internal
{
public static class EmptyArray<TElement>
{
public static TElement[] Instance { get; } =
#if NET451
new TElement[0];
#else
Array.Empty<TElement>();
#endif
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ private void SetContentDispositionHeader(ActionContext context, FileResult resul
private void SetContentType(ActionContext context, FileResult result)
{
var response = context.HttpContext.Response;
response.ContentType = result.ContentType.ToString();
response.ContentType = result.ContentType;
}

protected static ILogger CreateLogger<T>(ILoggerFactory factory)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
/// </summary>
public class PrefixContainer
{
private static readonly string[] EmptyArray = new string[0];
private static readonly char[] Delimiters = new char[] { '[', '.' };

private readonly ICollection<string> _originalValues;
Expand All @@ -31,7 +30,7 @@ public PrefixContainer(ICollection<string> values)

if (_originalValues.Count == 0)
{
_sortedValues = EmptyArray;
_sortedValues = EmptyArray<string>.Instance;
}
else
{
Expand Down
4 changes: 1 addition & 3 deletions src/Microsoft.AspNetCore.Mvc.Core/Internal/ValidatorCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
{
public class ValidatorCache
{
private readonly IReadOnlyList<IModelValidator> EmptyArray = new IModelValidator[0];

private readonly ConcurrentDictionary<ModelMetadata, CacheEntry> _cacheEntries = new ConcurrentDictionary<ModelMetadata, CacheEntry>();

public IReadOnlyList<IModelValidator> GetValidators(ModelMetadata metadata, IModelValidatorProvider validatorProvider)
Expand Down Expand Up @@ -105,7 +103,7 @@ private IReadOnlyList<IModelValidator> ExtractValidators(List<ValidatorItem> ite

if (count == 0)
{
return EmptyArray;
return EmptyArray<IModelValidator>.Instance;
}

var validators = new IModelValidator[count];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using Microsoft.AspNetCore.Mvc.Internal;

namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
{
Expand Down Expand Up @@ -36,7 +37,7 @@ protected override object CreateEmptyCollection(Type targetType)
{
Debug.Assert(targetType == typeof(TElement[]), "GenericModelBinder only creates this binder for arrays.");

return new TElement[0];
return EmptyArray<TElement>.Instance;
}

/// <inheritdoc />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using System.Linq;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc.ModelBinding.Validation;
using Microsoft.Extensions.Primitives;

namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
Expand Down Expand Up @@ -36,7 +35,7 @@ public async Task BindModelAsync(ModelBindingContext bindingContext)
{
model = new EmptyFormCollection();
}

bindingContext.Result = ModelBindingResult.Success(model);
}

Expand Down
11 changes: 3 additions & 8 deletions src/Microsoft.AspNetCore.Mvc.Localization/LocalizedHtmlString.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@

using System;
using System.IO;
using System.Text;
using System.Text.Encodings.Web;
using Microsoft.AspNetCore.Html;
using Microsoft.AspNetCore.Mvc.Internal;

namespace Microsoft.AspNetCore.Mvc.Localization
{
Expand All @@ -14,11 +14,6 @@ namespace Microsoft.AspNetCore.Mvc.Localization
/// </summary>
public class LocalizedHtmlString : IHtmlContent
{
#if NETSTANDARD1_4
private static readonly object[] EmptyArguments = Array.Empty<object>();
#else
private static readonly object[] EmptyArguments = new object[0];
#endif
private readonly object[] _arguments;

/// <summary>
Expand All @@ -27,7 +22,7 @@ public class LocalizedHtmlString : IHtmlContent
/// <param name="name">The name of the string resource.</param>
/// <param name="value">The string resource.</param>
public LocalizedHtmlString(string name, string value)
: this(name, value, isResourceNotFound: false, arguments: EmptyArguments)
: this(name, value, isResourceNotFound: false, arguments: EmptyArray<string>.Instance)
{
}

Expand All @@ -38,7 +33,7 @@ public LocalizedHtmlString(string name, string value)
/// <param name="value">The string resource.</param>
/// <param name="isResourceNotFound">A flag that indicates if the resource is not found.</param>
public LocalizedHtmlString(string name, string value, bool isResourceNotFound)
: this(name, value, isResourceNotFound, arguments: EmptyArguments)
: this(name, value, isResourceNotFound, arguments: EmptyArray<string>.Instance)
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Collections.Generic;
using System.Linq.Expressions;
using System.Reflection;
using Microsoft.AspNetCore.Mvc.Internal;
using Microsoft.AspNetCore.Mvc.Razor.Compilation;
using Microsoft.Extensions.Primitives;

Expand All @@ -22,7 +23,7 @@ public struct CompilerCacheResult
/// <param name="relativePath">Path of the view file relative to the application base.</param>
/// <param name="compilationResult">The <see cref="Compilation.CompilationResult"/>.</param>
public CompilerCacheResult(string relativePath, CompilationResult compilationResult)
: this(relativePath, compilationResult, new IChangeToken[0])
: this(relativePath, compilationResult, EmptyArray<IChangeToken>.Instance)
{
}

Expand Down Expand Up @@ -86,6 +87,5 @@ public CompilerCacheResult(IList<IChangeToken> expirationTokens)
/// Gets a delegate that creates an instance of the <see cref="IRazorPage"/>.
/// </summary>
public Func<IRazorPage> PageFactory { get; }

}
}
5 changes: 2 additions & 3 deletions src/Microsoft.AspNetCore.Mvc.Razor/RazorViewEngine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Globalization;
using System.Linq;
using System.Text.Encodings.Web;
using Microsoft.AspNetCore.Mvc.Internal;
using Microsoft.AspNetCore.Mvc.Razor.Internal;
using Microsoft.AspNetCore.Mvc.ViewEngines;
using Microsoft.Extensions.Caching.Memory;
Expand All @@ -31,8 +32,6 @@ public class RazorViewEngine : IRazorViewEngine

private const string ControllerKey = "controller";
private const string AreaKey = "area";
private static readonly ViewLocationCacheItem[] EmptyViewStartLocationCacheItems =
new ViewLocationCacheItem[0];
private static readonly TimeSpan _cacheExpirationDuration = TimeSpan.FromMinutes(20);

private readonly IRazorPageFactoryProvider _pageFactory;
Expand Down Expand Up @@ -415,7 +414,7 @@ private ViewLocationCacheResult CreateCacheResult(
// Only need to lookup _ViewStarts for the main page.
var viewStartPages = isMainPage ?
GetViewStartPages(relativePath, expirationTokens) :
EmptyViewStartLocationCacheItems;
EmptyArray<ViewLocationCacheItem>.Instance;

return new ViewLocationCacheResult(
new ViewLocationCacheItem(factoryResult.RazorPageFactory, relativePath),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc.Internal;
using Microsoft.Extensions.Caching.Memory;
using Microsoft.Extensions.FileProviders;
using Microsoft.Extensions.FileSystemGlobbing;
Expand All @@ -18,13 +19,6 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers.Internal
/// </summary>
public class GlobbingUrlBuilder
{
private static readonly IReadOnlyList<string> EmptyList =
#if NET451
new string[0];
#else
Array.Empty<string>();
#endif

// Valid whitespace characters defined by the HTML5 spec.
private static readonly char[] ValidAttributeWhitespaceChars =
new[] { '\t', '\n', '\u000C', '\r', ' ' };
Expand Down Expand Up @@ -116,7 +110,7 @@ private IReadOnlyList<string> ExpandGlobbedUrl(string include, string exclude)
{
if (string.IsNullOrEmpty(include))
{
return EmptyList;
return EmptyArray<string>.Instance;
}

var cacheKey = new GlobbingUrlKey(include, exclude);
Expand All @@ -130,7 +124,7 @@ private IReadOnlyList<string> ExpandGlobbedUrl(string include, string exclude)
var includeEnumerator = includeTokenizer.GetEnumerator();
if (!includeEnumerator.MoveNext())
{
return EmptyList;
return EmptyArray<string>.Instance;
}

var options = new MemoryCacheEntryOptions();
Expand Down
Loading

0 comments on commit 4cca6b0

Please sign in to comment.