Reduce `HtmlContentBuilder` allocations

- #3918
- lazy-load `TagBuilder.InnerHtml`; add `HasInnerHtml` property for conservative use
 - depends on aspnet/HtmlAbstractions@0781b5a (adds `HtmlContentBuilder.Count`)
- don't use a `HtmlContentBuilder` at all in `DefaultHtmlGenerator.GenerateValidationSummary()`
- avoid instantiating `HtmlContentBuilder` when short-circuits make it unnecessary
- provide correct `HtmlContentBuilder` capacity where known
This commit is contained in:
Doug Bunting 2016-10-01 13:30:24 -07:00
parent 69b4b64fd5
commit eea297975d
14 changed files with 149 additions and 73 deletions

View File

@ -1,6 +1,6 @@
Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio 14
VisualStudioVersion = 14.0.24720.0
VisualStudioVersion = 14.0.25420.1
MinimumVisualStudioVersion = 10.0.40219.1
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "samples", "samples", "{DAAE4C74-D06F-4874-A166-33305D2643CE}"
EndProject

View File

@ -226,7 +226,10 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
if (tagBuilder != null)
{
output.MergeAttributes(tagBuilder);
output.PostContent.AppendHtml(tagBuilder.InnerHtml);
if (tagBuilder.HasInnerHtml)
{
output.PostContent.AppendHtml(tagBuilder.InnerHtml);
}
}
if (string.Equals(Method, "get", StringComparison.OrdinalIgnoreCase))

View File

@ -218,10 +218,14 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
if (tagBuilder != null)
{
// This TagBuilder contains the one <input/> element of interest. Since this is not the "checkbox"
// special-case, output is a self-closing element no longer guaranteed.
// This TagBuilder contains the one <input/> element of interest.
output.MergeAttributes(tagBuilder);
output.Content.AppendHtml(tagBuilder.InnerHtml);
if (tagBuilder.HasInnerHtml)
{
// Since this is not the "checkbox" special-case, no guarantee that output is a self-closing
// element. A later tag helper targeting this element may change output.TagMode.
output.Content.AppendHtml(tagBuilder.InnerHtml);
}
}
}

View File

@ -72,17 +72,20 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
{
output.MergeAttributes(tagBuilder);
// We check for whitespace to detect scenarios such as:
// <label for="Name">
// </label>
// Do not update the content if another tag helper targeting this element has already done so.
if (!output.IsContentModified)
{
// We check for whitespace to detect scenarios such as:
// <label for="Name">
// </label>
var childContent = await output.GetChildContentAsync();
if (childContent.IsEmptyOrWhiteSpace)
{
// Provide default label text since there was nothing useful in the Razor source.
output.Content.SetHtmlContent(tagBuilder.InnerHtml);
// Provide default label text (if any) since there was nothing useful in the Razor source.
if (tagBuilder.HasInnerHtml)
{
output.Content.SetHtmlContent(tagBuilder.InnerHtml);
}
}
else
{

View File

@ -147,7 +147,10 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
if (tagBuilder != null)
{
output.MergeAttributes(tagBuilder);
output.PostContent.AppendHtml(tagBuilder.InnerHtml);
if (tagBuilder.HasInnerHtml)
{
output.PostContent.AppendHtml(tagBuilder.InnerHtml);
}
}
}
}

View File

@ -70,10 +70,12 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
if (tagBuilder != null)
{
// Overwrite current Content to ensure expression result round-trips correctly.
output.Content.SetHtmlContent(tagBuilder.InnerHtml);
output.MergeAttributes(tagBuilder);
if (tagBuilder.HasInnerHtml)
{
// Overwrite current Content to ensure expression result round-trips correctly.
output.Content.SetHtmlContent(tagBuilder.InnerHtml);
}
}
}
}

View File

