Improve performance of RetrievePackageDimensions (#442)

Author: zivkan

src/Stats.ImportAzureCdnStatistics/Dimensions/PackageDimension.cs

@@ -6,39 +6,43 @@
 
 namespace Stats.ImportAzureCdnStatistics
 {
-    public class PackageDimension
+    public class PackageDimension : IEquatable<PackageDimension>
     {
         private static CultureInfo EnUsCulture = CultureInfo.GetCultureInfo("en-US");
 
+        private readonly string _comparablePackageId;
+        private readonly int _hashCode;
+
         public PackageDimension(string packageId, string packageVersion)
         {
             PackageId = packageId;
             PackageVersion = packageVersion;
-        }
-
-        protected bool Equals(PackageDimension other)
-        {
-            return string.Equals(PackageId.ToLower(EnUsCulture), other.PackageId.ToLower(EnUsCulture)) && string.Equals(PackageVersion, other.PackageVersion, StringComparison.OrdinalIgnoreCase);
-        }
+            _comparablePackageId = packageId.ToLower(EnUsCulture);
 
-        public override int GetHashCode()
-        {
             unchecked
             {
-                return ((PackageId != null ? PackageId.ToLower(EnUsCulture).GetHashCode() : 0)*397) ^ (PackageVersion != null ? PackageVersion.GetHashCode() : 0);
+                _hashCode = ((PackageId != null ? _comparablePackageId.GetHashCode() : 0) * 397) ^ (PackageVersion != null ? PackageVersion.GetHashCode() : 0);
             }
         }
 
+        public bool Equals(PackageDimension other)
+        {
+            if (other == null) return false;
+            return string.Equals(_comparablePackageId, other._comparablePackageId) && string.Equals(PackageVersion, other.PackageVersion, StringComparison.OrdinalIgnoreCase);
+        }
+
+        public override int GetHashCode() => _hashCode;
+
         public int Id { get; set; }
         public string PackageId { get; }
         public string PackageVersion { get; }
 
         public override bool Equals(object obj)
         {
-            if (ReferenceEquals(null, obj)) return false;
             if (ReferenceEquals(this, obj)) return true;
-            if (obj.GetType() != GetType()) return false;
-            return Equals((PackageDimension) obj);
+            var other = obj as PackageDimension;
+            if (other == null) return false;
+            return Equals(other);
         }
     }
 }

src/Stats.ImportAzureCdnStatistics/Warehouse.cs

@@ -22,7 +22,7 @@ internal class Warehouse
         private readonly TimeSpan _retryDelay = TimeSpan.FromSeconds(5);
         private readonly ILogger _logger;
         private readonly ISqlConnectionFactory _statisticsDbConnectionFactory;
-        private readonly IList<PackageDimension> _cachedPackageDimensions = new List<PackageDimension>();
+        private readonly IDictionary<PackageDimension, PackageDimension> _cachedPackageDimensions = new Dictionary<PackageDimension, PackageDimension>();
         private readonly IList<ToolDimension> _cachedToolDimensions = new List<ToolDimension>();
         private readonly IDictionary<string, int> _cachedClientDimensions = new Dictionary<string, int>();
         private readonly IDictionary<string, int> _cachedPlatformDimensions = new Dictionary<string, int>();
@@ -687,26 +687,28 @@ private async Task<IReadOnlyCollection<ToolDimension>> RetrieveToolDimensions(IR
 
         private async Task<IReadOnlyCollection<PackageDimension>> RetrievePackageDimensions(IReadOnlyCollection<PackageStatistics> sourceData, SqlConnection connection)
         {
-            var packages = sourceData
-                .Select(e => new PackageDimension(e.PackageId, e.PackageVersion))
-                .Distinct()
-                .ToList();
-
             var results = new List<PackageDimension>();
-            if (!packages.Any())
-            {
-                return results;
-            }
+            var nonCachedPackageDimensions = new List<PackageDimension>();
+            var sourceDataPackages = new HashSet<PackageDimension>();
 
-            results.AddRange(_cachedPackageDimensions
-                .Where(p1 => packages
-                    .FirstOrDefault(p2 =>
-                        string.Equals(p1.PackageId, p2.PackageId, StringComparison.OrdinalIgnoreCase)
-                        && string.Equals(p1.PackageVersion, p2.PackageVersion, StringComparison.OrdinalIgnoreCase)) != null
-                    )
-                );
+            foreach (var sourceStatistics in sourceData)
+            {
+                var sourcePackage = new PackageDimension(sourceStatistics.PackageId, sourceStatistics.PackageVersion);
+                if (!sourceDataPackages.Add(sourcePackage))
+                {
+                    // This package has already been seen in the sourceData
+                    continue;
+                }
 
-            var nonCachedPackageDimensions = packages.Except(results).ToList();
+                if (_cachedPackageDimensions.TryGetValue(sourcePackage, out var cachedPackage))
+                {
+                    results.Add(cachedPackage);
+                }
+                else
+                {
+                    nonCachedPackageDimensions.Add(sourcePackage);
+                }
+            }
 
             if (nonCachedPackageDimensions.Any())
             {
@@ -730,15 +732,8 @@ private async Task<IReadOnlyCollection<PackageDimension>> RetrievePackageDimensi
                             Id = dataReader.GetInt32(0)
                         };
 
-                        if (!results.Contains(package))
-                        {
-                            results.Add(package);
-                        }
-
-                        if (!_cachedPackageDimensions.Contains(package))
-                        {
-                            _cachedPackageDimensions.Add(package);
-                        }
+                        results.Add(package);
+                        _cachedPackageDimensions[package] = package;
                     }
                 }
             }