Use Sets and Freezing to make conversions faster (#430)

* Use Sets and Freezing to make conversions faster

* fmt

* move class to own file

Connectors/ArcGIS/Speckle.Connectors.ArcGIS3/Operations/Receive/ArcGISHostObjectBuilder.cs

@@ -89,7 +89,7 @@ CancellationToken cancellationToken
     }
 
     int count = 0;
-    List<LocalToGlobalMap> objectsToConvert = GetObjectsToConvert(rootObject);
+    IReadOnlyCollection<LocalToGlobalMap> objectsToConvert = GetObjectsToConvert(rootObject);
     Dictionary<TraversalContext, ObjectConversionTracker> conversionTracker = new();
 
     // 1. convert everything
@@ -242,7 +242,7 @@ await _featureClassUtils
     return new(bakedObjectIds, results);
   }
 
-  private List<LocalToGlobalMap> GetObjectsToConvert(Base rootObject)
+  private IReadOnlyCollection<LocalToGlobalMap> GetObjectsToConvert(Base rootObject)
   {
     // keep GISlayers in the list, because they are still needed to extract CRS of the commit (code below)
     List<TraversalContext> objectsToConvertTc = _traverseFunction.Traverse(rootObject).ToList();

Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadGroupBaker.cs

@@ -2,6 +2,7 @@
 using Microsoft.Extensions.Logging;
 using Speckle.Connectors.Common.Conversion;
 using Speckle.Sdk;
+using Speckle.Sdk.Dependencies;
 using Speckle.Sdk.Models.Proxies;
 
 namespace Speckle.Connectors.Autocad.HostApp;
@@ -29,12 +30,12 @@ public AutocadGroupBaker(AutocadContext autocadContext, ILogger<AutocadGroupBake
   /// <param name="applicationIdMap"></param>
   /// <returns></returns>
   // TODO: Oguzhan! Do not report here too! But this is TBD that we don't know the shape of the report yet.
-  public List<ReceiveConversionResult> CreateGroups(
+  public IReadOnlyCollection<ReceiveConversionResult> CreateGroups(
     IEnumerable<GroupProxy> groupProxies,
-    Dictionary<string, List<Entity>> applicationIdMap
+    Dictionary<string, IReadOnlyCollection<Entity>> applicationIdMap
   )
   {
-    List<ReceiveConversionResult> results = new();
+    HashSet<ReceiveConversionResult> results = new();
 
     using var groupCreationTransaction =
       Application.DocumentManager.CurrentDocument.Database.TransactionManager.StartTransaction();
@@ -75,6 +76,6 @@ Dictionary<string, List<Entity>> applicationIdMap
 
     groupCreationTransaction.Commit();
 
-    return results;
+    return results.Freeze();
   }
 }

Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadInstanceBaker.cs

@@ -12,6 +12,7 @@
 using Speckle.Sdk;
 using Speckle.Sdk.Common;
 using Speckle.Sdk.Common.Exceptions;
+using Speckle.Sdk.Dependencies;
 using Speckle.Sdk.Models;
 using Speckle.Sdk.Models.Collections;
 using Speckle.Sdk.Models.Instances;
@@ -22,7 +23,7 @@ namespace Speckle.Connectors.Autocad.HostApp;
 /// <summary>
 /// Expects to be a scoped dependency receive operation.
 /// </summary>
-public class AutocadInstanceBaker : IInstanceBaker<List<Entity>>
+public class AutocadInstanceBaker : IInstanceBaker<IReadOnlyCollection<Entity>>
 {
   private readonly AutocadLayerBaker _layerBaker;
   private readonly AutocadColorBaker _colorBaker;
@@ -50,8 +51,8 @@ IConverterSettingsStore<AutocadConversionSettings> converterSettings
 
   [SuppressMessage("Maintainability", "CA1506:Avoid excessive class coupling")]
   public BakeResult BakeInstances(
-    IReadOnlyCollection<(Collection[] collectionPath, IInstanceComponent obj)> instanceComponents,
-    Dictionary<string, List<Entity>> applicationIdMap,
+    ICollection<(Collection[] collectionPath, IInstanceComponent obj)> instanceComponents,
+    Dictionary<string, IReadOnlyCollection<Entity>> applicationIdMap,
     string baseLayerName,
     IProgress<CardProgress> onOperationProgressed
   )
@@ -64,9 +65,9 @@ IProgress<CardProgress> onOperationProgressed
     var definitionIdAndApplicationIdMap = new Dictionary<string, ObjectId>();
 
     using var transaction = Application.DocumentManager.CurrentDocument.Database.TransactionManager.StartTransaction();
-    var conversionResults = new List<ReceiveConversionResult>();
-    var createdObjectIds = new List<string>();
-    var consumedObjectIds = new List<string>();
+    var conversionResults = new HashSet<ReceiveConversionResult>();
+    var createdObjectIds = new HashSet<string>();
+    var consumedObjectIds = new HashSet<string>();
     var count = 0;
 
     foreach (var (collectionPath, instanceOrDefinition) in sortedInstanceComponents)
@@ -79,7 +80,9 @@ IProgress<CardProgress> onOperationProgressed
         {
           // TODO: create definition (block table record)
           var constituentEntities = definitionProxy
-            .objects.Select(id => applicationIdMap.TryGetValue(id, out List<Entity>? value) ? value : null)
+            .objects.Select(id =>
+              applicationIdMap.TryGetValue(id, out IReadOnlyCollection<Entity>? value) ? value : null
+            )
             .Where(x => x is not null)
             .SelectMany(ent => ent!)
             .ToList();
@@ -109,8 +112,8 @@ IProgress<CardProgress> onOperationProgressed
           definitionIdAndApplicationIdMap[definitionProxy.applicationId] = id;
           transaction.AddNewlyCreatedDBObject(record, true);
           var consumedEntitiesHandleValues = constituentEntities.Select(ent => ent.GetSpeckleApplicationId()).ToArray();
-          consumedObjectIds.AddRange(consumedEntitiesHandleValues);
-          createdObjectIds.RemoveAll(newId => consumedEntitiesHandleValues.Contains(newId));
+          consumedObjectIds.UnionWith(consumedEntitiesHandleValues);
+          createdObjectIds.RemoveWhere(newId => consumedEntitiesHandleValues.Contains(newId));
         }
         else if (
           instanceOrDefinition is InstanceProxy instanceProxy
@@ -167,7 +170,7 @@ instanceOrDefinition is InstanceProxy instanceProxy
     }
 
     transaction.Commit();
-    return new(createdObjectIds, consumedObjectIds, conversionResults);
+    return new(createdObjectIds.Freeze(), consumedObjectIds.Freeze(), conversionResults.Freeze());
   }
 
   /// <summary>

Connectors/Autocad/Speckle.Connectors.AutocadShared/HostApp/AutocadMaterialBaker.cs

@@ -92,7 +92,7 @@ public void PurgeMaterials(string namePrefix)
   }
 
   public void ParseAndBakeRenderMaterials(
-    List<RenderMaterialProxy> materialProxies,
+    IReadOnlyCollection<RenderMaterialProxy> materialProxies,
     string baseLayerPrefix,
     IProgress<CardProgress> onOperationProgressed
   )

Connectors/Autocad/Speckle.Connectors.AutocadShared/Operations/Receive/AutocadHostObjectBuilder.cs

@@ -3,10 +3,12 @@
 using Speckle.Connectors.Autocad.HostApp.Extensions;
 using Speckle.Connectors.Common.Builders;
 using Speckle.Connectors.Common.Conversion;
+using Speckle.Connectors.Common.Extensions;
 using Speckle.Connectors.Common.Operations;
 using Speckle.Connectors.Common.Operations.Receive;
 using Speckle.Converters.Common;
 using Speckle.Sdk;
+using Speckle.Sdk.Dependencies;
 using Speckle.Sdk.Models;
 using Speckle.Sdk.Models.Collections;
 using Speckle.Sdk.Models.Instances;
@@ -123,21 +125,21 @@ IProgress<CardProgress> onOperationProgressed
     }
 
     // 5 - Convert atomic objects
