From e4005a6c547a2a2410d86329c2309fc999275092 Mon Sep 17 00:00:00 2001 From: Lex Date: Sat, 5 Sep 2026 19:44:39 -0300 Subject: [PATCH 1/2] perf(statescript): index graph property definitions by name Every resolver bound to a node input is defined as a property, and resolving one walked the whole property list - so reading a single input cost a scan of the entire graph, and a graph's cost grew with the square of its own size. The lists keep their definition order; lookups go through a dictionary beside them. --- .../Statescript/GraphProcessorTests.cs | 130 +++++++++++++++++ Forge/Statescript/GraphContext.cs | 134 ++++++++---------- Forge/Statescript/GraphVariableDefinitions.cs | 24 +++- 3 files changed, 207 insertions(+), 81 deletions(-) diff --git a/Forge.Tests/Statescript/GraphProcessorTests.cs b/Forge.Tests/Statescript/GraphProcessorTests.cs index 1ba8a046..db456710 100644 --- a/Forge.Tests/Statescript/GraphProcessorTests.cs +++ b/Forge.Tests/Statescript/GraphProcessorTests.cs @@ -1,6 +1,8 @@ // Copyright © Gamesmiths Guild. using FluentAssertions; +using Gamesmiths.Forge.Effects; +using Gamesmiths.Forge.Effects.Duration; using Gamesmiths.Forge.Statescript; using Gamesmiths.Forge.Statescript.Nodes; using Gamesmiths.Forge.Statescript.Nodes.State; @@ -996,4 +998,132 @@ public void ValidatePropertyType_checks_array_property_type() definitions.ValidatePropertyType("ids", typeof(double[])).Should().BeFalse(); definitions.ValidatePropertyType("ids", typeof(int[])).Should().BeTrue(); } + + [Fact] + [Trait("Graph", "Resolve")] + public void TryResolve_uses_the_first_of_two_properties_sharing_a_name() + { + var graph = new Graph(); + graph.VariableDefinitions.DefineProperty( + "value", + new VariantResolver(new Variant128(11.0), typeof(double))); + graph.VariableDefinitions.DefineProperty( + "value", + new VariantResolver(new Variant128(22.0), typeof(double))); + + var node = new ReadPropertyNode(); + node.BindInput(ReadPropertyNode.ValueInput, "value"); + graph.AddNode(node); + graph.AddConnection(new Connection( + graph.EntryNode.OutputPorts[EntryNode.OutputPort], + node.InputPorts[ActionNode.InputPort])); + + var processor = new GraphProcessor(graph); + processor.StartGraph(); + + node.Found.Should().BeTrue(); + node.LastReadValue.Should().Be(11.0); + } + + [Fact] + [Trait("Graph", "Resolve")] + public void TryResolveArray_uses_the_first_of_two_array_properties_sharing_a_name() + { + var graph = new Graph(); + graph.VariableDefinitions.DefineArrayProperty( + "ids", + new TestArrayPropertyResolver(typeof(int), [[new Variant128(1), new Variant128(2)]])); + graph.VariableDefinitions.DefineArrayProperty( + "ids", + new TestArrayPropertyResolver(typeof(int), [[new Variant128(9)]])); + + var node = new ReadArrayPropertyNode(); + node.BindInput(ReadArrayPropertyNode.InputArray, "ids"); + graph.AddNode(node); + graph.AddConnection(new Connection( + graph.EntryNode.OutputPorts[EntryNode.OutputPort], + node.InputPorts[ActionNode.InputPort])); + + var processor = new GraphProcessor(graph); + processor.StartGraph(); + + node.LastReadArray.Should().BeEquivalentTo([new Variant128(1), new Variant128(2)]); + } + + [Fact] + [Trait("Graph", "Resolve")] + public void TryResolveObject_uses_the_first_of_two_object_properties_sharing_a_name() + { + var firstData = new EffectData("First", new DurationData(DurationType.Instant)); + var secondData = new EffectData("Second", new DurationData(DurationType.Instant)); + + var graph = new Graph(); + graph.VariableDefinitions.DefineObjectProperty("effect", new EffectFromDataResolver(firstData)); + graph.VariableDefinitions.DefineObjectProperty("effect", new EffectFromDataResolver(secondData)); + + var node = new ReadObjectPropertyNode(); + node.BindInput(0, "effect"); + graph.AddNode(node); + graph.AddConnection(new Connection( + graph.EntryNode.OutputPorts[EntryNode.OutputPort], + node.InputPorts[ActionNode.InputPort])); + + var processor = new GraphProcessor(graph); + processor.StartGraph(); + + node.LastReadValue.Should().NotBeNull(); + node.LastReadValue!.EffectData.Name.Should().Be(firstData.Name); + } + + [Fact] + [Trait("Graph", "Resolve")] + public void TryResolveObjectArray_uses_the_first_of_two_object_array_properties_sharing_a_name() + { + var firstData = new EffectData("First", new DurationData(DurationType.Instant)); + var secondData = new EffectData("Second", new DurationData(DurationType.Instant)); + + var graph = new Graph(); + graph.VariableDefinitions.DefineObjectArrayProperty( + "effects", + new EffectArrayFromDataResolver([firstData])); + graph.VariableDefinitions.DefineObjectArrayProperty( + "effects", + new EffectArrayFromDataResolver([secondData])); + + var node = new ReadObjectArrayPropertyNode(); + node.BindInput(0, "effects"); + graph.AddNode(node); + graph.AddConnection(new Connection( + graph.EntryNode.OutputPorts[EntryNode.OutputPort], + node.InputPorts[ActionNode.InputPort])); + + var processor = new GraphProcessor(graph); + processor.StartGraph(); + + node.LastReadArray.Should().ContainSingle(); + node.LastReadArray![0].EffectData.Name.Should().Be(firstData.Name); + } + + [Fact] + [Trait("Graph", "Resolve")] + public void TryResolve_reports_a_name_that_is_neither_a_variable_nor_a_property() + { + var graph = new Graph(); + graph.VariableDefinitions.DefineProperty( + "defined", + new VariantResolver(new Variant128(1.0), typeof(double))); + + var node = new ReadPropertyNode(); + node.BindInput(ReadPropertyNode.ValueInput, "missing"); + graph.AddNode(node); + graph.AddConnection(new Connection( + graph.EntryNode.OutputPorts[EntryNode.OutputPort], + node.InputPorts[ActionNode.InputPort])); + + var processor = new GraphProcessor(graph); + processor.StartGraph(); + + node.ExecutionCount.Should().Be(1); + node.Found.Should().BeFalse(); + } } diff --git a/Forge/Statescript/GraphContext.cs b/Forge/Statescript/GraphContext.cs index b64c86de..3df4c1e5 100644 --- a/Forge/Statescript/GraphContext.cs +++ b/Forge/Statescript/GraphContext.cs @@ -125,14 +125,11 @@ public bool TryResolve(StringKey name, out T value) return false; } - foreach (PropertyDefinition definition in Processor.Graph.VariableDefinitions.PropertyDefinitions) + if (Processor.Graph.VariableDefinitions.PropertiesByName.TryGetValue(name, out PropertyDefinition definition)) { - if (definition.Name == name) - { - Variant128 resolved = definition.Resolver.Resolve(this); - value = resolved.Get(); - return true; - } + Variant128 resolved = definition.Resolver.Resolve(this); + value = resolved.Get(); + return true; } return false; @@ -157,13 +154,10 @@ public bool TryResolveVariant(StringKey name, out Variant128 value) return false; } - foreach (PropertyDefinition definition in Processor.Graph.VariableDefinitions.PropertyDefinitions) + if (Processor.Graph.VariableDefinitions.PropertiesByName.TryGetValue(name, out PropertyDefinition definition)) { - if (definition.Name == name) - { - value = definition.Resolver.Resolve(this); - return true; - } + value = definition.Resolver.Resolve(this); + return true; } return false; @@ -191,17 +185,13 @@ public bool TryResolveArray(StringKey name, [NotNullWhen(true)] out Variant128[] return true; } - if (Processor is not null) + if (Processor is not null + && Processor.Graph.VariableDefinitions.ArrayPropertiesByName.TryGetValue( + name, + out ArrayPropertyDefinition definition)) { - foreach (ArrayPropertyDefinition definition in - Processor.Graph.VariableDefinitions.ArrayPropertyDefinitions) - { - if (definition.Name == name) - { - values = definition.Resolver.ResolveArray(this); - return true; - } - } + values = definition.Resolver.ResolveArray(this); + return true; } values = null; @@ -230,19 +220,18 @@ public bool TryResolveObject(StringKey name, [MaybeNullWhen(false)] out T val return false; } - foreach (ObjectPropertyDefinition definition in Processor.Graph.VariableDefinitions.ObjectPropertyDefinitions) + if (Processor.Graph.VariableDefinitions.ObjectPropertiesByName.TryGetValue( + name, + out ObjectPropertyDefinition definition)) { - if (definition.Name == name) + if (!typeof(T).IsAssignableFrom(definition.Resolver.ValueType)) { - if (!typeof(T).IsAssignableFrom(definition.Resolver.ValueType)) - { - value = default!; - return false; - } - - value = (T)definition.Resolver.Resolve(this)!; - return true; + value = default!; + return false; } + + value = (T)definition.Resolver.Resolve(this)!; + return true; } value = default!; @@ -271,19 +260,18 @@ public bool TryResolveObject(StringKey name, Type expectedType, out object? valu return false; } - foreach (ObjectPropertyDefinition definition in Processor.Graph.VariableDefinitions.ObjectPropertyDefinitions) + if (Processor.Graph.VariableDefinitions.ObjectPropertiesByName.TryGetValue( + name, + out ObjectPropertyDefinition definition)) { - if (definition.Name == name) + if (!expectedType.IsAssignableFrom(definition.Resolver.ValueType)) { - if (!expectedType.IsAssignableFrom(definition.Resolver.ValueType)) - { - value = null; - return false; - } - - value = definition.Resolver.Resolve(this); - return true; + value = null; + return false; } + + value = definition.Resolver.Resolve(this); + return true; } value = null; @@ -306,29 +294,25 @@ public bool TryResolveObjectArray(StringKey name, [NotNullWhen(true)] out T[] return true; } - if (Processor is not null) + if (Processor is not null + && Processor.Graph.VariableDefinitions.ObjectArrayPropertiesByName.TryGetValue( + name, + out ObjectArrayPropertyDefinition definition)) { - foreach (ObjectArrayPropertyDefinition definition in - Processor.Graph.VariableDefinitions.ObjectArrayPropertyDefinitions) + if (!typeof(T).IsAssignableFrom(definition.Resolver.ElementType)) + { + values = null; + return false; + } + + object?[] resolved = definition.Resolver.ResolveArray(this); + values = new T[resolved.Length]; + for (int i = 0; i < resolved.Length; i++) { - if (definition.Name == name) - { - if (!typeof(T).IsAssignableFrom(definition.Resolver.ElementType)) - { - values = null; - return false; - } - - object?[] resolved = definition.Resolver.ResolveArray(this); - values = new T[resolved.Length]; - for (int i = 0; i < resolved.Length; i++) - { - values[i] = (T)resolved[i]!; - } - - return true; - } + values[i] = (T)resolved[i]!; } + + return true; } values = null; @@ -354,23 +338,19 @@ public bool TryResolveObjectArray( return true; } - if (Processor is not null) + if (Processor is not null + && Processor.Graph.VariableDefinitions.ObjectArrayPropertiesByName.TryGetValue( + name, + out ObjectArrayPropertyDefinition definition)) { - foreach (ObjectArrayPropertyDefinition definition in - Processor.Graph.VariableDefinitions.ObjectArrayPropertyDefinitions) + if (!expectedElementType.IsAssignableFrom(definition.Resolver.ElementType)) { - if (definition.Name == name) - { - if (!expectedElementType.IsAssignableFrom(definition.Resolver.ElementType)) - { - values = null; - return false; - } - - values = definition.Resolver.ResolveArray(this); - return true; - } + values = null; + return false; } + + values = definition.Resolver.ResolveArray(this); + return true; } values = null; diff --git a/Forge/Statescript/GraphVariableDefinitions.cs b/Forge/Statescript/GraphVariableDefinitions.cs index b3094f1c..4ef70c6b 100644 --- a/Forge/Statescript/GraphVariableDefinitions.cs +++ b/Forge/Statescript/GraphVariableDefinitions.cs @@ -55,6 +55,14 @@ public class GraphVariableDefinitions /// public List ObjectArrayPropertyDefinitions { get; } = []; + internal Dictionary PropertiesByName { get; } = []; + + internal Dictionary ObjectPropertiesByName { get; } = []; + + internal Dictionary ArrayPropertiesByName { get; } = []; + + internal Dictionary ObjectArrayPropertiesByName { get; } = []; + /// /// Adds a mutable variable definition with the specified name and initial value. /// @@ -118,7 +126,9 @@ public void DefineObjectArrayVariable(StringKey name, params T[] initialValue /// The resolver used to compute the property's value at runtime. public void DefineProperty(StringKey name, IPropertyResolver resolver) { - PropertyDefinitions.Add(new PropertyDefinition(name, resolver)); + var definition = new PropertyDefinition(name, resolver); + PropertyDefinitions.Add(definition); + PropertiesByName.TryAdd(name, definition); } /// @@ -128,7 +138,9 @@ public void DefineProperty(StringKey name, IPropertyResolver resolver) /// The resolver used to compute the property's value at runtime. public void DefineObjectProperty(StringKey name, IObjectResolver resolver) { - ObjectPropertyDefinitions.Add(new ObjectPropertyDefinition(name, resolver)); + var definition = new ObjectPropertyDefinition(name, resolver); + ObjectPropertyDefinitions.Add(definition); + ObjectPropertiesByName.TryAdd(name, definition); } /// @@ -138,7 +150,9 @@ public void DefineObjectProperty(StringKey name, IObjectResolver resolver) /// The resolver used to compute the property's array value at runtime. public void DefineArrayProperty(StringKey name, IArrayPropertyResolver resolver) { - ArrayPropertyDefinitions.Add(new ArrayPropertyDefinition(name, resolver)); + var definition = new ArrayPropertyDefinition(name, resolver); + ArrayPropertyDefinitions.Add(definition); + ArrayPropertiesByName.TryAdd(name, definition); } /// @@ -148,7 +162,9 @@ public void DefineArrayProperty(StringKey name, IArrayPropertyResolver resolver) /// The resolver used to compute the property's array value at runtime. public void DefineObjectArrayProperty(StringKey name, IObjectArrayResolver resolver) { - ObjectArrayPropertyDefinitions.Add(new ObjectArrayPropertyDefinition(name, resolver)); + var definition = new ObjectArrayPropertyDefinition(name, resolver); + ObjectArrayPropertyDefinitions.Add(definition); + ObjectArrayPropertiesByName.TryAdd(name, definition); } /// From a02623af139de6de9d66e4060b59d2a4bfaedfed Mon Sep 17 00:00:00 2001 From: Lex Date: Sat, 5 Sep 2026 23:45:36 -0300 Subject: [PATCH 2/2] fix(statescript): keep the property indexes the only way in Review of #65: the definition lists stayed public and mutable, so a definition added straight to one would be invisible to every lookup, and ValidatePropertyType still scanned lists while resolution read the index. The four property collections are read-only now and validation goes through the same index. --- Forge/Statescript/GraphVariableDefinitions.cs | 97 +++++++++++-------- 1 file changed, 56 insertions(+), 41 deletions(-) diff --git a/Forge/Statescript/GraphVariableDefinitions.cs b/Forge/Statescript/GraphVariableDefinitions.cs index 4ef70c6b..fd3590b8 100644 --- a/Forge/Statescript/GraphVariableDefinitions.cs +++ b/Forge/Statescript/GraphVariableDefinitions.cs @@ -13,6 +13,14 @@ namespace Gamesmiths.Forge.Statescript; /// public class GraphVariableDefinitions { + private readonly List _propertyDefinitions = []; + + private readonly List _objectPropertyDefinitions = []; + + private readonly List _arrayPropertyDefinitions = []; + + private readonly List _objectArrayPropertyDefinitions = []; + /// /// Gets the list of variable definitions for the graph. /// @@ -34,26 +42,43 @@ public class GraphVariableDefinitions public List ObjectArrayVariableDefinitions { get; } = []; /// - /// Gets the list of property definitions for the graph. Properties are read-only computed values resolved on demand - /// from external sources (attributes, tags, comparisons, etc.). + /// Gets the property definitions for the graph, in definition order. Properties are read-only computed values + /// resolved on demand from external sources (attributes, tags, comparisons, etc.). /// - public List PropertyDefinitions { get; } = []; + /// + /// Read-only because a property is also indexed by name, and the index is what every lookup consults. Add one with + /// , which writes both. + /// + public IReadOnlyList PropertyDefinitions => _propertyDefinitions; /// - /// Gets the list of object-backed property definitions for the graph. + /// Gets the object-backed property definitions for the graph, in definition order. /// - public List ObjectPropertyDefinitions { get; } = []; + /// + /// Read-only for the same reason is. Add one with + /// . + /// + public IReadOnlyList ObjectPropertyDefinitions => _objectPropertyDefinitions; /// - /// Gets the list of array property definitions for the graph. Array properties are read-only computed values that - /// resolve to arrays (e.g., a list of entity IDs within a radius). + /// Gets the array property definitions for the graph, in definition order. Array properties are read-only computed + /// values that resolve to arrays (e.g., a list of entity IDs within a radius). /// - public List ArrayPropertyDefinitions { get; } = []; + /// + /// Read-only for the same reason is. Add one with + /// . + /// + public IReadOnlyList ArrayPropertyDefinitions => _arrayPropertyDefinitions; /// - /// Gets the list of object-backed array property definitions for the graph. + /// Gets the object-backed array property definitions for the graph, in definition order. /// - public List ObjectArrayPropertyDefinitions { get; } = []; + /// + /// Read-only for the same reason is. Add one with + /// . + /// + public IReadOnlyList ObjectArrayPropertyDefinitions => + _objectArrayPropertyDefinitions; internal Dictionary PropertiesByName { get; } = []; @@ -127,7 +152,7 @@ public void DefineObjectArrayVariable(StringKey name, params T[] initialValue public void DefineProperty(StringKey name, IPropertyResolver resolver) { var definition = new PropertyDefinition(name, resolver); - PropertyDefinitions.Add(definition); + _propertyDefinitions.Add(definition); PropertiesByName.TryAdd(name, definition); } @@ -139,7 +164,7 @@ public void DefineProperty(StringKey name, IPropertyResolver resolver) public void DefineObjectProperty(StringKey name, IObjectResolver resolver) { var definition = new ObjectPropertyDefinition(name, resolver); - ObjectPropertyDefinitions.Add(definition); + _objectPropertyDefinitions.Add(definition); ObjectPropertiesByName.TryAdd(name, definition); } @@ -151,7 +176,7 @@ public void DefineObjectProperty(StringKey name, IObjectResolver resolver) public void DefineArrayProperty(StringKey name, IArrayPropertyResolver resolver) { var definition = new ArrayPropertyDefinition(name, resolver); - ArrayPropertyDefinitions.Add(definition); + _arrayPropertyDefinitions.Add(definition); ArrayPropertiesByName.TryAdd(name, definition); } @@ -163,7 +188,7 @@ public void DefineArrayProperty(StringKey name, IArrayPropertyResolver resolver) public void DefineObjectArrayProperty(StringKey name, IObjectArrayResolver resolver) { var definition = new ObjectArrayPropertyDefinition(name, resolver); - ObjectArrayPropertyDefinitions.Add(definition); + _objectArrayPropertyDefinitions.Add(definition); ObjectArrayPropertiesByName.TryAdd(name, definition); } @@ -177,47 +202,37 @@ public void DefineObjectArrayProperty(StringKey name, IObjectArrayResolver resol /// otherwise. public bool ValidatePropertyType(StringKey name, Type expectedType) { - foreach (PropertyDefinition definition in PropertyDefinitions) + // Through the same indexes the resolve paths use, so validation and resolution can never disagree about which + // definition a name refers to. + if (PropertiesByName.TryGetValue(name, out PropertyDefinition property)) { - if (definition.Name == name) - { - return expectedType.IsAssignableFrom(definition.Resolver.ValueType); - } + return expectedType.IsAssignableFrom(property.Resolver.ValueType); } - foreach (ObjectPropertyDefinition definition in ObjectPropertyDefinitions) + if (ObjectPropertiesByName.TryGetValue(name, out ObjectPropertyDefinition objectProperty)) { - if (definition.Name == name) - { - return expectedType.IsAssignableFrom(definition.Resolver.ValueType); - } + return expectedType.IsAssignableFrom(objectProperty.Resolver.ValueType); } - foreach (ArrayPropertyDefinition definition in ArrayPropertyDefinitions) + if (ArrayPropertiesByName.TryGetValue(name, out ArrayPropertyDefinition arrayProperty)) { - if (definition.Name == name) + // expectedType should be an array type (e.g., typeof(int[])), and element type must match + if (expectedType.IsArray && expectedType.GetElementType() is Type elementType) { - // expectedType should be an array type (e.g., typeof(int[])), and element type must match - if (expectedType.IsArray && expectedType.GetElementType() is Type elementType) - { - return elementType.IsAssignableFrom(definition.Resolver.ElementType); - } - - return false; + return elementType.IsAssignableFrom(arrayProperty.Resolver.ElementType); } + + return false; } - foreach (ObjectArrayPropertyDefinition definition in ObjectArrayPropertyDefinitions) + if (ObjectArrayPropertiesByName.TryGetValue(name, out ObjectArrayPropertyDefinition objectArrayProperty)) { - if (definition.Name == name) + if (expectedType.IsArray && expectedType.GetElementType() is Type objectElementType) { - if (expectedType.IsArray && expectedType.GetElementType() is Type elementType) - { - return elementType.IsAssignableFrom(definition.Resolver.ElementType); - } - - return false; + return objectElementType.IsAssignableFrom(objectArrayProperty.Resolver.ElementType); } + + return false; } foreach (VariableDefinition definition in VariableDefinitions)