diff --git a/OpenRA.Game/Exts.cs b/OpenRA.Game/Exts.cs index 8b8f577b87..2a9ae19272 100644 --- a/OpenRA.Game/Exts.cs +++ b/OpenRA.Game/Exts.cs @@ -14,6 +14,7 @@ using System.Collections.Generic; using System.Globalization; using System.Linq; using System.Reflection; +using System.Text; using OpenRA.Primitives; using OpenRA.Support; using OpenRA.Traits; @@ -432,15 +433,26 @@ namespace OpenRA public static Dictionary ToDictionaryWithConflictLog( this IEnumerable source, Func keySelector, Func elementSelector, string debugName, Func logKey = null, Func logValue = null) + { + var output = new Dictionary(); + IntoDictionaryWithConflictLog(source, keySelector, elementSelector, debugName, output, logKey, logValue); + return output; + } + + public static void IntoDictionaryWithConflictLog( + this IEnumerable source, Func keySelector, Func elementSelector, + string debugName, Dictionary output, + Func logKey = null, Func logValue = null) { // Fall back on ToString() if null functions are provided: logKey ??= s => s.ToString(); logValue ??= s => s.ToString(); // Try to build a dictionary and log all duplicates found (if any): - var dupKeys = new Dictionary>(); + Dictionary> dupKeys = null; var capacity = source is ICollection collection ? collection.Count : 0; - var d = new Dictionary(capacity); + output.Clear(); + output.EnsureCapacity(capacity); foreach (var item in source) { var key = keySelector(item); @@ -451,14 +463,15 @@ namespace OpenRA continue; // Check for a key conflict: - if (!d.TryAdd(key, element)) + if (!output.TryAdd(key, element)) { + dupKeys ??= new Dictionary>(); if (!dupKeys.TryGetValue(key, out var dupKeyMessages)) { // Log the initial conflicting value already inserted: dupKeyMessages = new List { - logValue(d[key]) + logValue(output[key]) }; dupKeys.Add(key, dupKeyMessages); } @@ -469,15 +482,14 @@ namespace OpenRA } // If any duplicates were found, throw a descriptive error - if (dupKeys.Count > 0) + if (dupKeys != null) { - var badKeysFormatted = string.Join(", ", dupKeys.Select(p => $"{logKey(p.Key)}: [{string.Join(",", p.Value)}]")); - var msg = $"{debugName}, duplicate values found for the following keys: {badKeysFormatted}"; - throw new ArgumentException(msg); + var badKeysFormatted = new StringBuilder( + $"{debugName}, duplicate values found for the following keys: "); + foreach (var p in dupKeys) + badKeysFormatted.Append($"{logKey(p.Key)}: [{string.Join(",", p.Value)}]"); + throw new ArgumentException(badKeysFormatted.ToString()); } - - // Return the dictionary we built: - return d; } public static Color ColorLerp(float t, Color c1, Color c2) diff --git a/OpenRA.Game/MiniYaml.cs b/OpenRA.Game/MiniYaml.cs index 63613fef36..2a75a65104 100644 --- a/OpenRA.Game/MiniYaml.cs +++ b/OpenRA.Game/MiniYaml.cs @@ -115,6 +115,7 @@ namespace OpenRA const int SpacesPerLevel = 4; static readonly Func StringIdentity = s => s; static readonly Func MiniYamlIdentity = my => my; + static readonly Dictionary ConflictScratch = new(); public readonly string Value; public readonly ImmutableArray Nodes; @@ -419,7 +420,8 @@ namespace OpenRA // Resolve any top-level removals (e.g. removing whole actor blocks) var nodes = new MiniYaml("", resolved.Select(kv => new MiniYamlNode(kv.Key, kv.Value))); - return ResolveInherits(nodes, tree, ImmutableDictionary.Empty); + var result = ResolveInherits(nodes, tree, ImmutableDictionary.Empty); + return result as List ?? result.ToList(); } static void MergeIntoResolved(MiniYamlNode overrideNode, List existingNodes, HashSet existingNodeKeys, @@ -444,9 +446,12 @@ namespace OpenRA existingNodes.Add(overrideNode.WithValue(value)); } - static List ResolveInherits( + static IReadOnlyCollection ResolveInherits( MiniYaml node, Dictionary tree, ImmutableDictionary inherited) { + if (node.Nodes.Length == 0) + return node.Nodes; + var resolved = new List(node.Nodes.Length); var resolvedKeys = new HashSet(node.Nodes.Length); @@ -491,6 +496,9 @@ namespace OpenRA /// static IReadOnlyCollection MergeSelfPartial(IReadOnlyCollection existingNodes) { + if (existingNodes.Count == 0) + return existingNodes; + var keys = new HashSet(existingNodes.Count); var ret = new List(existingNodes.Count); foreach (var n in existingNodes) @@ -511,8 +519,15 @@ namespace OpenRA static MiniYaml MergePartial(MiniYaml existingNodes, MiniYaml overrideNodes) { - existingNodes?.Nodes.ToDictionaryWithConflictLog(x => x.Key, "MiniYaml.Merge", null, x => $"{x.Key} (at {x.Location})"); - overrideNodes?.Nodes.ToDictionaryWithConflictLog(x => x.Key, "MiniYaml.Merge", null, x => $"{x.Key} (at {x.Location})"); + lock (ConflictScratch) + { + // PERF: Reuse ConflictScratch for all conflict checks to avoid allocations. + existingNodes?.Nodes.IntoDictionaryWithConflictLog( + n => n.Key, n => n, "MiniYaml.Merge", ConflictScratch, k => k, n => $"{n.Key} (at {n.Location})"); + overrideNodes?.Nodes.IntoDictionaryWithConflictLog( + n => n.Key, n => n, "MiniYaml.Merge", ConflictScratch, k => k, n => $"{n.Key} (at {n.Location})"); + ConflictScratch.Clear(); + } if (existingNodes == null) return overrideNodes;