Dim/dui3/send caching di (DUI3-79) (#3491)

* feat(dui3): wip send caching di

implements rhino

* fix(dui3): fixes fatal crash on object deleted and re-send/highlight

i distinctly remember fixing this before in dui3 😔

* feat(dui3): di send cache in acad

* wip(dui3): di cache revit wip

* fix(revit): we seem to be cursed to be unable to do basic boolean logic

* feat(dui3): wraps up di caching revit

* feat(caching): cleanup

* chore(dui3): rename arcgis object builder as others

* feat(dui3): implements di cache in arcgis - NOT TESTED

* chore(caching): adds comment on nullability

* feat(arcgis): makes cache non-nullable

* fix(caching): makes cache non-nullable where we know it's going to be non-nullable:

specifically, in host app implementations of the root object builder and send bindings

* chore(dui3): removes a bunch of dead code and comments

* feat(caching): adds a "null" do nothing cache and makes it non-optional in the root object sender

* chore(caching): removes unnecessary comment

* fix(acgis): registers cache!

DUI3-DX/Connectors/ArcGIS/Speckle.Connectors.ArcGIS3/Bindings/ArcGISSendBinding.cs

@@ -16,8 +16,8 @@
 using ArcGIS.Core.Data;
 using Speckle.Connectors.DUI.Exceptions;
 using Speckle.Connectors.Utils;
+using Speckle.Connectors.Utils.Caching;
 using Speckle.Connectors.Utils.Operations;
-using Speckle.Core.Models;
 
 namespace Speckle.Connectors.ArcGIS.Bindings;
 
@@ -31,8 +31,7 @@ public sealed class ArcGISSendBinding : ISendBinding, ICancelable
   private readonly IUnitOfWorkFactory _unitOfWorkFactory; // POC: unused? :D
   private readonly List<ISendFilter> _sendFilters;
   private readonly CancellationManager _cancellationManager;
-
-  private readonly Dictionary<string, ObjectReference> _convertedObjectReferences = new();
+  private readonly ISendConversionCache _sendConversionCache;
 
   /// <summary>
   /// Used internally to aggregate the changed objects' id.
@@ -46,14 +45,15 @@ public ArcGISSendBinding(
     IBridge parent,
     IEnumerable<ISendFilter> sendFilters,
     IUnitOfWorkFactory unitOfWorkFactory,
-    CancellationManager cancellationManager
+    CancellationManager cancellationManager,
+    ISendConversionCache sendConversionCache
   )
   {
     _store = store;
     _unitOfWorkFactory = unitOfWorkFactory;
     _sendFilters = sendFilters.ToList();
     _cancellationManager = cancellationManager;
-
+    _sendConversionCache = sendConversionCache;
     Parent = parent;
     Commands = new SendBindingUICommands(parent);
     SubscribeToArcGISEvents();
@@ -277,9 +277,7 @@ public async Task Send(string modelCardId)
         modelCard.AccountId.NotNull(),
         modelCard.ProjectId.NotNull(),
         modelCard.ModelId.NotNull(),
-        "ArcGIS",
-        _convertedObjectReferences,
-        modelCard.ChangedObjectIds
+        "ArcGIS"
       );
 
       var sendResult = await QueuedTask
@@ -309,15 +307,6 @@ public async Task Send(string modelCardId)
             )
             .ConfigureAwait(false);
 
-          // Store the converted references in memory for future send operations, overwriting the existing values for the given application id.
-          foreach (var kvp in result.ConvertedReferences)
-          {
-            _convertedObjectReferences[kvp.Key + modelCard.ProjectId] = kvp.Value;
-          }
-
-          // It's important to reset the model card's list of changed obj ids so as to ensure we accurately keep track of changes between send operations.
-          modelCard.ChangedObjectIds = new();
-
           return result;
         })
         .ConfigureAwait(false);
@@ -347,6 +336,8 @@ private void RunExpirationChecks(bool idsDeleted)
     List<string> expiredSenderIds = new();
     string[] objectIdsList = ChangedObjectIds.ToArray();
 