@ -76,17 +76,20 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
{
output.MergeAttributes(tagBuilder);
// We check for whitespace to detect scenarios such as:
// <span validation-for="Name">
// </span>
// Do not update the content if another tag helper targeting this element has already done so.
if (!output.IsContentModified)
{
// We check for whitespace to detect scenarios such as:
// <span validation-for="Name">
// </span>
var childContent = await output.GetChildContentAsync();
if (childContent.IsEmptyOrWhiteSpace)
{
// Provide default label text since there was nothing useful in the Razor source.
output.Content.SetHtmlContent(tagBuilder.InnerHtml);
// Provide default message text (if any) since there was nothing useful in the Razor source.
if (tagBuilder.HasInnerHtml)
{
output.Content.SetHtmlContent(tagBuilder.InnerHtml);
}
}
else
{

View File

@ -112,7 +112,10 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
}
output.MergeAttributes(tagBuilder);
output.PostContent.AppendHtml(tagBuilder.InnerHtml);
if (tagBuilder.HasInnerHtml)
{
output.PostContent.AppendHtml(tagBuilder.InnerHtml);
}
}
}
}

View File

@ -93,8 +93,8 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
return HtmlString.Empty;
}
var collection = model as IEnumerable;
if (collection == null)
var enumerable = model as IEnumerable;
if (enumerable == null)
{
// Only way we could reach here is if user passed templateName: "Collection" to a Display() overload.
throw new InvalidOperationException(Resources.FormatTemplates_TypeMustImplementIEnumerable(
@ -119,12 +119,13 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
{
htmlHelper.ViewData.TemplateInfo.HtmlFieldPrefix = string.Empty;
var result = new HtmlContentBuilder();
var collection = model as ICollection;
var result = collection == null ? new HtmlContentBuilder() : new HtmlContentBuilder(collection.Count);
var viewEngine = serviceProvider.GetRequiredService<ICompositeViewEngine>();
var viewBufferScope = serviceProvider.GetRequiredService<IViewBufferScope>();
var index = 0;
foreach (var item in collection)
foreach (var item in enumerable)
{
var itemMetadata = elementMetadata;
if (item != null && !typeInCollectionIsNullableValueType)
@ -224,7 +225,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
var viewEngine = serviceProvider.GetRequiredService<ICompositeViewEngine>();
var viewBufferScope = serviceProvider.GetRequiredService<IViewBufferScope>();
var content = new HtmlContentBuilder();
var content = new HtmlContentBuilder(modelExplorer.Metadata.Properties.Count);
foreach (var propertyExplorer in modelExplorer.Properties)
{
var propertyMetadata = propertyExplorer.Metadata;

View File

@ -60,8 +60,8 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
return HtmlString.Empty;
}
var collection = model as IEnumerable;
if (collection == null)
var enumerable = model as IEnumerable;
if (enumerable == null)
{
// Only way we could reach here is if user passed templateName: "Collection" to an Editor() overload.
throw new InvalidOperationException(Resources.FormatTemplates_TypeMustImplementIEnumerable(
@ -86,12 +86,13 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
{
viewData.TemplateInfo.HtmlFieldPrefix = string.Empty;
var result = new HtmlContentBuilder();
var collection = model as ICollection;
var result = collection == null ? new HtmlContentBuilder() : new HtmlContentBuilder(collection.Count);
var viewEngine = serviceProvider.GetRequiredService<ICompositeViewEngine>();
var viewBufferScope = serviceProvider.GetRequiredService<IViewBufferScope>();
var index = 0;
foreach (var item in collection)
foreach (var item in enumerable)
{
var itemMetadata = elementMetadata;
if (item != null && !typeInCollectionIsNullableValueType)
@ -143,24 +144,26 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
var viewData = htmlHelper.ViewData;
var model = viewData.Model;
var result = new HtmlContentBuilder();
if (!viewData.ModelMetadata.HideSurroundingHtml)
IHtmlContent display;
if (viewData.ModelMetadata.HideSurroundingHtml)
{
result.AppendHtml(DefaultDisplayTemplates.StringTemplate(htmlHelper));
display = null;
}
// Special-case opaque values and arbitrary binary data.
var modelAsByteArray = model as byte[];
if (modelAsByteArray != null)
else
{
model = Convert.ToBase64String(modelAsByteArray);
display = DefaultDisplayTemplates.StringTemplate(htmlHelper);
}
var htmlAttributesObject = viewData[HtmlAttributeKey];
var hiddenResult = htmlHelper.Hidden(expression: null, value: model, htmlAttributes: htmlAttributesObject);
result.AppendHtml(hiddenResult);
var hidden = htmlHelper.Hidden(expression: null, value: model, htmlAttributes: htmlAttributesObject);
if (viewData.ModelMetadata.HideSurroundingHtml)
{
return hidden;
}
return result;
return new HtmlContentBuilder(capacity: 2)
.AppendHtml(display)
.AppendHtml(hidden);
}
private static IDictionary<string, object> CreateHtmlAttributes(
@ -250,7 +253,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
var viewEngine = serviceProvider.GetRequiredService<ICompositeViewEngine>();
var viewBufferScope = serviceProvider.GetRequiredService<IViewBufferScope>();
var content = new HtmlContentBuilder();
var content = new HtmlContentBuilder(modelExplorer.Metadata.Properties.Count);
foreach (var propertyExplorer in modelExplorer.Properties)
{
var propertyMetadata = propertyExplorer.Metadata;

View File

@ -21,6 +21,7 @@ namespace Microsoft.AspNetCore.Mvc.Rendering
public class TagBuilder : IHtmlContent
{
private AttributeDictionary _attributes;
private HtmlContentBuilder _innerHtml;
/// <summary>
/// Creates a new HTML tag that has the specified tag name.
@ -34,7 +35,6 @@ namespace Microsoft.AspNetCore.Mvc.Rendering
}
TagName = tagName;
InnerHtml = new HtmlContentBuilder();
}
/// <summary>
@ -57,7 +57,23 @@ namespace Microsoft.AspNetCore.Mvc.Rendering
/// <summary>
/// Gets the inner HTML content of the element.
/// </summary>
public IHtmlContentBuilder InnerHtml { get; }
public IHtmlContentBuilder InnerHtml
{
get
{
if (_innerHtml == null)
{
_innerHtml = new HtmlContentBuilder();
}
return _innerHtml;
}
}
/// <summary>
/// Gets an indication <see cref="InnerHtml"/> is not empty.
/// </summary>
public bool HasInnerHtml => _innerHtml?.Count > 0;
/// <summary>
/// Gets the tag name for this tag.
@ -291,7 +307,11 @@ namespace Microsoft.AspNetCore.Mvc.Rendering
writer.Write(TagName);
AppendAttributes(writer, encoder);
writer.Write(">");
InnerHtml.WriteTo(writer, encoder);
if (_innerHtml != null)
{
_innerHtml.WriteTo(writer, encoder);
}
writer.Write("</");
writer.Write(TagName);
writer.Write(">");

View File

@ -881,21 +881,20 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures
return null;
}
var wrappedMessage = new HtmlContentBuilder();
if (!string.IsNullOrEmpty(message))
TagBuilder messageTag;
if (string.IsNullOrEmpty(message))
{
messageTag = null;
}
else
{
if (string.IsNullOrEmpty(headerTag))
{
headerTag = viewContext.ValidationSummaryMessageElement;
}
var messageTag = new TagBuilder(headerTag);
messageTag = new TagBuilder(headerTag);
messageTag.InnerHtml.SetContent(message);
wrappedMessage.AppendLine(messageTag);
}
else
{
wrappedMessage = null;
}
// If excludePropertyErrors is true, describe any validation issue with the current model in a single item.
@ -940,7 +939,11 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures
tagBuilder.AddCssClass(HtmlHelper.ValidationSummaryCssClassName);
}
tagBuilder.InnerHtml.AppendHtml(wrappedMessage);
if (messageTag != null)
{
tagBuilder.InnerHtml.AppendLine(messageTag);
}
tagBuilder.InnerHtml.AppendHtml(htmlSummary);
if (viewContext.ClientValidationEnabled && !excludePropertyErrors)
@ -1518,7 +1521,25 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures
IEnumerable<SelectListItem> selectList,
ICollection<string> currentValues)
{
var listItemBuilder = new HtmlContentBuilder();
var itemsList = selectList as IList<SelectListItem>;
if (itemsList == null)
{
itemsList = selectList.ToList();
}
var count = itemsList.Count;
if (optionLabel != null)
{
count++;
}
// Short-circuit work below if there's nothing to add.
if (count == 0)
{
return HtmlString.Empty;
}
var listItemBuilder = new HtmlContentBuilder(count);
// Make optionLabel the first item that gets rendered.
if (optionLabel != null)
@ -1533,12 +1554,6 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures
currentValues: null));
}
var itemsList = selectList as IList<SelectListItem>;
if (itemsList == null)
{
itemsList = selectList.ToList();
}
// Group items in the SelectList if requested.
// The worst case complexity of this algorithm is O(number of groups*n).
// If there aren't any groups, it is O(n) where n is number of items in the list.

View File

@ -769,24 +769,21 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures
isChecked,
htmlAttributes);
var hiddenForCheckboxTag = _htmlGenerator.GenerateHiddenForCheckbox(ViewContext, modelExplorer, expression);
if (checkbox == null || hiddenForCheckboxTag == null)
var hiddenForCheckbox = _htmlGenerator.GenerateHiddenForCheckbox(ViewContext, modelExplorer, expression);
if (checkbox == null || hiddenForCheckbox == null)
{
return HtmlString.Empty;
}
var checkboxContent = new HtmlContentBuilder().AppendHtml(checkbox);
if (ViewContext.FormContext.CanRenderAtEndOfForm)
{
ViewContext.FormContext.EndOfFormContent.Add(hiddenForCheckboxTag);
}
else
{
checkboxContent.AppendHtml(hiddenForCheckboxTag);
ViewContext.FormContext.EndOfFormContent.Add(hiddenForCheckbox);
return checkbox;
}
return checkboxContent;
return new HtmlContentBuilder(capacity: 2)
.AppendHtml(checkbox)
.AppendHtml(hiddenForCheckbox);
}
protected virtual string GenerateDisplayName(ModelExplorer modelExplorer, string expression)

View File

@ -117,7 +117,10 @@ namespace Microsoft.AspNetCore.Mvc.Core.Rendering
[InlineData("attribute", "value", "<p attribute=\"HtmlEncode[[value]]\"></p>")]
[InlineData("attribute", null, "<p attribute=\"\"></p>")]
[InlineData("attribute", "", "<p attribute=\"\"></p>")]
public void WriteTo_WriteEmptyAttribute_WhenValueIsNullOrEmpty(string attributeKey, string attributeValue, string expectedOutput)
public void WriteTo_WriteEmptyAttribute_WhenValueIsNullOrEmpty(
string attributeKey,
string attributeValue,
string expectedOutput)
{
// Arrange
var tagBuilder = new TagBuilder("p");
@ -149,6 +152,22 @@ namespace Microsoft.AspNetCore.Mvc.Core.Rendering
// Assert
Assert.Equal("<p><span>Hello</span>HtmlEncode[[, World!]]</p>", writer.ToString());
}
Assert.True(tagBuilder.HasInnerHtml);
}
[Fact]
public void ReadingInnerHtml_LeavesHasInnerHtmlFalse()
{
// Arrange
var tagBuilder = new TagBuilder("p");
// Act
var innerHtml = tagBuilder.InnerHtml;
// Assert
Assert.False(tagBuilder.HasInnerHtml);
Assert.NotNull(innerHtml);
}
}
}