diff --git a/.jules/bolt.md b/.jules/bolt.md index 9064926..6645e15 100644 --- a/.jules/bolt.md +++ b/.jules/bolt.md @@ -85,3 +85,10 @@ ## 2024-06-15 - Optimize Dictionary allocations in C# hot loops **Learning:** In C#, LINQ `.ToDictionary` allocates a new dictionary, delegates, and enumerators. Using `.Any()` afterwards also introduces another O(N) pass. **Action:** Replace `.ToDictionary` and subsequent `.Any()` combinations with a manual `Dictionary` pre-allocated by count, populated via a `foreach` loop, and track boolean flags (e.g., `hasFiniteEdges`) inside the same loop to avoid multiple iterations and delegate allocations. +## 2024-06-15 - Optimize Dictionary allocations in C# hot loops +**Learning:** In C#, LINQ `.ToDictionary` allocates a new dictionary, delegates, and enumerators. Using `.Any()` afterwards also introduces another O(N) pass. +**Action:** Replace `.ToDictionary` and subsequent `.Any()` combinations with a manual `Dictionary` pre-allocated by count, populated via a `foreach` loop, and track boolean flags (e.g., `hasFiniteEdges`) inside the same loop to avoid multiple iterations and delegate allocations. + +## 2024-06-18 - Replacing LINQ ToDictionary causes correctness regression on math operations +**Learning:** When refactoring a LINQ `.GroupBy` and `.ToDictionary` call (like in `SummarizeLandedUnitCosts`), changing the calculation inside the aggregation loop (e.g. accidentally changing `DeliveredCostPerUnit * Quantity` to `TotalMovementCost`) breaks fundamental simulation results because these domain concepts are distinct. +**Action:** Be extremely cautious when rewriting aggregation logic in standard loops. Ensure the math inside the `else` / `TryGetValue` block precisely mirrors the original LINQ delegate calculation without substituting properties. diff --git a/src/MedWNetworkSim.App/Services/NetworkSimulationEngine.cs b/src/MedWNetworkSim.App/Services/NetworkSimulationEngine.cs index 104dc2f..8328481 100644 --- a/src/MedWNetworkSim.App/Services/NetworkSimulationEngine.cs +++ b/src/MedWNetworkSim.App/Services/NetworkSimulationEngine.cs @@ -39,10 +39,17 @@ public IReadOnlyList Simulate(NetworkModel network) } var hasRecipeDependencies = HasStaticRecipeDependencies(network); - var definitionsByTraffic = network.TrafficTypes - .Where(definition => !string.IsNullOrWhiteSpace(definition.Name)) - .GroupBy(definition => definition.Name, Comparer) - .ToDictionary(group => group.Key, group => group.First(), Comparer); + + // Bolt: Replaced LINQ .ToDictionary() with manual foreach to avoid enumerator and delegate allocations + var definitionsByTraffic = new Dictionary(Comparer); + foreach (var def in network.TrafficTypes) + { + if (!string.IsNullOrWhiteSpace(def.Name) && !definitionsByTraffic.ContainsKey(def.Name)) + { + definitionsByTraffic[def.Name] = def; + } + } + var contexts = MixedRoutingAllocator.BuildStaticContexts(network, applyLocalAllocations: !hasRecipeDependencies).ToList(); // Bolt: Replaced LINQ .ToDictionary() with manual foreach to avoid enumerator and delegate allocations var remainingCapacityByEdgeId = new Dictionary(network.Edges.Count, Comparer); @@ -71,16 +78,21 @@ public IReadOnlyList Simulate(NetworkModel network) if (hasRecipeDependencies) { - var contextsByTraffic = contexts.ToDictionary(context => context.TrafficType, context => context, Comparer); - var allocationOrder = BuildStaticRecipeCostOrder(network, contexts.Select(context => context.TrafficType).ToList()); - var sourceUnitCosts = contexts.ToDictionary( - context => context.TrafficType, - _ => new Dictionary(Comparer), - Comparer); - var landedUnitCosts = contexts.ToDictionary( - context => context.TrafficType, - _ => new Dictionary(Comparer), - Comparer); + // Bolt: Replaced LINQ .ToDictionary() with manual loops to avoid enumerator and delegate allocations + var contextsByTraffic = new Dictionary(contexts.Count, Comparer); + var sourceUnitCosts = new Dictionary>(contexts.Count, Comparer); + var landedUnitCosts = new Dictionary>(contexts.Count, Comparer); + var trafficTypesList = new List(contexts.Count); + + foreach (var context in contexts) + { + contextsByTraffic[context.TrafficType] = context; + sourceUnitCosts[context.TrafficType] = new Dictionary(Comparer); + landedUnitCosts[context.TrafficType] = new Dictionary(Comparer); + trafficTypesList.Add(context.TrafficType); + } + + var allocationOrder = BuildStaticRecipeCostOrder(network, trafficTypesList); foreach (var trafficType in allocationOrder) { @@ -332,9 +344,13 @@ private static string GetNodeLabel(NetworkModel network, string nodeId) private NetworkModel OrderNetworkForLayerProcessing(NetworkModel network) { - var order = layerResolver.GetSimulationOrder(network) - .Select((layer, index) => new { layer.Id, index }) - .ToDictionary(item => item.Id, item => item.index); + // Bolt: Replaced LINQ Select().ToDictionary() with a manual loop to avoid enumerator and delegate allocations + var simulationOrderList = layerResolver.GetSimulationOrder(network); + var order = new Dictionary(simulationOrderList.Count); + for (var index = 0; index < simulationOrderList.Count; index++) + { + order[simulationOrderList[index].Id] = index; + } return new NetworkModel { @@ -736,24 +752,28 @@ private static double ResolveBaseProductionCost(NodeTrafficProfile? profile, Tra private static Dictionary SummarizeLandedUnitCosts(IEnumerable allocations) { - return allocations - .GroupBy(allocation => allocation.ConsumerNodeId, Comparer) - .ToDictionary( - group => group.Key, - group => - { - // Bolt: Accumulate quantity and total cost in a single loop to avoid multiple O(N) LINQ enumerations and delegate allocations - var quantity = 0d; - var totalCost = 0d; - foreach (var allocation in group) - { - quantity += allocation.Quantity; - totalCost += allocation.DeliveredCostPerUnit * allocation.Quantity; - } + // Bolt: Replaced GroupBy and ToDictionary with manual Dictionary accumulations to avoid all delegate and enumerator overhead + var costsByConsumer = new Dictionary(Comparer); + + foreach (var allocation in allocations) + { + if (costsByConsumer.TryGetValue(allocation.ConsumerNodeId, out var current)) + { + costsByConsumer[allocation.ConsumerNodeId] = (current.Quantity + allocation.Quantity, current.TotalCost + allocation.DeliveredCostPerUnit * allocation.Quantity); + } + else + { + costsByConsumer[allocation.ConsumerNodeId] = (allocation.Quantity, allocation.DeliveredCostPerUnit * allocation.Quantity); + } + } - return quantity > Epsilon ? totalCost / quantity : 0d; - }, - Comparer); + var result = new Dictionary(costsByConsumer.Count, Comparer); + foreach (var pair in costsByConsumer) + { + result[pair.Key] = pair.Value.Quantity > Epsilon ? pair.Value.TotalCost / pair.Value.Quantity : 0d; + } + + return result; } private static TrafficContext BuildContext(NetworkModel network, TrafficTypeDefinition definition) diff --git a/src/MedWNetworkSim.App/Services/TemporalNetworkSimulationEngine.cs b/src/MedWNetworkSim.App/Services/TemporalNetworkSimulationEngine.cs index f186edd..16480b1 100644 --- a/src/MedWNetworkSim.App/Services/TemporalNetworkSimulationEngine.cs +++ b/src/MedWNetworkSim.App/Services/TemporalNetworkSimulationEngine.cs @@ -38,7 +38,14 @@ public TemporalSimulationState Initialize(NetworkModel network) network = executionCache.GetStaticContext(network).EffectiveNetwork; var state = new TemporalSimulationState(); - var definitionsByTraffic = network.TrafficTypes.ToDictionary(definition => definition.Name, definition => definition, Comparer); + + // Bolt: Replaced LINQ .ToDictionary() with manual foreach to avoid enumerator and delegate allocations + var definitionsByTraffic = new Dictionary(network.TrafficTypes.Count, Comparer); + foreach (var def in network.TrafficTypes) + { + definitionsByTraffic[def.Name] = def; + } + foreach (var node in network.Nodes) { foreach (var profile in node.TrafficProfiles) @@ -1060,6 +1067,25 @@ private static void ApplyLocalAllocations( private static RoutingTrafficContext ToRoutingContext(TemporalTrafficContext context) { + // Bolt: Replaced LINQ .ToDictionary() with manual foreach to avoid enumerator and delegate allocations + var supply = new Dictionary(context.Supply.Count, Comparer); + foreach (var pair in context.Supply) + { + supply[pair.Key] = pair.Value; + } + + var supplyUnitCosts = new Dictionary(context.SupplyUnitCosts.Count, Comparer); + foreach (var pair in context.SupplyUnitCosts) + { + supplyUnitCosts[pair.Key] = pair.Value; + } + + var demand = new Dictionary(context.Demand.Count, Comparer); + foreach (var pair in context.Demand) + { + demand[pair.Key] = pair.Value; + } + return new RoutingTrafficContext { TrafficType = context.TrafficType, @@ -1072,9 +1098,9 @@ private static RoutingTrafficContext ToRoutingContext(TemporalTrafficContext con Seed = context.Seed, NodesById = context.NodesById, ProfilesByNodeId = context.ProfilesByNodeId, - Supply = context.Supply.ToDictionary(pair => pair.Key, pair => pair.Value, Comparer), - SupplyUnitCosts = context.SupplyUnitCosts.ToDictionary(pair => pair.Key, pair => pair.Value, Comparer), - Demand = context.Demand.ToDictionary(pair => pair.Key, pair => pair.Value, Comparer), + Supply = supply, + SupplyUnitCosts = supplyUnitCosts, + Demand = demand, MeetingDemandEligibleNodeIds = context.MeetingDemandEligibleNodeIds }; }