Skip to content

Commit

Permalink
Improve lookups of nodes by key in MiniYaml.
Browse files Browse the repository at this point in the history
When handling the Nodes collection in MiniYaml, individual nodes are located via one of two methods:

// Lookup a single key with linear search.
var node = yaml.Nodes.FirstOrDefault(n => n.Key == "SomeKey");

// Convert to dictionary, expecting many key lookups.
var dict = nodes.ToDictionary();

// Lookup a single key in the dictionary.
var node = dict["SomeKey"];

To simplify lookup of individual keys via linear search, provide helper methods NodeWithKeyOrDefault and NodeWithKey. These helpers do the equivalent of Single{OrDefault} searches. Whilst this requires checking the whole list, it provides a useful correctness check. Two duplicated keys in TS yaml are fixed as a result. We can also optimize the helpers to not use LINQ, avoiding allocation of the delegate to search for a key.

Adjust existing code to use either lnear searches or dictionary lookups based on whether it will be resolving many keys. Resolving few keys can be done with linear searches to avoid building a dictionary. Resolving many keys should be done with a dictionary to avoid quaradtic runtime from repeated linear searches.
  • Loading branch information
RoosterDragon committed Aug 26, 2023
1 parent d9787b1 commit 910f77c
Show file tree
Hide file tree
Showing 60 changed files with 194 additions and 194 deletions.
6 changes: 3 additions & 3 deletions OpenRA.Game/ExternalMods.cs
Original file line number Diff line number Diff line change
Expand Up @@ -94,17 +94,17 @@ void LoadMod(MiniYaml yaml, string path = null, bool forceRegistration = false)

