From acebfaf1a02eeafde24ff36796a3de2e0493211e Mon Sep 17 00:00:00 2001 From: Bendik Tobias Berg Date: Tue, 17 Sep 2024 19:01:11 +0200 Subject: [PATCH 1/4] Avoid list allocations in GetNestedData. Faster lookup in GetOrphanedSerializables --- src/Engine/ProtoCore/Lang/CallSite.cs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/Engine/ProtoCore/Lang/CallSite.cs b/src/Engine/ProtoCore/Lang/CallSite.cs index ea85ed6f996..56d922080c3 100644 --- a/src/Engine/ProtoCore/Lang/CallSite.cs +++ b/src/Engine/ProtoCore/Lang/CallSite.cs @@ -155,17 +155,20 @@ public bool Contains(string data) public List RecursiveGetNestedData() { List ret = new List(); + RecursiveFillWithNestedData(ret); + return ret; + } + public void RecursiveFillWithNestedData(List listToFill) + { if (HasData) - ret.Add(Data); + listToFill.Add(Data); if (HasNestedData) { foreach (SingleRunTraceData srtd in NestedData) - ret.AddRange(srtd.RecursiveGetNestedData()); + srtd.RecursiveFillWithNestedData(listToFill); } - - return ret; } } @@ -472,13 +475,11 @@ private static string CompressSerializedTraceData(string json) /// public IList GetOrphanedSerializables() { - var result = new List(); - - if (!beforeFirstRunSerializables.Any()) - return result; + if (beforeFirstRunSerializables.Count == 0) + return new List(); - var currentSerializables = traceData.SelectMany(td => td.RecursiveGetNestedData()); - result.AddRange(beforeFirstRunSerializables.Where(hs => !currentSerializables.Contains(hs)).ToList()); + var currentSerializables = traceData.SelectMany(td => td.RecursiveGetNestedData()).ToHashSet(); + var result = beforeFirstRunSerializables.Where(hs => !currentSerializables.Contains(hs)).ToList(); // Clear the historical serializable to avoid // them being used again. From 455c181955d7da443f3f2a5d43b0bb7b8f2932c4 Mon Sep 17 00:00:00 2001 From: Bendik Tobias Berg Date: Tue, 17 Sep 2024 19:05:18 +0200 Subject: [PATCH 2/4] Fix MAGN-7314? Trace reconciliation speedup. Remove unecessary List copies in AddRange. --- src/DynamoCore/Engine/EngineController.cs | 8 ++-- .../Graph/Workspaces/HomeWorkspaceModel.cs | 2 +- src/DynamoCore/Models/DynamoModel.cs | 39 ++++++++++++------- 3 files changed, 30 insertions(+), 19 deletions(-) diff --git a/src/DynamoCore/Engine/EngineController.cs b/src/DynamoCore/Engine/EngineController.cs index 00677531611..a70adf1ad1b 100644 --- a/src/DynamoCore/Engine/EngineController.cs +++ b/src/DynamoCore/Engine/EngineController.cs @@ -533,14 +533,14 @@ internal void ReconcileTraceDataAndNotify() var callsiteToOrphanMap = new Dictionary>(); foreach (var cs in liveRunnerServices.RuntimeCore.RuntimeData.CallsiteCache.Values) { - var orphanedSerializables = cs.GetOrphanedSerializables().ToList(); - if (callsiteToOrphanMap.ContainsKey(cs.CallSiteID)) + var orphanedSerializables = cs.GetOrphanedSerializables(); + if (callsiteToOrphanMap.TryGetValue(cs.CallSiteID, out var serializablesForCallsite)) { - callsiteToOrphanMap[cs.CallSiteID].AddRange(orphanedSerializables); + serializablesForCallsite.AddRange(orphanedSerializables); } else { - callsiteToOrphanMap.Add(cs.CallSiteID, orphanedSerializables); + callsiteToOrphanMap.Add(cs.CallSiteID, orphanedSerializables.ToList()); } } diff --git a/src/DynamoCore/Graph/Workspaces/HomeWorkspaceModel.cs b/src/DynamoCore/Graph/Workspaces/HomeWorkspaceModel.cs index 753a27c3da2..9017bd59081 100644 --- a/src/DynamoCore/Graph/Workspaces/HomeWorkspaceModel.cs +++ b/src/DynamoCore/Graph/Workspaces/HomeWorkspaceModel.cs @@ -897,7 +897,7 @@ internal IList GetOrphanedSerializablesAndClearHistoricalTraceData() if (Nodes.All(n => n.GUID != nodeGuid)) { - orphans.AddRange(nodeData.Value.SelectMany(CallSite.GetAllSerializablesFromSingleRunTraceData).ToList()); + orphans.AddRange(nodeData.Value.SelectMany(CallSite.GetAllSerializablesFromSingleRunTraceData)); } } diff --git a/src/DynamoCore/Models/DynamoModel.cs b/src/DynamoCore/Models/DynamoModel.cs index e233387a79f..cc8b2a46c4a 100644 --- a/src/DynamoCore/Models/DynamoModel.cs +++ b/src/DynamoCore/Models/DynamoModel.cs @@ -1260,22 +1260,30 @@ private void EngineController_TraceReconcliationComplete(TraceReconciliationEven // This dictionary gets redistributed into a dictionary keyed by the workspace id. var workspaceOrphanMap = new Dictionary>(); + var nodeToWorkspaceMap = new Dictionary(); - foreach (var ws in Workspaces.OfType()) + foreach (var maybeWs in Workspaces) { + foreach (var node in maybeWs.Nodes) + nodeToWorkspaceMap[node.GUID] = maybeWs; + + var ws = maybeWs as HomeWorkspaceModel; + if (ws == null) + continue; + // Get the orphaned serializables to this workspace - var wsOrphans = ws.GetOrphanedSerializablesAndClearHistoricalTraceData().ToList(); + var wsOrphans = (List)ws.GetOrphanedSerializablesAndClearHistoricalTraceData(); if (!wsOrphans.Any()) continue; - if (!workspaceOrphanMap.ContainsKey(ws.Guid)) + if (workspaceOrphanMap.TryGetValue(ws.Guid, out var workspaceOrphans)) { - workspaceOrphanMap.Add(ws.Guid, wsOrphans); + workspaceOrphans.AddRange(wsOrphans); } else { - workspaceOrphanMap[ws.Guid].AddRange(wsOrphans); + workspaceOrphanMap.Add(ws.Guid, wsOrphans); } } @@ -1287,23 +1295,26 @@ private void EngineController_TraceReconcliationComplete(TraceReconciliationEven // TODO: MAGN-7314 // Find the owning workspace for a node. - var nodeSpace = - Workspaces.FirstOrDefault( - ws => - ws.Nodes.FirstOrDefault(n => n.GUID == nodeGuid) - != null); + if (!nodeToWorkspaceMap.TryGetValue(nodeGuid, out var nodeSpace)) + continue; + + //var nodeSpace = + // Workspaces.FirstOrDefault( + // ws => + // ws.Nodes.FirstOrDefault(n => n.GUID == nodeGuid) + // != null); - if (nodeSpace == null) continue; + //if (nodeSpace == null) continue; // Add the node's orphaned serializables to the workspace // orphan map. - if (workspaceOrphanMap.ContainsKey(nodeSpace.Guid)) + if (workspaceOrphanMap.TryGetValue(nodeSpace.Guid, out var workspaceOrphans)) { - workspaceOrphanMap[nodeSpace.Guid].AddRange(kvp.Value); + workspaceOrphans.AddRange(kvp.Value); } else { - workspaceOrphanMap.Add(nodeSpace.Guid, kvp.Value); + workspaceOrphanMap.Add(nodeSpace.Guid, kvp.Value.ToList()); } } From 7595d9209749123479091f09a29b52c618dbdcc5 Mon Sep 17 00:00:00 2001 From: Bendik Tobias Berg Date: Thu, 26 Sep 2024 12:44:42 +0200 Subject: [PATCH 3/4] Slight optimization: Avoid unecessary lookup before setting key/value --- src/NodeServices/TraceSupport.cs | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/src/NodeServices/TraceSupport.cs b/src/NodeServices/TraceSupport.cs index ff9e170f70f..6e48bdc6acc 100644 --- a/src/NodeServices/TraceSupport.cs +++ b/src/NodeServices/TraceSupport.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Runtime.Serialization; namespace DynamoServices { @@ -70,15 +69,9 @@ internal static void ClearAllKnownTLSKeys() /// public static string GetTraceData(string key) { - string data; - if (!LocalStorageSlot.TryGetValue(key, out data)) - { - return null; - } - else - { + if (LocalStorageSlot.TryGetValue(key, out string data)) return data; - } + return null; } /// @@ -88,14 +81,7 @@ public static string GetTraceData(string key) /// public static void SetTraceData(string key, string value) { - if (LocalStorageSlot.ContainsKey(key)) - { - LocalStorageSlot[key] = value; - } - else - { - LocalStorageSlot.Add(key, value); - } + LocalStorageSlot[key] = value; } } } From 5c3b07c3c44a614b14dbc7e2646937bd04c9e78d Mon Sep 17 00:00:00 2001 From: Bendik Tobias Berg Date: Mon, 20 Jan 2025 10:30:00 +0100 Subject: [PATCH 4/4] Code cleanup. Make RecursiveFillW.. internal. Make GetOrphanedSerializablesAndClearHistoricalTraceData return List, add fast lookup. --- .../Graph/Workspaces/HomeWorkspaceModel.cs | 5 +++-- src/DynamoCore/Models/DynamoModel.cs | 21 +++++++++---------- src/Engine/ProtoCore/Lang/CallSite.cs | 10 +++++++-- 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/DynamoCore/Graph/Workspaces/HomeWorkspaceModel.cs b/src/DynamoCore/Graph/Workspaces/HomeWorkspaceModel.cs index 9017bd59081..eb3f763027c 100644 --- a/src/DynamoCore/Graph/Workspaces/HomeWorkspaceModel.cs +++ b/src/DynamoCore/Graph/Workspaces/HomeWorkspaceModel.cs @@ -880,7 +880,7 @@ private void OnPreviewGraphCompleted(AsyncTask asyncTask) /// trace data but do not exist in the current CallSite data. /// /// - internal IList GetOrphanedSerializablesAndClearHistoricalTraceData() + internal List GetOrphanedSerializablesAndClearHistoricalTraceData() { var orphans = new List(); @@ -891,11 +891,12 @@ internal IList GetOrphanedSerializablesAndClearHistoricalTraceData() // then add the serializables for that guid to the list of // orphans. + var nodeLookup = Nodes.Select(n => n.GUID).ToHashSet(); foreach (var nodeData in historicalTraceData) { var nodeGuid = nodeData.Key; - if (Nodes.All(n => n.GUID != nodeGuid)) + if (!nodeLookup.Contains(nodeGuid)) { orphans.AddRange(nodeData.Value.SelectMany(CallSite.GetAllSerializablesFromSingleRunTraceData)); } diff --git a/src/DynamoCore/Models/DynamoModel.cs b/src/DynamoCore/Models/DynamoModel.cs index cc8b2a46c4a..5cb25be7157 100644 --- a/src/DynamoCore/Models/DynamoModel.cs +++ b/src/DynamoCore/Models/DynamoModel.cs @@ -1265,17 +1265,22 @@ private void EngineController_TraceReconcliationComplete(TraceReconciliationEven foreach (var maybeWs in Workspaces) { foreach (var node in maybeWs.Nodes) + { nodeToWorkspaceMap[node.GUID] = maybeWs; + } - var ws = maybeWs as HomeWorkspaceModel; - if (ws == null) + if (maybeWs is not HomeWorkspaceModel ws) + { continue; + } // Get the orphaned serializables to this workspace - var wsOrphans = (List)ws.GetOrphanedSerializablesAndClearHistoricalTraceData(); + var wsOrphans = ws.GetOrphanedSerializablesAndClearHistoricalTraceData(); if (!wsOrphans.Any()) + { continue; + } if (workspaceOrphanMap.TryGetValue(ws.Guid, out var workspaceOrphans)) { @@ -1296,15 +1301,9 @@ private void EngineController_TraceReconcliationComplete(TraceReconciliationEven // TODO: MAGN-7314 // Find the owning workspace for a node. if (!nodeToWorkspaceMap.TryGetValue(nodeGuid, out var nodeSpace)) + { continue; - - //var nodeSpace = - // Workspaces.FirstOrDefault( - // ws => - // ws.Nodes.FirstOrDefault(n => n.GUID == nodeGuid) - // != null); - - //if (nodeSpace == null) continue; + } // Add the node's orphaned serializables to the workspace // orphan map. diff --git a/src/Engine/ProtoCore/Lang/CallSite.cs b/src/Engine/ProtoCore/Lang/CallSite.cs index 56d922080c3..db237faa6c3 100644 --- a/src/Engine/ProtoCore/Lang/CallSite.cs +++ b/src/Engine/ProtoCore/Lang/CallSite.cs @@ -159,15 +159,19 @@ public List RecursiveGetNestedData() return ret; } - public void RecursiveFillWithNestedData(List listToFill) + internal void RecursiveFillWithNestedData(List listToFill) { if (HasData) + { listToFill.Add(Data); + } if (HasNestedData) { foreach (SingleRunTraceData srtd in NestedData) + { srtd.RecursiveFillWithNestedData(listToFill); + } } } } @@ -475,8 +479,10 @@ private static string CompressSerializedTraceData(string json) /// public IList GetOrphanedSerializables() { - if (beforeFirstRunSerializables.Count == 0) + if (!beforeFirstRunSerializables.Any()) + { return new List(); + } var currentSerializables = traceData.SelectMany(td => td.RecursiveGetNestedData()).ToHashSet(); var result = beforeFirstRunSerializables.Where(hs => !currentSerializables.Contains(hs)).ToList();