Optimize IDisposable implementations (#153)

* Optimize IDisposable implementations

* optimize new List<>(1)

* Optimize EnableSystemOperationRegistration()

* Fix Race condition in tests

* Fix Race condition in tests

* Add [MethodImpl(MethodImplOptions.NoInlining)]

Author: SergeiPavlov

Orm/Xtensive.Orm.Tests/Storage/Prefetch/PrefetchManagerBasicTest.cs

@@ -7,6 +7,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Runtime.CompilerServices;
 using System.Threading;
 using NUnit.Framework;
 using Xtensive.Core;
@@ -27,16 +28,14 @@ namespace Xtensive.Orm.Tests.Storage.Prefetch
   [TestFixture]
   public class PrefetchManagerBasicTest : PrefetchManagerTestBase
   {
+    private const int Iterations = 10;
     private volatile static int instanceCount;
 
     #region Nested class
 
     public class MemoryLeakTester
     {
-      ~MemoryLeakTester()
-      {
-        instanceCount--;
-      }
+      ~MemoryLeakTester() => Interlocked.Decrement(ref instanceCount);
     }
 
     #endregion
@@ -932,17 +931,18 @@ public void RemoveTest()
     [Test]
     public void ReferenceToSessionIsNotPreservedInCacheTest()
     {
-      // Use separate method for session related processing
+      // Use separate method with [MethodImpl(MethodImplOptions.NoInlining)] attribute for session related processing
       // to make sure we don't hold session reference somewhere on stack
       OpenSessionsAndRunPrefetches();
       TestHelper.CollectGarbage(true);
       Assert.That(instanceCount, Is.EqualTo(0));
     }
 
+    [MethodImpl(MethodImplOptions.NoInlining)]
     private void OpenSessionsAndRunPrefetches()
     {
-      instanceCount = 10;
-      for (int i = 0; i < instanceCount; i++) {
+      instanceCount = Iterations;
+      for (int i = 0; i < Iterations; i++) {
         using (var session = Domain.OpenSession())
         using (var t = session.OpenTransaction()) {
           session.Extensions.Set(new MemoryLeakTester());

Orm/Xtensive.Orm/Collections/BindingCollection.cs

@@ -112,9 +112,7 @@ public virtual BindingScope Add(TKey key, TValue value)
     public virtual void PermanentAdd(TKey key, TValue value)
     {
       bindings[key] = value;
-      if (!permanentBindings.Contains(key)) {
-        permanentBindings.Add(key);
-      }
+      permanentBindings.Add(key);
     }
 
     /// <summary>

Orm/Xtensive.Orm/Collections/Graphs/Graph.cs

@@ -56,9 +56,8 @@ public Graph<Node<TNode>, Edge<TEdge>> CreateMutableCopy()
       foreach (var rNode in copy.Nodes) {
         var node = rNode.Value;
         foreach (var edge in node.Edges) {
-          if (!processedEdges.Contains(edge)) {
+          if (processedEdges.Add(edge)) {
             var rEdge = new Edge<TEdge>(nodeMap[edge.Source], nodeMap[edge.Target], (TEdge) edge);
-            processedEdges.Add(edge);
           }
         }
       }

Orm/Xtensive.Orm/Orm/Entity.cs

@@ -138,8 +138,7 @@ public VersionInfo VersionInfo {
         List<PrefetchFieldDescriptor> columnsToPrefetch = null;
         foreach (var columnInfo in versionColumns) {
           if (!tuple.GetFieldState(columnInfo.Field.MappingInfo.Offset).IsAvailable()) {
-            if (columnsToPrefetch==null)
-              columnsToPrefetch = new List<PrefetchFieldDescriptor>();
+            columnsToPrefetch ??= new List<PrefetchFieldDescriptor>(1);
             columnsToPrefetch.Add(new PrefetchFieldDescriptor(columnInfo.Field));
           }
         }

Orm/Xtensive.Orm/Orm/Internals/Pinner.cs

@@ -14,6 +14,14 @@ namespace Xtensive.Orm.Internals
 {
   internal sealed class Pinner : SessionBound
   {
+    private class DisposableRemover : IDisposable
+    {
+      public Pinner Pinner { get; init; }
+      public EntityState State { get; init; }
+
+      public void Dispose() => Pinner.roots.Remove(State);
+    }
+
     private readonly HashSet<EntityState> roots = new HashSet<EntityState>();
 
     private EntityChangeRegistry activeRegistry;
@@ -24,15 +32,8 @@ internal sealed class Pinner : SessionBound
     public EntityChangeRegistry PinnedItems { get; private set; }
     public EntityChangeRegistry PersistableItems { get; private set; }
 
-    public IDisposable RegisterRoot(EntityState state)
-    {
-      if (roots.Contains(state))
-        return null;
-      roots.Add(state);
-      return new Disposable<Pinner, EntityState>(
-        this, state,
-        (disposing, _this, _state) => _this.roots.Remove(_state));
-    }
+    public IDisposable RegisterRoot(EntityState state) =>
+      roots.Add(state) ? new DisposableRemover { Pinner = this, State = state } : null;
 
     public void ClearRoots()
     {

Orm/Xtensive.Orm/Orm/Model/FullTextIndexInfoCollection.cs

@@ -54,8 +54,7 @@ public bool TryGetValue(TypeInfo typeInfo, out FullTextIndexInfo fullTextIndexIn
     public void Add(TypeInfo typeInfo, FullTextIndexInfo fullTextIndexInfo)
     {
       EnsureNotLocked();
-      if (!container.Contains(fullTextIndexInfo))
-        container.Add(fullTextIndexInfo);
+      container.Add(fullTextIndexInfo);
       indexMap.Add(typeInfo, fullTextIndexInfo);
     }
 

Orm/Xtensive.Orm/Orm/OperationLog.cs

@@ -61,7 +61,7 @@ public KeyMapping Replay(Session session)
       KeyMapping keyMapping;
 
       using (session.Activate()) {
-        using (isSystemOperationLog ? session.OpenSystemLogicOnlyRegion() : null) 
+        using (isSystemOperationLog ? (IDisposable) session.OpenSystemLogicOnlyRegion() : null)
         using (var tx = session.OpenTransaction(TransactionOpenMode.New)) {
 
           foreach (var operation in operations)

Orm/Xtensive.Orm/Orm/Operations/OperationRegistry.cs

@@ -16,10 +16,24 @@ namespace Xtensive.Orm.Operations
   /// </summary>
   public sealed class OperationRegistry
   {
+    public readonly struct SystemOperationRegistrationScope : IDisposable
+    {
+      private readonly OperationRegistry registry;
+      private readonly bool prevIsSystemOperationRegistrationEnabled;
+
+      public SystemOperationRegistrationScope(OperationRegistry registry, bool enable)
+      {
+        this.registry = registry;
+        prevIsSystemOperationRegistrationEnabled = registry.IsSystemOperationRegistrationEnabled;
+        registry.IsSystemOperationRegistrationEnabled = enable;
+      }
+
+      public void Dispose() => registry.IsSystemOperationRegistrationEnabled = prevIsSystemOperationRegistrationEnabled;
+    }
+
     private readonly ICompletableScope blockingScope;
     private bool isOperationRegistrationEnabled = true;
     private bool isUndoOperationRegistrationEnabled = true;
-    private bool isSystemOperationRegistrationEnabled = true;
     private Collections.Deque<ICompletableScope> scopes = new Collections.Deque<ICompletableScope>();
 
     /// <summary>
@@ -41,10 +55,7 @@ public bool IsRegistrationEnabled {
     /// <summary>
     /// Gets a value indicating whether system operation registration is enabled.
     /// </summary>
-    public bool IsSystemOperationRegistrationEnabled {
-      get { return isSystemOperationRegistrationEnabled; }
-      internal set { isSystemOperationRegistrationEnabled = value; }
-    }
+    public bool IsSystemOperationRegistrationEnabled { get; internal set; } = true;
 
     /// <summary>
     /// Gets a value indicating whether this instance can register operation
@@ -219,29 +230,13 @@ public IDisposable DisableUndoOperationRegistration()
     /// Temporarily disables system operation logging.
     /// </summary>
     /// <returns>An <see cref="IDisposable"/> object enabling the logging back on its disposal.</returns>
-    public IDisposable DisableSystemOperationRegistration()
-    {
-      if (!isSystemOperationRegistrationEnabled)
-        return null;
-      var result = new Disposable<OperationRegistry, bool>(this, isSystemOperationRegistrationEnabled,
-        (disposing, _this, previousState) => _this.isSystemOperationRegistrationEnabled = previousState);
-      isSystemOperationRegistrationEnabled = false;
-      return result;
-    }
+    public SystemOperationRegistrationScope DisableSystemOperationRegistration() => new(this, false);
 
     /// <summary>
     /// Temporarily enables system operation logging.
     /// </summary>
     /// <returns>An <see cref="IDisposable"/> object disabling the logging back on its disposal.</returns>
-    public IDisposable EnableSystemOperationRegistration()
-    {
-      if (isSystemOperationRegistrationEnabled)
-        return null;
-      var result = new Disposable<OperationRegistry, bool>(this, isSystemOperationRegistrationEnabled,
-        (disposing, _this, previousState) => _this.isSystemOperationRegistrationEnabled = previousState);
-      isSystemOperationRegistrationEnabled = true;
-      return result;
-    }
+    public SystemOperationRegistrationScope EnableSystemOperationRegistration() => new(this, true);
 
     /// <summary>
     /// Registers the operation.

Orm/Xtensive.Orm/Orm/Services/DirectSessionAccessor.cs

@@ -28,10 +28,7 @@ public sealed class DirectSessionAccessor : SessionBound,
     /// disposal will restore previous state of
     /// <see cref="Session.IsSystemLogicOnly"/> property.
     /// </returns>
-    public IDisposable OpenSystemLogicOnlyRegion()
-    {
-      return Session.OpenSystemLogicOnlyRegion();
-    }
+    public Session.SystemLogicOnlyRegionScope OpenSystemLogicOnlyRegion() => Session.OpenSystemLogicOnlyRegion();
 
     /// <summary>
     /// Changes the value of <see cref="Session.Handler"/>.

Orm/Xtensive.Orm/Orm/Session.Persist.cs

@@ -18,6 +18,8 @@ namespace Xtensive.Orm
 {
   public partial class Session
   {
+    private static readonly IDisposable EmptyDisposable = new Disposable(b => { return; });
+
     private bool disableAutoSaveChanges;
     private KeyRemapper remapper;
     private bool persistingIsFailed;
@@ -242,9 +244,9 @@ public IDisposable DisableSaveChanges(IEntity target)
       ArgumentValidator.EnsureArgumentNotNull(target, "target");
       var targetEntity = (Entity) target;
       targetEntity.EnsureNotRemoved();
-      if (!Configuration.Supports(SessionOptions.AutoSaveChanges))
-        return new Disposable(b => {return;}); // No need to pin in this case
-      return pinner.RegisterRoot(targetEntity.State);
+      return Configuration.Supports(SessionOptions.AutoSaveChanges)
+        ? pinner.RegisterRoot(targetEntity.State)
+        : EmptyDisposable; // No need to pin in this case
     }
 
     /// <summary>
@@ -257,8 +259,9 @@ public IDisposable DisableSaveChanges(IEntity target)
     /// </summary>
     public IDisposable DisableSaveChanges()
     {
-      if (!Configuration.Supports(SessionOptions.AutoSaveChanges))
-        return new Disposable(b => { return; }); // No need to pin in this case
+      if (!Configuration.Supports(SessionOptions.AutoSaveChanges)) {
+        return EmptyDisposable; // No need to pin in this case
+      }
       if (disableAutoSaveChanges)
         return null;