if (sheetBuilder != null)
{
var iconNode = yaml.Nodes.FirstOrDefault(n => n.Key == "Icon");
var iconNode = yaml.NodeWithKeyOrDefault("Icon");
if (iconNode != null && !string.IsNullOrEmpty(iconNode.Value.Value))
using (var stream = new MemoryStream(Convert.FromBase64String(iconNode.Value.Value)))
mod.Icon = sheetBuilder.Add(new Png(stream));

var icon2xNode = yaml.Nodes.FirstOrDefault(n => n.Key == "Icon2x");
var icon2xNode = yaml.NodeWithKeyOrDefault("Icon2x");
if (icon2xNode != null && !string.IsNullOrEmpty(icon2xNode.Value.Value))
using (var stream = new MemoryStream(Convert.FromBase64String(icon2xNode.Value.Value)))
mod.Icon2x = sheetBuilder.Add(new Png(stream), 1f / 2);

var icon3xNode = yaml.Nodes.FirstOrDefault(n => n.Key == "Icon3x");
var icon3xNode = yaml.NodeWithKeyOrDefault("Icon3x");
if (icon3xNode != null && !string.IsNullOrEmpty(icon3xNode.Value.Value))
using (var stream = new MemoryStream(Convert.FromBase64String(icon3xNode.Value.Value)))
mod.Icon3x = sheetBuilder.Add(new Png(stream), 1f / 3);
Expand Down
4 changes: 2 additions & 2 deletions OpenRA.Game/GameRules/SoundInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,12 @@ public SoundInfo(MiniYaml y)
foreach (var t in classifiction.Value.Nodes)
{
var volumeModifier = 1f;
var volumeModifierNode = t.Value.Nodes.FirstOrDefault(x => x.Key == nameof(SoundPool.VolumeModifier));
var volumeModifierNode = t.Value.NodeWithKeyOrDefault(nameof(SoundPool.VolumeModifier));
if (volumeModifierNode != null)
volumeModifier = FieldLoader.GetValue<float>(volumeModifierNode.Key, volumeModifierNode.Value.Value);

var interruptType = SoundPool.DefaultInterruptType;
var interruptTypeNode = t.Value.Nodes.FirstOrDefault(x => x.Key == nameof(SoundPool.InterruptType));
var interruptTypeNode = t.Value.NodeWithKeyOrDefault(nameof(SoundPool.InterruptType));
if (interruptTypeNode != null)
interruptType = FieldLoader.GetValue<SoundPool.InterruptType>(interruptTypeNode.Key, interruptTypeNode.Value.Value);

Expand Down
3 changes: 2 additions & 1 deletion OpenRA.Game/GameRules/WeaponInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,8 @@ public WeaponInfo(MiniYaml content)

static object LoadProjectile(MiniYaml yaml)
{
if (!yaml.ToDictionary().TryGetValue("Projectile", out var proj))
var proj = yaml.NodeWithKeyOrDefault("Projectile")?.Value;
if (proj == null)
return null;

var ret = Game.CreateObject<IProjectileInfo>(proj.Value + "Info");
Expand Down
3 changes: 1 addition & 2 deletions OpenRA.Game/GameSpeed.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#endregion

using System.Collections.Generic;
using System.Linq;

namespace OpenRA
{
Expand Down Expand Up @@ -38,7 +37,7 @@ public class GameSpeeds : IGlobalModData
static object LoadSpeeds(MiniYaml y)
{
var ret = new Dictionary<string, GameSpeed>();
var speedsNode = y.Nodes.FirstOrDefault(n => n.Key == "Speeds");
var speedsNode = y.NodeWithKeyOrDefault("Speeds");
if (speedsNode == null)
throw new YamlException("Error parsing GameSpeeds: Missing Speeds node!");

Expand Down
6 changes: 3 additions & 3 deletions OpenRA.Game/Graphics/CursorProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,22 @@ public CursorProvider(ModData modData)
var sequenceYaml = MiniYaml.Merge(modData.Manifest.Cursors.Select(
s => MiniYaml.FromStream(fileSystem.Open(s), s)));

var nodesDict = new MiniYaml(null, sequenceYaml).ToDictionary();
var cursorsYaml = new MiniYaml(null, sequenceYaml).NodeWithKey("Cursors").Value;

// Overwrite previous definitions if there are duplicates
var pals = new Dictionary<string, IProvidesCursorPaletteInfo>();
foreach (var p in modData.DefaultRules.Actors[SystemActors.World].TraitInfos<IProvidesCursorPaletteInfo>())
if (p.Palette != null)
pals[p.Palette] = p;

Palettes = nodesDict["Cursors"].Nodes.Select(n => n.Value.Value)
Palettes = cursorsYaml.Nodes.Select(n => n.Value.Value)
.Where(p => p != null)
.Distinct()
.ToDictionary(p => p, p => pals[p].ReadPalette(modData.DefaultFileSystem));

var frameCache = new FrameCache(fileSystem, modData.SpriteLoaders);
var cursors = new Dictionary<string, CursorSequence>();
foreach (var s in nodesDict["Cursors"].Nodes)
foreach (var s in cursorsYaml.Nodes)
foreach (var sequence in s.Value.Nodes)
cursors.Add(sequence.Key, new CursorSequence(frameCache, sequence.Key, s.Key, s.Value.Value, sequence.Value));

Expand Down
28 changes: 12 additions & 16 deletions OpenRA.Game/HotkeyDefinition.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#endregion

using System.Collections.Generic;
using System.Linq;

namespace OpenRA
{
Expand All @@ -31,29 +30,26 @@ public HotkeyDefinition(string name, MiniYaml node)
if (!string.IsNullOrEmpty(node.Value))
Default = FieldLoader.GetValue<Hotkey>("value", node.Value);

var descriptionNode = node.Nodes.FirstOrDefault(n => n.Key == "Description");
if (descriptionNode != null)
Description = descriptionNode.Value.Value;
var nodeDict = node.ToDictionary();

var typesNode = node.Nodes.FirstOrDefault(n => n.Key == "Types");
if (typesNode != null)
Types = FieldLoader.GetValue<HashSet<string>>("Types", typesNode.Value.Value);
if (nodeDict.TryGetValue("Description", out var descriptionYaml))
Description = descriptionYaml.Value;

var contextsNode = node.Nodes.FirstOrDefault(n => n.Key == "Contexts");
if (contextsNode != null)
Contexts = FieldLoader.GetValue<HashSet<string>>("Contexts", contextsNode.Value.Value);
if (nodeDict.TryGetValue("Types", out var typesYaml))
Types = FieldLoader.GetValue<HashSet<string>>("Types", typesYaml.Value);

var platformNode = node.Nodes.FirstOrDefault(n => n.Key == "Platform");
if (platformNode != null)
if (nodeDict.TryGetValue("Contexts", out var contextYaml))
Contexts = FieldLoader.GetValue<HashSet<string>>("Contexts", contextYaml.Value);

if (nodeDict.TryGetValue("Platform", out var platformYaml))
{
var platformOverride = platformNode.Value.Nodes.FirstOrDefault(n => n.Key == Platform.CurrentPlatform.ToString());
var platformOverride = platformYaml.NodeWithKeyOrDefault(Platform.CurrentPlatform.ToString());
if (platformOverride != null)
Default = FieldLoader.GetValue<Hotkey>("value", platformOverride.Value.Value);
}

var readonlyNode = node.Nodes.FirstOrDefault(n => n.Key == "Readonly");
if (readonlyNode != null)
Readonly = FieldLoader.GetValue<bool>("Readonly", readonlyNode.Value.Value);
if (nodeDict.TryGetValue("Readonly", out var readonlyYaml))
Readonly = FieldLoader.GetValue<bool>("Readonly", readonlyYaml.Value);
}
}
}
2 changes: 1 addition & 1 deletion OpenRA.Game/Manifest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ static string[] YamlList(Dictionary<string, MiniYaml> yaml, string key)
if (!yaml.ContainsKey(key))
return Array.Empty<string>();

return yaml[key].ToDictionary().Keys.ToArray();
return yaml[key].Nodes.Select(n => n.Key).ToArray();
}

static IReadOnlyDictionary<string, string> YamlDictionary(Dictionary<string, MiniYaml> yaml, string key)
Expand Down
10 changes: 5 additions & 5 deletions OpenRA.Game/Map/Map.cs
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,9 @@ public MapField(string key, string fieldName = null, bool required = true, strin
t == typeof(MiniYaml) ? Type.MiniYaml : Type.Normal;
}

public void Deserialize(Map map, ImmutableArray<MiniYamlNode> nodes)
public void Deserialize(Map map, MiniYaml yaml)
{
var node = nodes.FirstOrDefault(n => n.Key == key);
var node = yaml.NodeWithKeyOrDefault(key);
if (node == null)
{
if (required)
Expand Down Expand Up @@ -363,13 +363,13 @@ public Map(ModData modData, IReadOnlyPackage package)

var yaml = new MiniYaml(null, MiniYaml.FromStream(Package.GetStream("map.yaml"), package.Name));
foreach (var field in YamlFields)
field.Deserialize(this, yaml.Nodes);
field.Deserialize(this, yaml);

if (MapFormat < SupportedMapFormat)
throw new InvalidDataException($"Map format {MapFormat} is not supported.\n File: {package.Name}");

PlayerDefinitions = MiniYaml.NodesOrEmpty(yaml, "Players");
ActorDefinitions = MiniYaml.NodesOrEmpty(yaml, "Actors");
PlayerDefinitions = yaml.NodeWithKeyOrDefault("Players")?.Value.Nodes ?? ImmutableArray<MiniYamlNode>.Empty;
ActorDefinitions = yaml.NodeWithKeyOrDefault("Actors")?.Value.Nodes ?? ImmutableArray<MiniYamlNode>.Empty;

Grid = modData.Manifest.Get<MapGrid>();

Expand Down
38 changes: 33 additions & 5 deletions OpenRA.Game/MiniYaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,34 @@ public MiniYaml WithNodesAppended(IEnumerable<MiniYamlNode> nodes)
return new MiniYaml(Value, newNodes);
}

public MiniYamlNode NodeWithKey(string key)
{
var result = NodeWithKeyOrDefault(key);
if (result == null)
throw new InvalidDataException($"No node with key '{key}'");
return result;
}

public MiniYamlNode NodeWithKeyOrDefault(string key)
{
// PERF: Avoid LINQ.
var first = true;
MiniYamlNode result = null;
foreach (var node in Nodes)
{
if (node.Key != key)
continue;

if (!first)
throw new InvalidDataException($"Duplicate key '{node.Key}' in {node.Location}");

first = false;
result = node;
}

return result;
}

public Dictionary<string, MiniYaml> ToDictionary()
{
return ToDictionary(MiniYamlIdentity);
Expand Down Expand Up @@ -175,11 +203,6 @@ public MiniYaml(string value, IEnumerable<MiniYamlNode> nodes)
Nodes = ImmutableArray.CreateRange(nodes);
}

public static ImmutableArray<MiniYamlNode> NodesOrEmpty(MiniYaml y, string s)
{
return y.Nodes.FirstOrDefault(n => n.Key == s)?.Value.Nodes ?? ImmutableArray<MiniYamlNode>.Empty;
}

static List<MiniYamlNode> FromLines(IEnumerable<ReadOnlyMemory<char>> lines, string filename, bool discardCommentsAndWhitespace, Dictionary<string, string> stringPool)
{
stringPool ??= new Dictionary<string, string>();
Expand Down Expand Up @@ -668,6 +691,11 @@ yield return (hasKey ? key + ":" : "")
foreach (var line in Nodes.ToLines())
yield return "\t" + line;
}

public MiniYamlNodeBuilder NodeWithKeyOrDefault(string key)
{
return Nodes.SingleOrDefault(n => n.Key == key);
}
}

[Serializable]
Expand Down
4 changes: 2 additions & 2 deletions OpenRA.Game/Network/GameServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public class GameServer
static object LoadClients(MiniYaml yaml)
{
var clients = new List<GameClient>();
var clientsNode = yaml.Nodes.FirstOrDefault(n => n.Key == "Clients");
var clientsNode = yaml.NodeWithKeyOrDefault("Clients");
if (clientsNode != null)
{
var regex = new Regex(@"Client@\d+");
Expand All @@ -159,7 +159,7 @@ public GameServer(MiniYaml yaml)
// Games advertised using the old API used a single Mods field
if (Mod == null || Version == null)
{
var modsNode = yaml.Nodes.FirstOrDefault(n => n.Key == "Mods");
var modsNode = yaml.NodeWithKeyOrDefault("Mods");
if (modsNode != null)
{
var modVersion = modsNode.Value.Value.Split('@');
Expand Down
2 changes: 1 addition & 1 deletion OpenRA.Game/Network/LocalizedMessage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public class LocalizedMessage
static object LoadArguments(MiniYaml yaml)
{
var arguments = new Dictionary<string, object>();
var argumentsNode = yaml.Nodes.FirstOrDefault(n => n.Key == "Arguments");
var argumentsNode = yaml.NodeWithKeyOrDefault("Arguments");
if (argumentsNode != null)
{
foreach (var argumentNode in argumentsNode.Value.Nodes)
Expand Down
2 changes: 1 addition & 1 deletion OpenRA.Game/Network/Session.cs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ public static Global Deserialize(MiniYaml data)
{
var gs = FieldLoader.Load<Global>(data);

var optionsNode = data.Nodes.FirstOrDefault(n => n.Key == "Options");
var optionsNode = data.NodeWithKeyOrDefault("Options");
if (optionsNode != null)
foreach (var n in optionsNode.Value.Nodes)
gs.LobbyOptions[n.Key] = FieldLoader.Load<LobbyOptionState>(n.Value);
Expand Down
9 changes: 4 additions & 5 deletions OpenRA.Game/PlayerDatabase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
*/
#endregion

using System.Linq;
using System.Threading.Tasks;
using OpenRA.FileFormats;
using OpenRA.Graphics;
Expand Down Expand Up @@ -94,10 +93,10 @@ public PlayerBadge LoadBadge(MiniYaml yaml)
});
}

var labelNode = yaml.Nodes.FirstOrDefault(n => n.Key == "Label");
var icon24Node = yaml.Nodes.FirstOrDefault(n => n.Key == "Icon24");
var icon48Node = yaml.Nodes.FirstOrDefault(n => n.Key == "Icon48");
var icon72Node = yaml.Nodes.FirstOrDefault(n => n.Key == "Icon72");
var labelNode = yaml.NodeWithKeyOrDefault("Label");
var icon24Node = yaml.NodeWithKeyOrDefault("Icon24");
var icon48Node = yaml.NodeWithKeyOrDefault("Icon48");
var icon72Node = yaml.NodeWithKeyOrDefault("Icon72");
if (labelNode == null)
return null;

Expand Down
3 changes: 1 addition & 2 deletions OpenRA.Game/PlayerProfile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#endregion

using System.Collections.Generic;
using System.Linq;

namespace OpenRA
{
Expand All @@ -31,7 +30,7 @@ static object LoadBadges(MiniYaml yaml)
{
var badges = new List<PlayerBadge>();

var badgesNode = yaml.Nodes.FirstOrDefault(n => n.Key == "Badges");
var badgesNode = yaml.NodeWithKeyOrDefault("Badges");
if (badgesNode != null)
{
var playerDatabase = Game.ModData.Manifest.Get<PlayerDatabase>();
Expand Down
2 changes: 1 addition & 1 deletion OpenRA.Game/Settings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ public void Save()
else
{
// Update or add the custom value
var fieldYaml = sectionYaml.Value.Nodes.FirstOrDefault(n => n.Key == fli.YamlName);
var fieldYaml = sectionYaml.Value.NodeWithKeyOrDefault(fli.YamlName);
if (fieldYaml != null)
fieldYaml.Value.Value = serialized;
else
Expand Down
3 changes: 1 addition & 2 deletions OpenRA.Game/Traits/TraitsInterfaces.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Collections.ObjectModel;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
Expand Down Expand Up @@ -350,7 +349,7 @@ public interface INotifyGameSaved { void GameSaved(World w); }
public interface IGameSaveTraitData
{
List<MiniYamlNode> IssueTraitData(Actor self);
void ResolveTraitData(Actor self, ImmutableArray<MiniYamlNode> data);
void ResolveTraitData(Actor self, MiniYaml data);
}

[RequireExplicitImplementation]
Expand Down
2 changes: 1 addition & 1 deletion OpenRA.Game/Widgets/WidgetLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public Widget LoadWidget(WidgetArgs args, Widget parent, MiniYamlNode node)
foreach (var c in child.Value.Nodes)
LoadWidget(args, widget, c);

var logicNode = node.Value.Nodes.FirstOrDefault(n => n.Key == "Logic");
var logicNode = node.Value.NodeWithKeyOrDefault("Logic");
var logic = logicNode?.Value.ToDictionary();
args.Add("logicArgs", logic);

Expand Down
2 changes: 1 addition & 1 deletion OpenRA.Game/World.cs
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ public void Tick()
if (tp.Actor == null)
break;

tp.Trait.ResolveTraitData(tp.Actor, kv.Value.Nodes);
tp.Trait.ResolveTraitData(tp.Actor, kv.Value);
}

gameSaveTraitData.Clear();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#endregion

using System.Collections.Generic;
using System.Linq;
using OpenRA.Graphics;
using OpenRA.Mods.Common.Graphics;

Expand Down Expand Up @@ -39,10 +38,10 @@ public ClassicTilesetSpecificSpriteSequence(SpriteCache cache, ISpriteSequenceLo

protected override IEnumerable<ReservationInfo> ParseFilenames(ModData modData, string tileset, int[] frames, MiniYaml data, MiniYaml defaults)
{
var node = data.Nodes.FirstOrDefault(n => n.Key == TilesetFilenames.Key) ?? defaults.Nodes.FirstOrDefault(n => n.Key == TilesetFilenames.Key);
var node = data.NodeWithKeyOrDefault(TilesetFilenames.Key) ?? defaults.NodeWithKeyOrDefault(TilesetFilenames.Key);
if (node != null)
{
var tilesetNode = node.Value.Nodes.FirstOrDefault(n => n.Key == tileset);
var tilesetNode = node.Value.NodeWithKeyOrDefault(tileset);
if (tilesetNode != null)
{
var loadFrames = CalculateFrameIndices(start, length, stride ?? length ?? 0, facings, frames, transpose, reverseFacings, shadowStart);
Expand All @@ -55,10 +54,10 @@ protected override IEnumerable<ReservationInfo> ParseFilenames(ModData modData,

protected override IEnumerable<ReservationInfo> ParseCombineFilenames(ModData modData, string tileset, int[] frames, MiniYaml data)
{
var node = data.Nodes.FirstOrDefault(n => n.Key == TilesetFilenames.Key);
var node = data.NodeWithKeyOrDefault(TilesetFilenames.Key);
if (node != null)
{
var tilesetNode = node.Value.Nodes.FirstOrDefault(n => n.Key == tileset);
var tilesetNode = node.Value.NodeWithKeyOrDefault(tileset);
if (tilesetNode != null)
{
if (frames == null)
Expand Down

0 comments on commit 910f77c

Please sign in to comment.