Reduce lookups needed in ConcurrentDictionaries in ContentNavigationServiceBase (#19603)

Reduce lookups needed in ConcurrentDictionaries and sort using List.Sort, make key removal O(1) by using hashsets and avoid duplicates, remove unneeded .ToList() and other minor tweaks

Author: Henr1k80

src/Umbraco.Core/Composing/TypeFinderConfig.cs

@@ -25,8 +25,8 @@ public IEnumerable<string> AssembliesAcceptingLoadExceptions
 
             var s = _settings.AssembliesAcceptingLoadExceptions;
             return _assembliesAcceptingLoadExceptions = string.IsNullOrWhiteSpace(s)
-                ? Array.Empty<string>()
-                : s.Split(',').Select(x => x.Trim()).ToArray();
+                ? []
+                : s.Split(',', StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries);
         }
     }
 }

src/Umbraco.Core/Extensions/StringExtensions.cs

@@ -509,8 +509,7 @@ public static Guid EncodeAsGuid(this string input)
         var convertToHex = input.ConvertToHex();
         var hexLength = convertToHex.Length < 32 ? convertToHex.Length : 32;
         var hex = convertToHex[..hexLength].PadLeft(32, '0');
-        Guid output = Guid.Empty;
-        return Guid.TryParse(hex, out output) ? output : Guid.Empty;
+        return Guid.TryParse(hex, out Guid output) ? output : Guid.Empty;
     }
 
     /// <summary>

src/Umbraco.Core/Factories/UserSettingsFactory.cs

@@ -36,7 +36,6 @@ private PasswordSettingsModel CreatePasswordSettingsModel() =>
 
     private IEnumerable<ConsentLevelModel> CreateConsentLevelModels() =>
         Enum.GetValues<TelemetryLevel>()
