From cba4d1dd0c8938cdd4d7504d5d7f0d17c30a3e01 Mon Sep 17 00:00:00 2001 From: mnltejaswini Date: Wed, 27 Apr 2016 14:34:51 -0700 Subject: [PATCH] [Perf] Reduce SelectListItem and other allocations when generating HTML for select lists Fixes #3953 --- .../Rendering/MultiSelectList.cs | 76 +++++++--- .../ViewFeatures/DefaultHtmlGenerator.cs | 139 +++++++++--------- 2 files changed, 126 insertions(+), 89 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Rendering/MultiSelectList.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Rendering/MultiSelectList.cs index bbeb03b392..d09a9bc768 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Rendering/MultiSelectList.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Rendering/MultiSelectList.cs @@ -18,6 +18,7 @@ namespace Microsoft.AspNetCore.Mvc.Rendering public class MultiSelectList : IEnumerable { private IList _groups; + private IList _selectListItems; public MultiSelectList(IEnumerable items) : this(items, selectedValues: null) @@ -111,10 +112,15 @@ namespace Microsoft.AspNetCore.Mvc.Rendering public virtual IEnumerator GetEnumerator() { - return GetListItems().GetEnumerator(); + if (_selectListItems == null) + { + _selectListItems = GetListItems(); + } + + return _selectListItems.GetEnumerator(); } - internal IList GetListItems() + private IList GetListItems() { return (!string.IsNullOrEmpty(DataValueField)) ? GetListItemsWithValueField() : @@ -126,20 +132,29 @@ namespace Microsoft.AspNetCore.Mvc.Rendering var selectedValues = new HashSet(StringComparer.OrdinalIgnoreCase); if (SelectedValues != null) { - selectedValues.UnionWith(from object value in SelectedValues - select Convert.ToString(value, CultureInfo.CurrentCulture)); + foreach (var value in SelectedValues) + { + var stringValue = Convert.ToString(value, CultureInfo.CurrentCulture); + selectedValues.Add(stringValue); + } } - var listItems = from object item in Items - let value = Eval(item, DataValueField) - select new SelectListItem - { - Group = GetGroup(item), - Value = value, - Text = Eval(item, DataTextField), - Selected = selectedValues.Contains(value) - }; - return listItems.ToList(); + var listItems = new List(); + foreach (var item in Items) + { + var value = Eval(item, DataValueField); + var newListItem = new SelectListItem + { + Group = GetGroup(item), + Value = value, + Text = Eval(item, DataTextField), + Selected = selectedValues.Contains(value), + }; + + listItems.Add(newListItem); + } + + return listItems; } private IList GetListItemsWithoutValueField() @@ -150,14 +165,20 @@ namespace Microsoft.AspNetCore.Mvc.Rendering selectedValues.UnionWith(SelectedValues.Cast()); } - var listItems = from object item in Items - select new SelectListItem - { - Group = GetGroup(item), - Text = Eval(item, DataTextField), - Selected = selectedValues.Contains(item) - }; - return listItems.ToList(); + var listItems = new List(); + foreach (var item in Items) + { + var newListItem = new SelectListItem + { + Group = GetGroup(item), + Text = Eval(item, DataTextField), + Selected = selectedValues.Contains(item), + }; + + listItems.Add(newListItem); + } + + return listItems; } private static string Eval(object container, string expression) @@ -187,7 +208,16 @@ namespace Microsoft.AspNetCore.Mvc.Rendering // We use StringComparison.CurrentCulture because the group name is used to display as the value of // optgroup HTML tag's label attribute. - var group = _groups.FirstOrDefault(g => string.Equals(g.Name, groupName, StringComparison.CurrentCulture)); + SelectListGroup group = null; + for (var index = 0; index < _groups.Count; index++) + { + if (string.Equals(_groups[index].Name, groupName, StringComparison.CurrentCulture)) + { + group = _groups[index]; + break; + } + } + if (group == null) { group = new SelectListGroup() { Name = groupName }; diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/DefaultHtmlGenerator.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/DefaultHtmlGenerator.cs index a76897ab13..31666b43ab 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/DefaultHtmlGenerator.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/DefaultHtmlGenerator.cs @@ -556,13 +556,9 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures modelExplorer = modelExplorer ?? ExpressionMetadataProvider.FromStringExpression(expression, viewContext.ViewData, _metadataProvider); - if (currentValues != null) - { - selectList = UpdateSelectListItemsWithDefaultValue(modelExplorer, selectList, currentValues); - } // Convert each ListItem to an if requested. - var listItemBuilder = GenerateGroupsAndOptions(optionLabel, selectList); + var listItemBuilder = GenerateGroupsAndOptions(optionLabel, selectList, currentValues); var tagBuilder = new TagBuilder("select"); tagBuilder.InnerHtml.SetHtmlContent(listItemBuilder); @@ -1032,6 +1028,11 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures /// Not used directly in HtmlHelper. Exposed for use in DefaultDisplayTemplates. /// internal static TagBuilder GenerateOption(SelectListItem item, string text) + { + return GenerateOption(item, text, item.Selected); + } + + internal static TagBuilder GenerateOption(SelectListItem item, string text, bool selected) { var tagBuilder = new TagBuilder("option"); tagBuilder.InnerHtml.SetContent(text); @@ -1041,7 +1042,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures tagBuilder.Attributes["value"] = item.Value; } - if (item.Selected) + if (selected) { tagBuilder.Attributes["selected"] = "selected"; } @@ -1433,82 +1434,81 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures return selectList; } - private static IEnumerable UpdateSelectListItemsWithDefaultValue( - ModelExplorer modelExplorer, - IEnumerable selectList, - ICollection currentValues) - { - // Perform deep copy of selectList to avoid changing user's Selected property values. - var newSelectList = new List(); - foreach (SelectListItem item in selectList) - { - var value = item.Value ?? item.Text; - var selected = currentValues.Contains(value); - var copy = new SelectListItem - { - Disabled = item.Disabled, - Group = item.Group, - Selected = selected, - Text = item.Text, - Value = item.Value, - }; - - newSelectList.Add(copy); - } - - return newSelectList; - } - /// public IHtmlContent GenerateGroupsAndOptions(string optionLabel, IEnumerable selectList) + { + return GenerateGroupsAndOptions(optionLabel: optionLabel, selectList: selectList, currentValues: null); + } + + private IHtmlContent GenerateGroupsAndOptions( + string optionLabel, + IEnumerable selectList, + ICollection currentValues) { var listItemBuilder = new HtmlContentBuilder(); // Make optionLabel the first item that gets rendered. if (optionLabel != null) { - listItemBuilder.AppendLine(GenerateOption(new SelectListItem() - { - Text = optionLabel, - Value = string.Empty, - Selected = false, - })); + listItemBuilder.AppendLine(GenerateOption( + new SelectListItem() + { + Text = optionLabel, + Value = string.Empty, + Selected = false, + }, + currentValues: null)); + } + + var itemsList = selectList as IList; + if (itemsList == null) + { + itemsList = selectList.ToList(); } // Group items in the SelectList if requested. - // Treat each item with Group == null as a member of a unique group - // so they are added according to the original order. - var groupedSelectList = selectList.GroupBy( - item => (item.Group == null) ? item.GetHashCode() : item.Group.GetHashCode()); - foreach (var group in groupedSelectList) + // 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. + var optionGenerated = new bool[itemsList.Count]; + for (var i = 0; i < itemsList.Count; i++) { - var optGroup = group.First().Group; - if (optGroup != null) + if (!optionGenerated[i]) { - var groupBuilder = new TagBuilder("optgroup"); - if (optGroup.Name != null) + var item = itemsList[i]; + var optGroup = item.Group; + if (optGroup != null) { - groupBuilder.MergeAttribute("label", optGroup.Name); - } + var groupBuilder = new TagBuilder("optgroup"); + if (optGroup.Name != null) + { + groupBuilder.MergeAttribute("label", optGroup.Name); + } - if (optGroup.Disabled) - { - groupBuilder.MergeAttribute("disabled", "disabled"); - } + if (optGroup.Disabled) + { + groupBuilder.MergeAttribute("disabled", "disabled"); + } - groupBuilder.InnerHtml.AppendLine(); - foreach (var item in group) - { - groupBuilder.InnerHtml.AppendLine(GenerateOption(item)); - } + groupBuilder.InnerHtml.AppendLine(); - listItemBuilder.AppendLine(groupBuilder); - } - else - { - foreach (var item in group) + for (var j = i; j < itemsList.Count; j++) + { + var groupItem = itemsList[j]; + + if (!optionGenerated[j] && + object.ReferenceEquals(optGroup, groupItem.Group)) + { + groupBuilder.InnerHtml.AppendLine(GenerateOption(groupItem, currentValues)); + optionGenerated[j] = true; + } + } + + listItemBuilder.AppendLine(groupBuilder); + } + else { - listItemBuilder.AppendLine(GenerateOption(item)); + listItemBuilder.AppendLine(GenerateOption(item, currentValues)); + optionGenerated[i] = true; } } } @@ -1516,9 +1516,16 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures return listItemBuilder; } - private IHtmlContent GenerateOption(SelectListItem item) + private IHtmlContent GenerateOption(SelectListItem item, ICollection currentValues) { - var tagBuilder = GenerateOption(item, item.Text); + var selected = item.Selected; + if (currentValues != null) + { + var value = item.Value ?? item.Text; + selected = currentValues.Contains(value); + } + + var tagBuilder = GenerateOption(item, item.Text, selected); return tagBuilder; } }