From 7628386259072208861913757f7752d9b64de306 Mon Sep 17 00:00:00 2001
From: Justin Perez <justinperez@microsoft.com>
Date: Mon, 24 Apr 2023 11:17:10 -0700
Subject: [PATCH] perf: process experiments in parallel

---
 .../Experiments/ExperimentService.cs          | 16 ++++----
 .../Experiments/Models/ExperimentDiff.cs      | 39 +++++++++++--------
 2 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/ExperimentService.cs b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/ExperimentService.cs
index 93f79c2fa..6fb68b4fb 100644
--- a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/ExperimentService.cs
+++ b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/ExperimentService.cs
@@ -98,16 +98,14 @@ public async Task FinishAsync()
 
             var diff = new ExperimentDiff(controlComponents, experimentComponents);
 
-            foreach (var processor in this.experimentProcessors)
+            try
             {
-                try
-                {
-                    await processor.ProcessExperimentAsync(config, diff);
-                }
-                catch (Exception e)
-                {
-                    this.logger.LogWarning(e, "Error processing experiment {Experiment}", config.Name);
-                }
+                var tasks = this.experimentProcessors.Select(x => x.ProcessExperimentAsync(config, diff));
+                await Task.WhenAll(tasks);
+            }
+            catch (Exception e)
+            {
+                this.logger.LogError(e, "Error processing experiment {Experiment}", config.Name);
             }
         }
     }
diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Models/ExperimentDiff.cs b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Models/ExperimentDiff.cs
index 496426d51..b0a2a75cb 100644
--- a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Models/ExperimentDiff.cs
+++ b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/Models/ExperimentDiff.cs
@@ -1,6 +1,7 @@
 namespace Microsoft.ComponentDetection.Orchestrator.Experiments.Models;
 
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.Linq;
 
 /// <summary>
@@ -20,12 +21,12 @@ public ExperimentDiff(
         var oldComponentDictionary = controlGroupComponents.ToDictionary(x => x.Id);
         var newComponentDictionary = experimentGroupComponents.ToDictionary(x => x.Id);
 
-        this.AddedIds = newComponentDictionary.Keys.Except(oldComponentDictionary.Keys).ToList();
-        this.RemovedIds = oldComponentDictionary.Keys.Except(newComponentDictionary.Keys).ToList();
+        this.AddedIds = newComponentDictionary.Keys.Except(oldComponentDictionary.Keys).ToImmutableList();
+        this.RemovedIds = oldComponentDictionary.Keys.Except(newComponentDictionary.Keys).ToImmutableList();
 
-        this.DevelopmentDependencyChanges = new List<DevelopmentDependencyChange>();
-        this.AddedRootIds = new Dictionary<string, HashSet<string>>();
-        this.RemovedRootIds = new Dictionary<string, HashSet<string>>();
+        var developmentDependencyChanges = new List<DevelopmentDependencyChange>();
+        var addedRootIds = new Dictionary<string, IReadOnlySet<string>>();
+        var removedRootIds = new Dictionary<string, IReadOnlySet<string>>();
 
         // Need performance benchmark to see if this is worth parallelization
         foreach (var id in newComponentDictionary.Keys.Intersect(oldComponentDictionary.Keys))
@@ -35,53 +36,57 @@ public ExperimentDiff(
 
             if (oldComponent.DevelopmentDependency != newComponent.DevelopmentDependency)
             {
-                this.DevelopmentDependencyChanges.Add(new DevelopmentDependencyChange(
+                developmentDependencyChanges.Add(new DevelopmentDependencyChange(
                     id,
                     oldComponent.DevelopmentDependency,
                     newComponent.DevelopmentDependency));
             }
 
-            var addedRootIds = newComponent.RootIds.Except(oldComponent.RootIds).ToHashSet();
-            var removedRootIds = oldComponent.RootIds.Except(newComponent.RootIds).ToHashSet();
+            var newRoots = newComponent.RootIds.Except(oldComponent.RootIds).ToImmutableHashSet();
+            var removedRoots = oldComponent.RootIds.Except(newComponent.RootIds).ToImmutableHashSet();
 
-            if (addedRootIds.Count > 0)
+            if (newRoots.Count > 0)
             {
-                this.AddedRootIds[id] = addedRootIds;
+                addedRootIds[id] = newRoots;
             }
 
-            if (removedRootIds.Count > 0)
+            if (removedRoots.Count > 0)
             {
-                this.RemovedRootIds[id] = removedRootIds;
+                removedRootIds[id] = removedRoots;
             }
         }
+
+        this.DevelopmentDependencyChanges = developmentDependencyChanges.AsReadOnly();
+        this.AddedRootIds = addedRootIds.ToImmutableDictionary();
+        this.RemovedRootIds = removedRootIds.ToImmutableDictionary();
     }
 
     /// <summary>
     /// Gets a list of component IDs that were present in the experimental group but not the control group.
     /// </summary>
-    public List<string> AddedIds { get; }
+    public IReadOnlyCollection<string> AddedIds { get; }
 
     /// <summary>
     /// Gets a list of component IDs that were present in the control group but not the experimental group.
     /// </summary>
-    public List<string> RemovedIds { get; }
+    public IReadOnlyCollection<string> RemovedIds { get; }
 
     /// <summary>
     /// Gets a list of changes to the development dependency status of components.
     /// </summary>
-    public List<DevelopmentDependencyChange> DevelopmentDependencyChanges { get; }
+    public IReadOnlyCollection<DevelopmentDependencyChange> DevelopmentDependencyChanges { get; }
 
     /// <summary>
     /// Gets a dictionary of component IDs to the set of root IDs that were added to the component. The component ID
     /// is the key.
     /// </summary>
-    public Dictionary<string, HashSet<string>> AddedRootIds { get; }
+    public IReadOnlyDictionary<string, IReadOnlySet<string>> AddedRootIds { get; }
 
     /// <summary>
     /// Gets a dictionary of component IDs to the set of root IDs that were removed from the component. The component
     /// ID is the key.
     /// </summary>
-    public Dictionary<string, HashSet<string>> RemovedRootIds { get; }
+    public IReadOnlyDictionary<string, IReadOnlySet<string>> RemovedRootIds { get; }
 
     /// <summary>
     /// Stores information about a change to the development dependency status of a component.