From 74c8c8cc3383b57ecf16d0cd25951e9f4823130b Mon Sep 17 00:00:00 2001 From: Sergei Pavlov Date: Wed, 18 Oct 2023 23:06:13 -0700 Subject: [PATCH 1/6] Optimize IDisposable implementations --- .../Collections/BindingCollection.cs | 4 +--- Orm/Xtensive.Orm/Collections/Graphs/Graph.cs | 3 +-- Orm/Xtensive.Orm/Orm/Internals/Pinner.cs | 19 +++++++-------- .../Orm/Model/FullTextIndexInfoCollection.cs | 3 +-- Orm/Xtensive.Orm/Orm/OperationLog.cs | 2 +- .../Orm/Services/DirectSessionAccessor.cs | 5 +--- Orm/Xtensive.Orm/Orm/Session.Persist.cs | 13 +++++++---- Orm/Xtensive.Orm/Orm/Session.SystemLogic.cs | 23 +++++++++++++------ 8 files changed, 39 insertions(+), 33 deletions(-) diff --git a/Orm/Xtensive.Orm/Collections/BindingCollection.cs b/Orm/Xtensive.Orm/Collections/BindingCollection.cs index 0d8efef136..de594dbe0e 100644 --- a/Orm/Xtensive.Orm/Collections/BindingCollection.cs +++ b/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); } /// diff --git a/Orm/Xtensive.Orm/Collections/Graphs/Graph.cs b/Orm/Xtensive.Orm/Collections/Graphs/Graph.cs index 97799678e3..4d476f797e 100644 --- a/Orm/Xtensive.Orm/Collections/Graphs/Graph.cs +++ b/Orm/Xtensive.Orm/Collections/Graphs/Graph.cs @@ -56,9 +56,8 @@ public Graph, Edge> 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(nodeMap[edge.Source], nodeMap[edge.Target], (TEdge) edge); - processedEdges.Add(edge); } } } diff --git a/Orm/Xtensive.Orm/Orm/Internals/Pinner.cs b/Orm/Xtensive.Orm/Orm/Internals/Pinner.cs index b18c2bc902..fc73f3f9fb 100644 --- a/Orm/Xtensive.Orm/Orm/Internals/Pinner.cs +++ b/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 roots = new HashSet(); 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( - 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() { diff --git a/Orm/Xtensive.Orm/Orm/Model/FullTextIndexInfoCollection.cs b/Orm/Xtensive.Orm/Orm/Model/FullTextIndexInfoCollection.cs index 97af6944e8..c2c3894938 100644 --- a/Orm/Xtensive.Orm/Orm/Model/FullTextIndexInfoCollection.cs +++ b/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); } diff --git a/Orm/Xtensive.Orm/Orm/OperationLog.cs b/Orm/Xtensive.Orm/Orm/OperationLog.cs index eea570e86b..333a83a430 100644 --- a/Orm/Xtensive.Orm/Orm/OperationLog.cs +++ b/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) diff --git a/Orm/Xtensive.Orm/Orm/Services/DirectSessionAccessor.cs b/Orm/Xtensive.Orm/Orm/Services/DirectSessionAccessor.cs index 93fa845709..0d5e115273 100644 --- a/Orm/Xtensive.Orm/Orm/Services/DirectSessionAccessor.cs +++ b/Orm/Xtensive.Orm/Orm/Services/DirectSessionAccessor.cs @@ -28,10 +28,7 @@ public sealed class DirectSessionAccessor : SessionBound, /// disposal will restore previous state of /// property. /// - public IDisposable OpenSystemLogicOnlyRegion() - { - return Session.OpenSystemLogicOnlyRegion(); - } + public Session.SystemLogicOnlyRegionScope OpenSystemLogicOnlyRegion() => Session.OpenSystemLogicOnlyRegion(); /// /// Changes the value of . diff --git a/Orm/Xtensive.Orm/Orm/Session.Persist.cs b/Orm/Xtensive.Orm/Orm/Session.Persist.cs index e963a0dd80..07fcd69c95 100644 --- a/Orm/Xtensive.Orm/Orm/Session.Persist.cs +++ b/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; @@ -245,9 +247,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 } /// @@ -260,8 +262,9 @@ public IDisposable DisableSaveChanges(IEntity target) /// 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; diff --git a/Orm/Xtensive.Orm/Orm/Session.SystemLogic.cs b/Orm/Xtensive.Orm/Orm/Session.SystemLogic.cs index c14a77535f..5b31a1b9da 100644 --- a/Orm/Xtensive.Orm/Orm/Session.SystemLogic.cs +++ b/Orm/Xtensive.Orm/Orm/Session.SystemLogic.cs @@ -11,17 +11,26 @@ namespace Xtensive.Orm { public partial class Session { + public readonly struct SystemLogicOnlyRegionScope : IDisposable + { + private readonly Session session; + private readonly bool prevIsSystemLogicOnly; + + public SystemLogicOnlyRegionScope(Session session) + { + this.session = session; + prevIsSystemLogicOnly = session.IsSystemLogicOnly; + session.IsSystemLogicOnly = true; + } + + public void Dispose() => session.IsSystemLogicOnly = prevIsSystemLogicOnly; + } + /// /// Gets a value indicating whether only a system logic is enabled. /// internal bool IsSystemLogicOnly { get; set; } - internal IDisposable OpenSystemLogicOnlyRegion() - { - var result = new Disposable(this, IsSystemLogicOnly, - (disposing, session, previousState) => session.IsSystemLogicOnly = previousState); - IsSystemLogicOnly = true; - return result; - } + internal SystemLogicOnlyRegionScope OpenSystemLogicOnlyRegion() => new(this); } } \ No newline at end of file From 6a15d36e5b42dab165fe11608384d072c6337a8e Mon Sep 17 00:00:00 2001 From: Sergei Pavlov Date: Wed, 18 Oct 2023 23:33:12 -0700 Subject: [PATCH 2/6] optimize new List<>(1) --- Orm/Xtensive.Orm/Orm/Entity.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Orm/Xtensive.Orm/Orm/Entity.cs b/Orm/Xtensive.Orm/Orm/Entity.cs index acfb714124..b9d5103351 100644 --- a/Orm/Xtensive.Orm/Orm/Entity.cs +++ b/Orm/Xtensive.Orm/Orm/Entity.cs @@ -138,8 +138,7 @@ public VersionInfo VersionInfo { List columnsToPrefetch = null; foreach (var columnInfo in versionColumns) { if (!tuple.GetFieldState(columnInfo.Field.MappingInfo.Offset).IsAvailable()) { - if (columnsToPrefetch==null) - columnsToPrefetch = new List(); + columnsToPrefetch ??= new List(1); columnsToPrefetch.Add(new PrefetchFieldDescriptor(columnInfo.Field)); } } From 8f656af0adb0b4a94136363ea0ebd1ea344f7ca4 Mon Sep 17 00:00:00 2001 From: Sergei Pavlov Date: Wed, 18 Oct 2023 23:42:40 -0700 Subject: [PATCH 3/6] Optimize EnableSystemOperationRegistration() --- .../Orm/Operations/OperationRegistry.cs | 41 ++++++++----------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/Orm/Xtensive.Orm/Orm/Operations/OperationRegistry.cs b/Orm/Xtensive.Orm/Orm/Operations/OperationRegistry.cs index dfd867091a..61fb2b8b88 100644 --- a/Orm/Xtensive.Orm/Orm/Operations/OperationRegistry.cs +++ b/Orm/Xtensive.Orm/Orm/Operations/OperationRegistry.cs @@ -16,10 +16,24 @@ namespace Xtensive.Orm.Operations /// 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 scopes = new Collections.Deque(); /// @@ -41,10 +55,7 @@ public bool IsRegistrationEnabled { /// /// Gets a value indicating whether system operation registration is enabled. /// - public bool IsSystemOperationRegistrationEnabled { - get { return isSystemOperationRegistrationEnabled; } - internal set { isSystemOperationRegistrationEnabled = value; } - } + public bool IsSystemOperationRegistrationEnabled { get; internal set; } = true; /// /// Gets a value indicating whether this instance can register operation @@ -219,29 +230,13 @@ public IDisposable DisableUndoOperationRegistration() /// Temporarily disables system operation logging. /// /// An object enabling the logging back on its disposal. - public IDisposable DisableSystemOperationRegistration() - { - if (!isSystemOperationRegistrationEnabled) - return null; - var result = new Disposable(this, isSystemOperationRegistrationEnabled, - (disposing, _this, previousState) => _this.isSystemOperationRegistrationEnabled = previousState); - isSystemOperationRegistrationEnabled = false; - return result; - } + public SystemOperationRegistrationScope DisableSystemOperationRegistration() => new(this, false); /// /// Temporarily enables system operation logging. /// /// An object disabling the logging back on its disposal. - public IDisposable EnableSystemOperationRegistration() - { - if (isSystemOperationRegistrationEnabled) - return null; - var result = new Disposable(this, isSystemOperationRegistrationEnabled, - (disposing, _this, previousState) => _this.isSystemOperationRegistrationEnabled = previousState); - isSystemOperationRegistrationEnabled = true; - return result; - } + public SystemOperationRegistrationScope EnableSystemOperationRegistration() => new(this, true); /// /// Registers the operation. From 6f7e79d53c82f12274a5f886afd4f76b75de8b8a Mon Sep 17 00:00:00 2001 From: Sergei Pavlov Date: Thu, 19 Oct 2023 00:50:46 -0700 Subject: [PATCH 4/6] Fix Race condition in tests --- .../Storage/Prefetch/PrefetchManagerBasicTest.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Orm/Xtensive.Orm.Tests/Storage/Prefetch/PrefetchManagerBasicTest.cs b/Orm/Xtensive.Orm.Tests/Storage/Prefetch/PrefetchManagerBasicTest.cs index d03bdda37e..6e447632f6 100644 --- a/Orm/Xtensive.Orm.Tests/Storage/Prefetch/PrefetchManagerBasicTest.cs +++ b/Orm/Xtensive.Orm.Tests/Storage/Prefetch/PrefetchManagerBasicTest.cs @@ -33,10 +33,7 @@ public class PrefetchManagerBasicTest : PrefetchManagerTestBase public class MemoryLeakTester { - ~MemoryLeakTester() - { - instanceCount--; - } + ~MemoryLeakTester() => Interlocked.Decrement(ref instanceCount); } #endregion From e62b553e7179a6368446c711bb07cc4f6fc13161 Mon Sep 17 00:00:00 2001 From: Sergei Pavlov Date: Thu, 19 Oct 2023 02:18:32 -0700 Subject: [PATCH 5/6] Fix Race condition in tests --- .../Storage/Prefetch/PrefetchManagerBasicTest.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Orm/Xtensive.Orm.Tests/Storage/Prefetch/PrefetchManagerBasicTest.cs b/Orm/Xtensive.Orm.Tests/Storage/Prefetch/PrefetchManagerBasicTest.cs index 6e447632f6..72ec9ee271 100644 --- a/Orm/Xtensive.Orm.Tests/Storage/Prefetch/PrefetchManagerBasicTest.cs +++ b/Orm/Xtensive.Orm.Tests/Storage/Prefetch/PrefetchManagerBasicTest.cs @@ -27,6 +27,7 @@ namespace Xtensive.Orm.Tests.Storage.Prefetch [TestFixture] public class PrefetchManagerBasicTest : PrefetchManagerTestBase { + private const int Iterations = 10; private volatile static int instanceCount; #region Nested class @@ -938,8 +939,8 @@ public void ReferenceToSessionIsNotPreservedInCacheTest() 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()); From 3335ed94ec0c88d5c0c8b67b0349602871804d9e Mon Sep 17 00:00:00 2001 From: Sergei Pavlov Date: Thu, 19 Oct 2023 02:20:59 -0700 Subject: [PATCH 6/6] Add [MethodImpl(MethodImplOptions.NoInlining)] --- .../Storage/Prefetch/PrefetchManagerBasicTest.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Orm/Xtensive.Orm.Tests/Storage/Prefetch/PrefetchManagerBasicTest.cs b/Orm/Xtensive.Orm.Tests/Storage/Prefetch/PrefetchManagerBasicTest.cs index 72ec9ee271..ca375845f8 100644 --- a/Orm/Xtensive.Orm.Tests/Storage/Prefetch/PrefetchManagerBasicTest.cs +++ b/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; @@ -930,13 +931,14 @@ 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 = Iterations;