Performance: Adds finalizer optimizations in a few places (#1857)

There are a few places where finalizers are used and most of them are actually not useful and just cause overhead. But there is one class which currently doesn't have a finalizer, but must have one - SecureStringHMACSHA256Helper holds native resources
Azure · Sep 18, 2020 · fc46b1d · fc46b1d
1 parent 40ceaff
commit fc46b1d
Show file tree

Hide file tree

Showing 7 changed files with 12 additions and 43 deletions.
diff --git a/Microsoft.Azure.Cosmos/src/Batch/TransactionalBatchResponse.cs b/Microsoft.Azure.Cosmos/src/Batch/TransactionalBatchResponse.cs
@@ -22,8 +22,6 @@ namespace Microsoft.Azure.Cosmos
     public class TransactionalBatchResponse : IReadOnlyList<TransactionalBatchOperationResult>, IDisposable
 #pragma warning restore CA1710 // Identifiers should have correct suffix
     {
-        private bool isDisposed;
-
         private List<TransactionalBatchOperationResult> results;
 
         /// <summary>
@@ -198,7 +196,6 @@ virtual IEnumerable<string> GetActivityIds()
         public void Dispose()
         {
             this.Dispose(true);
-            GC.SuppressFinalize(this);
         }
 
         /// <inheritdoc />
@@ -385,9 +382,8 @@ private static async Task<TransactionalBatchResponse> PopulateFromContentAsync(
         /// <param name="disposing">Indicates whether to dispose managed resources or not.</param>
         protected virtual void Dispose(bool disposing)
         {
-            if (disposing && !this.isDisposed)
+            if (disposing)
             {
-                this.isDisposed = true;
                 if (this.Operations != null)
                 {
                     foreach (ItemBatchOperation operation in this.Operations)

diff --git a/Microsoft.Azure.Cosmos/src/DocumentClient.cs b/Microsoft.Azure.Cosmos/src/DocumentClient.cs
@@ -1376,11 +1376,7 @@ public void Dispose()
                 this.storeClientFactory = null;
             }
 
-            if (this.AddressResolver != null)
-            {
-                this.AddressResolver.Dispose();
-                this.AddressResolver = null;
-            }
+            this.AddressResolver = null;
 
             if (this.httpClient != null)
             {

diff --git a/Microsoft.Azure.Cosmos/src/GatewayStoreModel.cs b/Microsoft.Azure.Cosmos/src/GatewayStoreModel.cs
@@ -145,7 +145,6 @@ public virtual async Task<AccountProperties> GetDatabaseAccountAsync(Func<ValueT
         public void Dispose()
         {
             this.Dispose(true);
-            GC.SuppressFinalize(this);
         }
 
         private void CaptureSessionToken(

diff --git a/Microsoft.Azure.Cosmos/src/Routing/GatewayAddressCache.cs b/Microsoft.Azure.Cosmos/src/Routing/GatewayAddressCache.cs
@@ -22,7 +22,7 @@ namespace Microsoft.Azure.Cosmos.Routing
     using Microsoft.Azure.Documents.Rntbd;
     using Microsoft.Azure.Documents.Routing;
 
-    internal class GatewayAddressCache : IAddressCache, IDisposable
+    internal class GatewayAddressCache : IAddressCache
     {
         private const string protocolFilterFormat = "{0} eq {1}";
 
@@ -515,11 +515,6 @@ await ClientExtensions.ParseResponseAsync(httpResponseMessage))
             }
         }
 
-        public void Dispose()
-        {
-            GC.SuppressFinalize(this);
-        }
-
         internal Tuple<PartitionKeyRangeIdentity, PartitionAddressInformation> ToPartitionAddressAndRange(string collectionRid, IList<Address> addresses)
         {
             Address address = addresses.First();

diff --git a/Microsoft.Azure.Cosmos/src/Routing/GlobalAddressResolver.cs b/Microsoft.Azure.Cosmos/src/Routing/GlobalAddressResolver.cs
@@ -20,7 +20,7 @@ namespace Microsoft.Azure.Cosmos.Routing
     /// AddressCache implementation for client SDK. Supports cross region address routing based on
     /// avaialbility and preference list.
     /// </summary>
-    internal sealed class GlobalAddressResolver : IAddressResolver, IDisposable
+    internal sealed class GlobalAddressResolver : IAddressResolver
     {
         private const int MaxBackupReadRegions = 3;
 
@@ -153,14 +153,6 @@ private IAddressResolver GetAddressResolver(DocumentServiceRequest request)
             return this.GetOrAddEndpoint(endpoint).AddressResolver;
         }
 
-        public void Dispose()
-        {
-            foreach (EndpointCache endpointCache in this.addressCacheByEndpoint.Values)
-            {
-                endpointCache.AddressCache.Dispose();
-            }
-        }
-
         private EndpointCache GetOrAddEndpoint(Uri endpoint)
         {
             // The GetorAdd is followed by a call to .Count which in a ConcurrentDictionary

diff --git a/Microsoft.Azure.Cosmos/src/SecureStringHMACSHA256Helper.cs b/Microsoft.Azure.Cosmos/src/SecureStringHMACSHA256Helper.cs
@@ -53,20 +53,19 @@ public void Dispose()
             GC.SuppressFinalize(this);
         }
 
+        ~SecureStringHMACSHA256Helper() => this.Dispose(false);
+
         private void Dispose(bool disposing)
         {
-            if (disposing)
+            if (this.algorithmHandle != IntPtr.Zero)
             {
-                if (this.algorithmHandle != null)
+                int status = NativeMethods.BCryptCloseAlgorithmProvider(this.algorithmHandle, 0);
+                if (status != 0)
                 {
-                    int status = NativeMethods.BCryptCloseAlgorithmProvider(this.algorithmHandle, 0);
-                    if (status != 0)
-                    {
-                        DefaultTrace.TraceError("Failed to close algorithm provider: {0}", status);
-                    }
-
-                    this.algorithmHandle = IntPtr.Zero;
+                    DefaultTrace.TraceError("Failed to close algorithm provider: {0}", status);
                 }
+
+                this.algorithmHandle = IntPtr.Zero;
             }
         }
 

diff --git a/Microsoft.Azure.Cosmos/src/SessionContainer.cs b/Microsoft.Azure.Cosmos/src/SessionContainer.cs
@@ -472,14 +472,6 @@ public SessionContainerState(string hostName)
             {
                 this.hostName = hostName;
             }
-
-            ~SessionContainerState()
-            {
-                if (this.rwlock != null)
-                {
-                    this.rwlock.Dispose();
-                }
-            }
         }
 
         private sealed class SessionContainerSnapshot