-    List<ReceiveConversionResult> results = new();
-    List<string> bakedObjectIds = new();
-    Dictionary<string, List<Entity>> applicationIdMap = new();
+    HashSet<ReceiveConversionResult> results = new();
+    HashSet<string> bakedObjectIds = new();
+    Dictionary<string, IReadOnlyCollection<Entity>> applicationIdMap = new();
     var count = 0;
     foreach (var (layerPath, atomicObject) in atomicObjectsWithPath)
     {
       string objectId = atomicObject.applicationId ?? atomicObject.id;
       onOperationProgressed.Report(new("Converting objects", (double)++count / atomicObjects.Count));
       try
       {
-        List<Entity> convertedObjects = ConvertObject(atomicObject, layerPath, baseLayerPrefix);
+        IReadOnlyCollection<Entity> convertedObjects = ConvertObject(atomicObject, layerPath, baseLayerPrefix);
 
         applicationIdMap[objectId] = convertedObjects;
 
-        results.AddRange(
+        results.UnionWith(
           convertedObjects.Select(e => new ReceiveConversionResult(
             Status.SUCCESS,
             atomicObject,
@@ -146,7 +148,7 @@ IProgress<CardProgress> onOperationProgressed
           ))
         );
 
-        bakedObjectIds.AddRange(convertedObjects.Select(e => e.GetSpeckleApplicationId()));
+        bakedObjectIds.UnionWith(convertedObjects.Select(e => e.GetSpeckleApplicationId()));
       }
       catch (Exception ex) when (!ex.IsFatal())
       {
@@ -162,19 +164,19 @@ IProgress<CardProgress> onOperationProgressed
       onOperationProgressed
     );
 
-    bakedObjectIds.RemoveAll(id => consumedObjectIds.Contains(id));
-    bakedObjectIds.AddRange(createdInstanceIds);
-    results.RemoveAll(result => result.ResultId != null && consumedObjectIds.Contains(result.ResultId));
-    results.AddRange(instanceConversionResults);
+    bakedObjectIds.RemoveWhere(id => consumedObjectIds.Contains(id));
+    bakedObjectIds.UnionWith(createdInstanceIds);
+    results.RemoveWhere(result => result.ResultId != null && consumedObjectIds.Contains(result.ResultId));
+    results.UnionWith(instanceConversionResults);
 
     // 7 - Create groups
     if (unpackedRoot.GroupProxies != null)
     {
-      List<ReceiveConversionResult> groupResults = _groupBaker.CreateGroups(
+      IReadOnlyCollection<ReceiveConversionResult> groupResults = _groupBaker.CreateGroups(
         unpackedRoot.GroupProxies,
         applicationIdMap
       );
-      results.AddRange(groupResults);
+      results.UnionWith(groupResults);
     }
 
     return new HostObjectBuilderResult(bakedObjectIds, results);
@@ -187,10 +189,10 @@ private void PreReceiveDeepClean(string baseLayerPrefix)
     _materialBaker.PurgeMaterials(baseLayerPrefix);
   }
 
-  private List<Entity> ConvertObject(Base obj, Collection[] layerPath, string baseLayerNamePrefix)
+  private IReadOnlyCollection<Entity> ConvertObject(Base obj, Collection[] layerPath, string baseLayerNamePrefix)
   {
     string layerName = _layerBaker.CreateLayerForReceive(layerPath, baseLayerNamePrefix);
-    var convertedEntities = new List<Entity>();
+    var convertedEntities = new HashSet<Entity>();
 
     using var tr = Application.DocumentManager.CurrentDocument.Database.TransactionManager.StartTransaction();
 
@@ -206,11 +208,11 @@ private List<Entity> ConvertObject(Base obj, Collection[] layerPath, string base
     else if (converted is IEnumerable<(object, Base)> fallbackConversionResult)
     {
       var bakedFallbackEntities = BakeObjectsAsGroup(fallbackConversionResult, obj, layerName, baseLayerNamePrefix);
-      convertedEntities.AddRange(bakedFallbackEntities);
+      convertedEntities.UnionWith(bakedFallbackEntities);
     }
 
     tr.Commit();
-    return convertedEntities;
+    return convertedEntities.Freeze();
   }
 
   private Entity BakeObject(Entity entity, Base originalObject, string layerName, Base? parentObject = null)

Connectors/Revit/Speckle.Connectors.RevitShared/HostApp/RevitMaterialBaker.cs

@@ -108,7 +108,7 @@ public void MapLayersRenderMaterials(RootObjectUnpackerResult unpackedRoot)
   /// <param name="baseLayerName"></param>
   /// <returns></returns>
   public Dictionary<string, ElementId> BakeMaterials(
-    List<RenderMaterialProxy> speckleRenderMaterialProxies,
+    IReadOnlyCollection<RenderMaterialProxy> speckleRenderMaterialProxies,
     string baseLayerName
   )
   {

Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Receive/RevitHostObjectBuilder.cs

@@ -34,7 +34,7 @@ internal sealed class RevitHostObjectBuilder : IHostObjectBuilder, IDisposable
   private readonly RevitMaterialBaker _materialBaker;
   private readonly ILogger<RevitHostObjectBuilder> _logger;
   private readonly ITypedConverter<
-    (Base atomicObject, List<Matrix4x4> matrix),
+    (Base atomicObject, IReadOnlyCollection<Matrix4x4> matrix),
     DirectShape
   > _localToGlobalDirectShapeConverter;
 
@@ -52,7 +52,10 @@ public RevitHostObjectBuilder(
     RootObjectUnpacker rootObjectUnpacker,
     ILogger<RevitHostObjectBuilder> logger,
     RevitToHostCacheSingleton revitToHostCacheSingleton,
-    ITypedConverter<(Base atomicObject, List<Matrix4x4> matrix), DirectShape> localToGlobalDirectShapeConverter
+    ITypedConverter<
+      (Base atomicObject, IReadOnlyCollection<Matrix4x4> matrix),
+      DirectShape
+    > localToGlobalDirectShapeConverter
   )
   {
     _converter = converter;
@@ -161,7 +164,7 @@ CancellationToken cancellationToken
     HostObjectBuilderResult builderResult,
     List<(DirectShape res, string applicationId)> postBakePaintTargets
   ) BakeObjects(
-    List<LocalToGlobalMap> localToGlobalMaps,
+    IReadOnlyCollection<LocalToGlobalMap> localToGlobalMaps,
     IProgress<CardProgress> onOperationProgressed,
     CancellationToken cancellationToken
   )
@@ -195,7 +198,7 @@ localToGlobalMap.AtomicObject is ITransformable transformable // and ICurve
           }
 
           localToGlobalMap.AtomicObject = (newTransformable as Base)!;
-          localToGlobalMap.Matrix = new(); // flush out the list, as we've applied the transforms already
+          localToGlobalMap.Matrix = new HashSet<Matrix4x4>(); // flush out the list, as we've applied the transforms already
         }
 
         // actual conversion happens here!

Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoColorBaker.cs

@@ -23,7 +23,7 @@ public RhinoColorBaker(ILogger<RhinoColorBaker> logger)
   /// Parse Color Proxies and stores in ObjectColorsIdMap the relationship between object ids and colors
   /// </summary>
   /// <param name="colorProxies"></param>
-  public void ParseColors(List<ColorProxy> colorProxies)
+  public void ParseColors(IReadOnlyCollection<ColorProxy> colorProxies)
   {
     foreach (ColorProxy colorProxy in colorProxies)
     {

Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoGroupBaker.cs

@@ -25,8 +25,8 @@ ISdkActivityFactory activityFactory
   }
 
   public void BakeGroups(
-    List<GroupProxy> groupProxies,
-    Dictionary<string, List<string>> applicationIdMap,
+    IReadOnlyCollection<GroupProxy> groupProxies,
+    Dictionary<string, IReadOnlyCollection<string>> applicationIdMap,
     string baseLayerName
   )
   {

Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoInstanceBaker.cs

@@ -16,7 +16,7 @@
 
 namespace Speckle.Connectors.Rhino.HostApp;
 
-public class RhinoInstanceBaker : IInstanceBaker<List<string>>
+public class RhinoInstanceBaker : IInstanceBaker<IReadOnlyCollection<string>>
 {
   private readonly RhinoMaterialBaker _materialBaker;
   private readonly RhinoLayerBaker _layerBaker;
@@ -43,8 +43,8 @@ ILogger<RhinoInstanceBaker> logger
   /// <param name="applicationIdMap">A dict mapping { original application id -> [resulting application ids post conversion] }</param>
   /// <param name="onOperationProgressed"></param>
   public BakeResult BakeInstances(
-    IReadOnlyCollection<(Collection[] collectionPath, IInstanceComponent obj)> instanceComponents,
-    Dictionary<string, List<string>> applicationIdMap,
+    ICollection<(Collection[] collectionPath, IInstanceComponent obj)> instanceComponents,
+    Dictionary<string, IReadOnlyCollection<string>> applicationIdMap,
     string baseLayerName,
     IProgress<CardProgress> onOperationProgressed
   )
@@ -59,9 +59,9 @@ IProgress<CardProgress> onOperationProgressed
     var definitionIdAndApplicationIdMap = new Dictionary<string, int>();
 
     var count = 0;
-    var conversionResults = new List<ReceiveConversionResult>();
-    var createdObjectIds = new List<string>();
-    var consumedObjectIds = new List<string>();
+    var conversionResults = new HashSet<ReceiveConversionResult>();
+    var createdObjectIds = new HashSet<string>();
+    var consumedObjectIds = new HashSet<string>();
     foreach (var (layerCollection, instanceOrDefinition) in sortedInstanceComponents)
     {
       onOperationProgressed.Report(new("Converting blocks", (double)++count / sortedInstanceComponents.Count));
@@ -70,10 +70,10 @@ IProgress<CardProgress> onOperationProgressed
         if (instanceOrDefinition is InstanceDefinitionProxy definitionProxy)
         {
           var currentApplicationObjectsIds = definitionProxy
-            .objects.Select(x => applicationIdMap.TryGetValue(x, out List<string>? value) ? value : null)
+            .objects.Select(x => applicationIdMap.TryGetValue(x, out IReadOnlyCollection<string>? value) ? value : null)
             .Where(x => x is not null)
             .SelectMany(id => id.NotNull())
-            .ToList();
+            .ToHashSet();
 
           var definitionGeometryList = new List<GeometryBase>();
           var attributes = new List<ObjectAttributes>();
@@ -114,8 +114,8 @@ IProgress<CardProgress> onOperationProgressed
 
           // Rhino deletes original objects on block creation - we should do the same.
           doc.Objects.Delete(currentApplicationObjectsIds.Select(stringId => new Guid(stringId)), false);
-          consumedObjectIds.AddRange(currentApplicationObjectsIds);
-          createdObjectIds.RemoveAll(id => consumedObjectIds.Contains(id)); // in case we've consumed some existing instances
+          consumedObjectIds.UnionWith(currentApplicationObjectsIds);
+          createdObjectIds.RemoveWhere(id => consumedObjectIds.Contains(id)); // in case we've consumed some existing instances
         }
 
         if (

Connectors/Rhino/Speckle.Connectors.RhinoShared/HostApp/RhinoMaterialBaker.cs

@@ -28,7 +28,7 @@ ILogger<RhinoMaterialBaker> logger
   /// </summary>
   public Dictionary<string, int> ObjectIdAndMaterialIndexMap { get; } = new();
 
-  public void BakeMaterials(List<RenderMaterialProxy> speckleRenderMaterialProxies, string baseLayerName)
+  public void BakeMaterials(IReadOnlyCollection<RenderMaterialProxy> speckleRenderMaterialProxies, string baseLayerName)
   {
     var doc = _converterSettings.Current.Document; // POC: too much right now to interface around
     // List<ReceiveConversionResult> conversionResults = new(); // TODO: return this guy