+    _sendConversionCache.EvictObjects(objectIdsList);
+
     foreach (SenderModelCard sender in senders)
     {
       var objIds = sender.SendFilter.NotNull().GetObjectIds();
@@ -355,7 +346,6 @@ private void RunExpirationChecks(bool idsDeleted)
       if (isExpired)
       {
         expiredSenderIds.Add(sender.ModelCardId.NotNull());
-        sender.ChangedObjectIds.UnionWith(intersection.NotNull());
 
         // Update the model card object Ids
         if (idsDeleted && sender.SendFilter is ArcGISSelectionFilter filter)

DUI3-DX/Connectors/ArcGIS/Speckle.Connectors.ArcGIS3/DependencyInjection/ArcGISConnectorModule.cs

@@ -14,6 +14,7 @@
 using Speckle.Connectors.DUI;
 using Speckle.Connectors.DUI.Models;
 using Speckle.Connectors.Utils;
+using Speckle.Connectors.Utils.Caching;
 using Speckle.Connectors.Utils.Operations;
 using Speckle.Core.Models.GraphTraversal;
 
@@ -53,7 +54,10 @@ public void Load(SpeckleContainerBuilder builder)
 
     // register send operation and dependencies
     builder.AddScoped<SendOperation<MapMember>>();
-    builder.AddScoped<RootObjectBuilder>();
-    builder.AddSingleton<IRootObjectBuilder<MapMember>, RootObjectBuilder>();
+    builder.AddScoped<ArcGISRootObjectBuilder>();
+    builder.AddSingleton<IRootObjectBuilder<MapMember>, ArcGISRootObjectBuilder>();
+
+    // register send conversion cache
+    builder.AddSingleton<ISendConversionCache, SendConversionCache>();
   }
 }

DUI3-DX/Connectors/ArcGIS/Speckle.Connectors.ArcGIS3/Operations/Send/RootObjectBuilder.cs → DUI3-DX/Connectors/ArcGIS/Speckle.Connectors.ArcGIS3/Operations/Send/ArcGISRootObjectBuilder.cs

@@ -1,6 +1,8 @@
+using System.Diagnostics;
 using ArcGIS.Desktop.Mapping;
 using Speckle.Autofac.DependencyInjection;
 using Speckle.Connectors.Utils.Builders;
+using Speckle.Connectors.Utils.Caching;
 using Speckle.Connectors.Utils.Conversion;
 using Speckle.Connectors.Utils.Operations;
 using Speckle.Converters.Common;
@@ -12,13 +14,15 @@ namespace Speckle.Connectors.ArcGis.Operations.Send;
 /// <summary>
 /// Stateless builder object to turn an ISendFilter into a <see cref="Base"/> object
 /// </summary>