-            .ToList()
             .Select(level => new ConsentLevelModel
             {
                 Level = level,

src/Umbraco.Core/Logging/LoggingConfiguration.cs

@@ -58,8 +58,7 @@ public LoggingConfiguration(string logDirectory, string logFileNameFormat, strin
     public string LogFileNameFormat { get; }
 
     /// <inheritdoc/>
-    public string[] GetLogFileNameFormatArguments() => _logFileNameFormatArguments.Split(',', StringSplitOptions.RemoveEmptyEntries)
-        .Select(x => x.Trim())
+    public string[] GetLogFileNameFormatArguments() => _logFileNameFormatArguments.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries)
         .Select(GetValue)
         .ToArray();
 

src/Umbraco.Core/Models/Blocks/BlockEditorDataConverter.cs

@@ -71,12 +71,12 @@ public BlockEditorData<TValue, TLayout> Convert(TValue? value)
 
     // this method is only meant to have any effect when migrating block editor values
     // from the original format to the new, variant enabled format
-    private void AmendExpose(TValue value)
-        => value.Expose = value.ContentData.Select(cd => new BlockItemVariation(cd.Key, null, null)).ToList();
+    private static void AmendExpose(TValue value)
+        => value.Expose = value.ContentData.ConvertAll(cd => new BlockItemVariation(cd.Key, null, null));
 
     // this method is only meant to have any effect when migrating block editor values
     // from the original format to the new, variant enabled format
-    private bool ConvertOriginalBlockFormat(List<BlockItemData> blockItemDatas)
+    private static bool ConvertOriginalBlockFormat(List<BlockItemData> blockItemDatas)
     {
         var converted = false;
         foreach (BlockItemData blockItemData in blockItemDatas)

src/Umbraco.Core/Routing/DomainUtilities.cs

@@ -256,14 +256,14 @@ private static bool ContentHasAssignedDomains(IPublishedContent? content, IDomai
             // if a culture is specified, then try to get domains for that culture
             // (else cultureDomains will be null)
             // do NOT specify a default culture, else it would pick those domains
-            IReadOnlyCollection<DomainAndUri>? cultureDomains = SelectByCulture(domainsAndUris, culture, defaultCulture: null);
+            IReadOnlyList<DomainAndUri>? cultureDomains = SelectByCulture(domainsAndUris, culture, defaultCulture: null);
             IReadOnlyCollection<DomainAndUri> considerForBaseDomains = domainsAndUris;
             if (cultureDomains != null)
             {
                 if (cultureDomains.Count == 1)
                 {
                     // only 1, return
-                    return cultureDomains.First();
+                    return cultureDomains[0];
                 }
 
                 // else restrict to those domains, for base lookup
@@ -272,11 +272,11 @@ private static bool ContentHasAssignedDomains(IPublishedContent? content, IDomai
 
             // look for domains that would be the base of the uri
             // we need to order so example.com/foo matches before example.com/
-            IReadOnlyCollection<DomainAndUri> baseDomains = SelectByBase(considerForBaseDomains.OrderByDescending(d => d.Uri.ToString()).ToList(), uri, culture);
+            List<DomainAndUri> baseDomains = SelectByBase(considerForBaseDomains.OrderByDescending(d => d.Uri.ToString()).ToArray(), uri, culture);
             if (baseDomains.Count > 0)
             {
                 // found, return
-                return baseDomains.First();
+                return baseDomains[0];
             }
 
             // if nothing works, then try to run the filter to select a domain
@@ -296,7 +296,7 @@ private static bool IsBaseOf(DomainAndUri domain, Uri uri)
         private static bool MatchesCulture(DomainAndUri domain, string? culture)
             => culture == null || domain.Culture.InvariantEquals(culture);
 
-        private static IReadOnlyCollection<DomainAndUri> SelectByBase(IReadOnlyCollection<DomainAndUri> domainsAndUris, Uri uri, string? culture)
+        private static List<DomainAndUri> SelectByBase(DomainAndUri[] domainsAndUris, Uri uri, string? culture)
         {
             // look for domains that would be the base of the uri
             // ie current is www.example.com/foo/bar, look for domain www.example.com
@@ -314,7 +314,7 @@ private static IReadOnlyCollection<DomainAndUri> SelectByBase(IReadOnlyCollectio
             return baseDomains;
         }
 
-        private static IReadOnlyCollection<DomainAndUri>? SelectByCulture(IReadOnlyCollection<DomainAndUri> domainsAndUris, string? culture, string? defaultCulture)
+        private static List<DomainAndUri>? SelectByCulture(DomainAndUri[] domainsAndUris, string? culture, string? defaultCulture)
         {
             // we try our best to match cultures, but may end with a bogus domain
             if (culture is not null)
@@ -434,13 +434,18 @@ internal static bool ExistsDomainInPath(IEnumerable<Domain> domains, string path
 
         private static Domain? FindDomainInPath(IEnumerable<Domain>? domains, string path, int? rootNodeId, bool isWildcard)
         {
+            if (domains is null)
+            {
+                return null;
+            }
+
             var stopNodeId = rootNodeId ?? -1;
 
             return path.Split(Constants.CharArrays.Comma)
                        .Reverse()
                        .Select(s => int.Parse(s, CultureInfo.InvariantCulture))
                        .TakeWhile(id => id != stopNodeId)
-                       .Select(id => domains?.FirstOrDefault(d => d.ContentId == id && d.IsWildcard == isWildcard))
+                       .Select(id => domains.FirstOrDefault(d => d.ContentId == id && d.IsWildcard == isWildcard))
                        .FirstOrDefault(domain => domain is not null);
         }
 

src/Umbraco.Core/Services/Navigation/ContentNavigationServiceBase.cs

@@ -17,8 +17,8 @@ internal abstract class ContentNavigationServiceBase<TContentType, TContentTypeS
     private Lazy<Dictionary<string, Guid>> _contentTypeAliasToKeyMap;
     private ConcurrentDictionary<Guid, NavigationNode> _navigationStructure = new();
     private ConcurrentDictionary<Guid, NavigationNode> _recycleBinNavigationStructure = new();
-    private IList<Guid> _roots = new List<Guid>();
-    private IList<Guid> _recycleBinRoots = new List<Guid>();
+    private HashSet<Guid> _roots = [];
+    private HashSet<Guid> _recycleBinRoots = [];
 
     protected ContentNavigationServiceBase(ICoreScopeProvider coreScopeProvider, INavigationRepository navigationRepository, TContentTypeService typeService)
     {
@@ -321,7 +321,7 @@ protected Task HandleRebuildAsync(int readLock, Guid objectTypeKey, bool trashed
         return Task.CompletedTask;
     }
 
-    private bool TryGetParentKeyFromStructure(ConcurrentDictionary<Guid, NavigationNode> structure, Guid childKey, out Guid? parentKey)
+    private static bool TryGetParentKeyFromStructure(ConcurrentDictionary<Guid, NavigationNode> structure, Guid childKey, out Guid? parentKey)
     {
         if (structure.TryGetValue(childKey, out NavigationNode? childNode))
         {
@@ -335,25 +335,32 @@ private bool TryGetParentKeyFromStructure(ConcurrentDictionary<Guid, NavigationN
     }
 
     private bool TryGetRootKeysFromStructure(
-        IList<Guid> input,
+        HashSet<Guid> input,
         out IEnumerable<Guid> rootKeys,
         Guid? contentTypeKey = null)
     {
-        // Apply contentTypeKey filter
-        IEnumerable<Guid> filteredKeys = contentTypeKey.HasValue
-            ? input.Where(key => _navigationStructure[key].ContentTypeKey == contentTypeKey.Value)
-            : input;
+        var keysWithSortOrder = new List<(Guid Key, int SortOrder)>(input.Count);
+        foreach (Guid key in input)
+        {
+            NavigationNode navigationNode = _navigationStructure[key];
+
+            // Apply contentTypeKey filter
+            if (contentTypeKey.HasValue && navigationNode.ContentTypeKey != contentTypeKey.Value)
+            {
+                continue;
+            }
+
+            keysWithSortOrder.Add((key, navigationNode.SortOrder));
+        }
 
-        // TODO can we make this more efficient?
         // Sort by SortOrder
-        rootKeys = filteredKeys
-            .OrderBy(key => _navigationStructure[key].SortOrder)
-            .ToList();
+        keysWithSortOrder.Sort((a, b) => a.SortOrder.CompareTo(b.SortOrder));
+        rootKeys = keysWithSortOrder.ConvertAll(keyWithSortOrder => keyWithSortOrder.Key);
 
         return true;
     }
 
-    private bool TryGetChildrenKeysFromStructure(
+    private static bool TryGetChildrenKeysFromStructure(
         ConcurrentDictionary<Guid, NavigationNode> structure,
         Guid parentKey,
         out IEnumerable<Guid> childrenKeys,
@@ -367,12 +374,12 @@ private bool TryGetChildrenKeysFromStructure(
         }
 
         // Keep children keys ordered based on their SortOrder
-        childrenKeys = GetOrderedChildren(parentNode, structure, contentTypeKey).ToList();
+        childrenKeys = GetOrderedChildren(parentNode, structure, contentTypeKey);
 
         return true;
     }
 
-    private bool TryGetDescendantsKeysFromStructure(
+    private static bool TryGetDescendantsKeysFromStructure(
         ConcurrentDictionary<Guid, NavigationNode> structure,
         Guid parentKey,
         out IEnumerable<Guid> descendantsKeys,
@@ -393,7 +400,7 @@ private bool TryGetDescendantsKeysFromStructure(
         return true;
     }
 
-    private bool TryGetAncestorsKeysFromStructure(
+    private static bool TryGetAncestorsKeysFromStructure(
         ConcurrentDictionary<Guid, NavigationNode> structure,
         Guid childKey,
         out IEnumerable<Guid> ancestorsKeys,
@@ -421,7 +428,7 @@ private bool TryGetAncestorsKeysFromStructure(
         return true;
     }
 
-    private bool TryGetSiblingsKeysFromStructure(
+    private static bool TryGetSiblingsKeysFromStructure(
         ConcurrentDictionary<Guid, NavigationNode> structure,
         Guid key,
         out IEnumerable<Guid> siblingsKeys,
@@ -463,14 +470,14 @@ private bool TryGetSiblingsKeysFromStructure(
         return true;
     }
 
-    private void GetDescendantsRecursively(
+    private static void GetDescendantsRecursively(
         ConcurrentDictionary<Guid, NavigationNode> structure,
         NavigationNode node,
         List<Guid> descendants,
         Guid? contentTypeKey = null)
     {
         // Get all children regardless of contentType
-        var childrenKeys = GetOrderedChildren(node, structure).ToList();
+        IReadOnlyList<Guid> childrenKeys = GetOrderedChildren(node, structure);
         foreach (Guid childKey in childrenKeys)
         {
             // Apply contentTypeKey filter
@@ -487,7 +494,7 @@ private void GetDescendantsRecursively(
         }
     }
 
-    private bool TryRemoveNodeFromParentInStructure(ConcurrentDictionary<Guid, NavigationNode> structure, Guid key, out NavigationNode? nodeToRemove)
+    private static bool TryRemoveNodeFromParentInStructure(ConcurrentDictionary<Guid, NavigationNode> structure, Guid key, out NavigationNode? nodeToRemove)
     {
         if (structure.TryGetValue(key, out nodeToRemove) is false)
         {
@@ -507,7 +514,7 @@ private void AddDescendantsToRecycleBinRecursively(NavigationNode node)
     {
         _recycleBinRoots.Add(node.Key);
         _roots.Remove(node.Key);
-        var childrenKeys = GetOrderedChildren(node, _navigationStructure).ToList();
+        IReadOnlyList<Guid> childrenKeys = GetOrderedChildren(node, _navigationStructure);
 
         foreach (Guid childKey in childrenKeys)
         {
@@ -530,7 +537,7 @@ private void AddDescendantsToRecycleBinRecursively(NavigationNode node)
 
     private void RemoveDescendantsRecursively(NavigationNode node)
     {
-        var childrenKeys = GetOrderedChildren(node, _recycleBinNavigationStructure).ToList();
+        IReadOnlyList<Guid> childrenKeys = GetOrderedChildren(node, _recycleBinNavigationStructure);
         foreach (Guid childKey in childrenKeys)
         {
             if (_recycleBinNavigationStructure.TryGetValue(childKey, out NavigationNode? childNode) is false)
@@ -551,7 +558,7 @@ private void RestoreNodeAndDescendantsRecursively(NavigationNode node)
         }
 
         _recycleBinRoots.Remove(node.Key);
-        var childrenKeys = GetOrderedChildren(node, _recycleBinNavigationStructure).ToList();
+        IReadOnlyList<Guid> childrenKeys = GetOrderedChildren(node, _recycleBinNavigationStructure);
 
         foreach (Guid childKey in childrenKeys)
         {
@@ -570,24 +577,35 @@ private void RestoreNodeAndDescendantsRecursively(NavigationNode node)
         }
     }
 
-    private IEnumerable<Guid> GetOrderedChildren(
+    private static IReadOnlyList<Guid> GetOrderedChildren(
         NavigationNode node,
         ConcurrentDictionary<Guid, NavigationNode> structure,
         Guid? contentTypeKey = null)
     {
-        IEnumerable<Guid> children = node
-            .Children
-            .Where(structure.ContainsKey);
+        if (node.Children.Count < 1)
+        {
+            return [];
+        }
 
-        // Apply contentTypeKey filter
-        if (contentTypeKey.HasValue)
+        var childrenWithSortOrder = new List<(Guid ChildNodeKey, int SortOrder)>(node.Children.Count);
+        foreach (Guid childNodeKey in node.Children)
         {
-            children = children.Where(childKey => structure[childKey].ContentTypeKey == contentTypeKey.Value);
+            if (!structure.TryGetValue(childNodeKey, out NavigationNode? childNode))
+            {
+                continue;
+            }
+
+            // Apply contentTypeKey filter
+            if (contentTypeKey.HasValue && childNode.ContentTypeKey != contentTypeKey.Value)
+            {
+                continue;
+            }
+
+            childrenWithSortOrder.Add((childNodeKey, childNode.SortOrder));
         }
 
-        return children
-            .OrderBy(childKey => structure[childKey].SortOrder)
-            .ToList();
+        childrenWithSortOrder.Sort((a, b) => a.SortOrder.CompareTo(b.SortOrder));
+        return childrenWithSortOrder.ConvertAll(childWithSortOrder => childWithSortOrder.ChildNodeKey);
     }
 
     private bool TryGetContentTypeKey(string contentTypeAlias, out Guid? contentTypeKey)
@@ -613,7 +631,7 @@ private bool TryGetContentTypeKey(string contentTypeAlias, out Guid? contentType
         return true;
     }
 
-    private static void BuildNavigationDictionary(ConcurrentDictionary<Guid, NavigationNode> nodesStructure, IList<Guid> roots, IEnumerable<INavigationModel> entities)
+    private static void BuildNavigationDictionary(ConcurrentDictionary<Guid, NavigationNode> nodesStructure, HashSet<Guid> roots, IEnumerable<INavigationModel> entities)
     {
         var entityList = entities.ToList();
         var idToKeyMap = entityList.ToDictionary(x => x.Id, x => x.Key);

src/Umbraco.Core/Strings/Css/StylesheetHelper.cs

@@ -39,8 +39,7 @@ public static IEnumerable<StylesheetRule> ParseRules(string? input)
                     // Only match first selector when chained together
                     Styles = string.Join(
                         Environment.NewLine,
-                        match.Groups["Styles"].Value.Split(new[] { "\r\n", "\n" }, StringSplitOptions.None)
-                            .Select(x => x.Trim()).ToArray()),
+                        match.Groups["Styles"].Value.Split(new[] { "\r\n", "\n" }, StringSplitOptions.TrimEntries)),
                 });
             }
         }

src/Umbraco.Core/Strings/Css/StylesheetRule.cs

@@ -30,13 +30,13 @@ public override string ToString()
             // instead of using string interpolation (for increased performance)
             foreach (var style in
                      Styles?.Split(Constants.CharArrays.Semicolon, StringSplitOptions.RemoveEmptyEntries) ??
-                     Array.Empty<string>())
+                     [])
             {
-                sb.Append("\t").Append(style.StripNewLines().Trim()).Append(";").Append(Environment.NewLine);
+                sb.Append('\t').Append(style.StripNewLines().Trim()).Append(';').Append(Environment.NewLine);
             }
         }
 
-        sb.Append("}");
+        sb.Append('}');
 
         return sb.ToString();
     }

src/Umbraco.Web.Website/Models/RegisterModelBuilder.cs

@@ -68,7 +68,7 @@ public RegisterModel Build()
             UsernameIsEmail = _usernameIsEmail,
             MemberProperties = _lookupProperties
                 ? GetMemberPropertiesViewModel(memberType)
-                : Enumerable.Empty<MemberPropertyModel>().ToList(),
+                : [],
             AutomaticLogIn = _automaticLogIn
         };
         return model;