-public class RootObjectBuilder : IRootObjectBuilder<MapMember>
+public class ArcGISRootObjectBuilder : IRootObjectBuilder<MapMember>
 {
   private readonly IUnitOfWorkFactory _unitOfWorkFactory;
+  private readonly ISendConversionCache _sendConversionCache;
 
-  public RootObjectBuilder(IUnitOfWorkFactory unitOfWorkFactory)
+  public ArcGISRootObjectBuilder(IUnitOfWorkFactory unitOfWorkFactory, ISendConversionCache sendConversionCache)
   {
     _unitOfWorkFactory = unitOfWorkFactory;
+    _sendConversionCache = sendConversionCache;
   }
 
   public RootObjectBuilderResult Build(
@@ -38,6 +42,8 @@ public RootObjectBuilderResult Build(
     Collection rootObjectCollection = new(); //TODO: Collections
 
     List<SendConversionResult> results = new(objects.Count);
+    var cacheHitCount = 0;
+
     foreach (MapMember mapMember in objects)
     {
       ct.ThrowIfCancellationRequested();
@@ -47,12 +53,10 @@ public RootObjectBuilderResult Build(
       try
       {
         Base converted;
-        if (
-          !sendInfo.ChangedObjectIds.Contains(applicationId)
-          && sendInfo.ConvertedObjects.TryGetValue(applicationId + sendInfo.ProjectId, out ObjectReference? value)
-        )
+        if (_sendConversionCache.TryGetValue(sendInfo.ProjectId, applicationId, out ObjectReference value))
         {
           converted = value;
+          cacheHitCount++;
         }
         else
         {
@@ -73,6 +77,11 @@ public RootObjectBuilderResult Build(
       onOperationProgressed?.Invoke("Converting", (double)++count / objects.Count);
     }
 
+    // POC: Log would be nice, or can be removed.
+    Debug.WriteLine(
+      $"Cache hit count {cacheHitCount} out of {objects.Count} ({(double)cacheHitCount / objects.Count})"
+    );
+
     return new(rootObjectCollection, results);
   }
 }

DUI3-DX/Connectors/Autocad/Speckle.Connectors.AutocadShared/Bindings/AutocadSendBinding.cs

@@ -10,10 +10,10 @@
 using Speckle.Connectors.Autocad.Operations.Send;
 using Speckle.Connectors.DUI.Exceptions;
 using Speckle.Connectors.Utils.Operations;
-using Speckle.Core.Models;
 using ICancelable = System.Reactive.Disposables.ICancelable;
 using Speckle.Connectors.DUI.Models.Card.SendFilter;
 using Speckle.Connectors.Utils;
+using Speckle.Connectors.Utils.Caching;
 
 namespace Speckle.Connectors.Autocad.Bindings;
 
@@ -29,25 +29,22 @@ public sealed class AutocadSendBinding : ISendBinding, ICancelable
   private readonly CancellationManager _cancellationManager;
   private readonly IUnitOfWorkFactory _unitOfWorkFactory;
   private readonly AutocadSettings _autocadSettings;
+  private readonly ISendConversionCache _sendConversionCache;
 
   /// <summary>
   /// Used internally to aggregate the changed objects' id.
   /// </summary>
   private HashSet<string> ChangedObjectIds { get; set; } = new();
 
-  /// <summary>
-  /// Keeps track of previously converted objects as a dictionary of (applicationId, object reference).
-  /// </summary>
-  private readonly Dictionary<string, ObjectReference> _convertedObjectReferences = new();
-
   public AutocadSendBinding(
     DocumentModelStore store,
     AutocadIdleManager idleManager,
     IBridge parent,
     IEnumerable<ISendFilter> sendFilters,
     CancellationManager cancellationManager,
     AutocadSettings autocadSettings,
-    IUnitOfWorkFactory unitOfWorkFactory
+    IUnitOfWorkFactory unitOfWorkFactory,
+    ISendConversionCache sendConversionCache
   )
   {
     _store = store;
@@ -56,7 +53,7 @@ IUnitOfWorkFactory unitOfWorkFactory
     _autocadSettings = autocadSettings;
     _cancellationManager = cancellationManager;
     _sendFilters = sendFilters.ToList();
-
+    _sendConversionCache = sendConversionCache;
     Parent = parent;
     Commands = new SendBindingUICommands(parent);
 
@@ -95,14 +92,15 @@ private void RunExpirationChecks()
     string[] objectIdsList = ChangedObjectIds.ToArray();
     List<string> expiredSenderIds = new();
 
+    _sendConversionCache.EvictObjects(objectIdsList);
+
     foreach (SenderModelCard modelCard in senders)
     {
       var intersection = modelCard.SendFilter.NotNull().GetObjectIds().Intersect(objectIdsList).ToList();
       bool isExpired = intersection.Count != 0;
       if (isExpired)
       {
         expiredSenderIds.Add(modelCard.ModelCardId.NotNull());
-        modelCard.ChangedObjectIds.UnionWith(intersection);
       }
     }
 
@@ -148,9 +146,7 @@ private async Task SendInternal(string modelCardId)
         modelCard.AccountId.NotNull(),
         modelCard.ProjectId.NotNull(),
         modelCard.ModelId.NotNull(),
-        _autocadSettings.HostAppInfo.Name,
-        _convertedObjectReferences,
-        modelCard.ChangedObjectIds
+        _autocadSettings.HostAppInfo.Name
       );
 
       var sendResult = await uow.Service
@@ -162,15 +158,6 @@ private async Task SendInternal(string modelCardId)
         )
         .ConfigureAwait(false);
 
-      // Store the converted references in memory for future send operations, overwriting the existing values for the given application id.
-      foreach (var kvp in sendResult.ConvertedReferences)
-      {
-        _convertedObjectReferences[kvp.Key + modelCard.ProjectId] = kvp.Value;
-      }
-
-      // It's important to reset the model card's list of changed obj ids so as to ensure we accurately keep track of changes between send operations.
-      modelCard.ChangedObjectIds = new();
-
       Commands.SetModelSendResult(modelCardId, sendResult.RootObjId, sendResult.ConversionResults);
     }
     // Catch here specific exceptions if they related to model card.

DUI3-DX/Connectors/Autocad/Speckle.Connectors.AutocadShared/DependencyInjection/AutocadConnectorModule.cs

@@ -14,6 +14,7 @@
 using Speckle.Connectors.DUI.WebView;
 using Speckle.Connectors.Utils;
 using Speckle.Connectors.Utils.Builders;
+using Speckle.Connectors.Utils.Caching;
 using Speckle.Connectors.Utils.Operations;
 using Speckle.Core.Models.GraphTraversal;
 
@@ -58,5 +59,8 @@ public void Load(SpeckleContainerBuilder builder)
 
     // register send filters
     builder.AddTransient<ISendFilter, AutocadSelectionFilter>();
+
+    // register send conversion cache
+    builder.AddSingleton<ISendConversionCache, SendConversionCache>();
   }
 }

DUI3-DX/Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/Extensions/DocumentExtensions.cs

@@ -1,4 +1,5 @@
 using Autodesk.AutoCAD.DatabaseServices;
+using Autodesk.AutoCAD.Runtime;
 using Speckle.Connectors.Autocad.Operations.Send;
 
 namespace Speckle.Connectors.Autocad.HostApp.Extensions;
@@ -17,13 +18,26 @@ public static List<AutocadRootObject> GetObjects(this Document doc, IEnumerable<
         if (long.TryParse(objectIdHandle, out long parsedId))
         {
           Handle handle = new(parsedId);
-          if (doc.Database.TryGetObjectId(handle, out ObjectId myObjectId))
+          // Note: Fatal crash happens here when objects are deleted, so we need to catch it.
+          try
           {
-            if (tr.GetObject(myObjectId, OpenMode.ForRead) is DBObject dbObject)
+            if (doc.Database.TryGetObjectId(handle, out ObjectId myObjectId))
             {
-              objects.Add(new(dbObject, objectIdHandle));
+              if (tr.GetObject(myObjectId, OpenMode.ForRead) is DBObject dbObject)
+              {
+                objects.Add(new(dbObject, objectIdHandle));
+              }
             }
           }
+          catch (Autodesk.AutoCAD.Runtime.Exception e)
+          {
+            if (e.ErrorStatus == ErrorStatus.WasErased) // Note: TBD if we want to catch more things in here. For now maybe not, but it does seem like this function gets into "crashes the host app territory"
+            {
+              continue;
+            }
+
+            throw;
+          }
         }
       }
     }

DUI3-DX/Connectors/Autocad/Speckle.Connectors.AutocadShared/Operations/Send/AutocadRootObjectBuilder.cs

@@ -1,5 +1,7 @@
-using Autodesk.AutoCAD.DatabaseServices;
+using System.Diagnostics;
+using Autodesk.AutoCAD.DatabaseServices;
 using Speckle.Connectors.Utils.Builders;
+using Speckle.Connectors.Utils.Caching;
 using Speckle.Connectors.Utils.Conversion;
 using Speckle.Connectors.Utils.Operations;
 using Speckle.Converters.Common;
@@ -12,10 +14,12 @@ public class AutocadRootObjectBuilder : IRootObjectBuilder<AutocadRootObject>
 {
   private readonly IRootToSpeckleConverter _converter;
   private readonly string[] _documentPathSeparator = { "\\" };
+  private readonly ISendConversionCache _sendConversionCache;
 
-  public AutocadRootObjectBuilder(IRootToSpeckleConverter converter)
+  public AutocadRootObjectBuilder(IRootToSpeckleConverter converter, ISendConversionCache sendConversionCache)
   {
     _converter = converter;
+    _sendConversionCache = sendConversionCache;
   }
 
   public RootObjectBuilderResult Build(
@@ -40,19 +44,18 @@ public RootObjectBuilderResult Build(
     int count = 0;
 
     List<SendConversionResult> results = new(objects.Count);
+    var cacheHitCount = 0;
     foreach (var (dbObject, applicationId) in objects)
     {
       ct.ThrowIfCancellationRequested();
 
       try
       {
         Base converted;
-        if (
-          !sendInfo.ChangedObjectIds.Contains(applicationId)
-          && sendInfo.ConvertedObjects.TryGetValue(applicationId + sendInfo.ProjectId, out ObjectReference value) // POC: Interface out constructing keys here to use it otherplaces with a same code. -> https://spockle.atlassian.net/browse/CNX-9313
-        )
+        if (_sendConversionCache.TryGetValue(sendInfo.ProjectId, applicationId, out ObjectReference value))
         {
           converted = value;
+          cacheHitCount++;
         }
         else
         {
@@ -84,6 +87,11 @@ public RootObjectBuilderResult Build(
       onOperationProgressed?.Invoke("Converting", (double)++count / objects.Count);
     }
 
+    // POC: Log would be nice, or can be removed.
+    Debug.WriteLine(
+      $"Cache hit count {cacheHitCount} out of {objects.Count} ({(double)cacheHitCount / objects.Count})"
+    );
+
     return new(modelWithLayers, results);
   }
 }

DUI3-DX/Connectors/Revit/Speckle.Connectors.RevitShared/Bindings/BasicConnectorBindingRevit.cs

@@ -88,7 +88,7 @@ public void HighlightModel(string modelCardId)
     SenderModelCard model = (SenderModelCard)_store.GetModelById(modelCardId);
 
     var elementIds = model.SendFilter.NotNull().GetObjectIds().Select(ElementId.Parse).ToList();
-    if (elementIds.Count != 0)
+    if (elementIds.Count == 0)
     {
       Commands.SetModelError(modelCardId, new InvalidOperationException("No objects found to highlight."));
       return;