Skip to content

Commit aec2333

Browse files
SergeiPavlovSergeiPavlov
committed
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)]
1 parent 2be511d commit aec2333

11 files changed

Lines changed: 65 additions & 65 deletions

File tree

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System;
88
using System.Collections.Generic;
99
using System.Linq;
10+
using System.Runtime.CompilerServices;
1011
using System.Threading;
1112
using NUnit.Framework;
1213
using Xtensive.Core;
@@ -27,16 +28,14 @@ namespace Xtensive.Orm.Tests.Storage.Prefetch
2728
[TestFixture]
2829
public class PrefetchManagerBasicTest : PrefetchManagerTestBase
2930
{
31+
private const int Iterations = 10;
3032
private volatile static int instanceCount;
3133

3234
#region Nested class
3335

3436
public class MemoryLeakTester
3537
{
36-
~MemoryLeakTester()
37-
{
38-
instanceCount--;
39-
}
38+
~MemoryLeakTester() => Interlocked.Decrement(ref instanceCount);
4039
}
4140

4241
#endregion
@@ -932,17 +931,18 @@ public void RemoveTest()
932931
[Test]
933932
public void ReferenceToSessionIsNotPreservedInCacheTest()
934933
{
935-
// Use separate method for session related processing
934+
// Use separate method with [MethodImpl(MethodImplOptions.NoInlining)] attribute for session related processing
936935
// to make sure we don't hold session reference somewhere on stack
937936
OpenSessionsAndRunPrefetches();
938937
TestHelper.CollectGarbage(true);
939938
Assert.That(instanceCount, Is.EqualTo(0));
940939
}
941940

941+
[MethodImpl(MethodImplOptions.NoInlining)]
942942
private void OpenSessionsAndRunPrefetches()
943943
{
944-
instanceCount = 10;
945-
for (int i = 0; i < instanceCount; i++) {
944+
instanceCount = Iterations;
945+
for (int i = 0; i < Iterations; i++) {
946946
using (var session = Domain.OpenSession())
947947
using (var t = session.OpenTransaction()) {
948948
session.Extensions.Set(new MemoryLeakTester());

‎Orm/Xtensive.Orm/Collections/BindingCollection.cs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,7 @@ public virtual BindingScope Add(TKey key, TValue value)
112112
public virtual void PermanentAdd(TKey key, TValue value)
113113
{
114114
bindings[key] = value;
115-
if (!permanentBindings.Contains(key)) {
116-
permanentBindings.Add(key);
117-
}
115+
permanentBindings.Add(key);
118116
}
119117

120118
/// <summary>

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,8 @@ public Graph<Node<TNode>, Edge<TEdge>> CreateMutableCopy()
5656
foreach (var rNode in copy.Nodes) {
5757
var node = rNode.Value;
5858
foreach (var edge in node.Edges) {
59-
if (!processedEdges.Contains(edge)) {
59+
if (processedEdges.Add(edge)) {
6060
var rEdge = new Edge<TEdge>(nodeMap[edge.Source], nodeMap[edge.Target], (TEdge) edge);
61-
processedEdges.Add(edge);
6261
}
6362
}
6463
}

‎Orm/Xtensive.Orm/Orm/Entity.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,7 @@ public VersionInfo VersionInfo {
138138
List<PrefetchFieldDescriptor> columnsToPrefetch = null;
139139
foreach (var columnInfo in versionColumns) {
140140
if (!tuple.GetFieldState(columnInfo.Field.MappingInfo.Offset).IsAvailable()) {
141-
if (columnsToPrefetch==null)
142-
columnsToPrefetch = new List<PrefetchFieldDescriptor>();
141+
columnsToPrefetch ??= new List<PrefetchFieldDescriptor>(1);
143142
columnsToPrefetch.Add(new PrefetchFieldDescriptor(columnInfo.Field));
144143
}
145144
}

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

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,14 @@ namespace Xtensive.Orm.Internals
1414
{
1515
internal sealed class Pinner : SessionBound
1616
{
17+
private class DisposableRemover : IDisposable
18+
{
19+
public Pinner Pinner { get; init; }
20+
public EntityState State { get; init; }
21+
22+
public void Dispose() => Pinner.roots.Remove(State);
23+
}
24+
1725
private readonly HashSet<EntityState> roots = new HashSet<EntityState>();
1826

1927
private EntityChangeRegistry activeRegistry;
@@ -24,15 +32,8 @@ internal sealed class Pinner : SessionBound
2432
public EntityChangeRegistry PinnedItems { get; private set; }
2533
public EntityChangeRegistry PersistableItems { get; private set; }
2634

27-
public IDisposable RegisterRoot(EntityState state)
28-
{
29-
if (roots.Contains(state))
30-
return null;
31-
roots.Add(state);
32-
return new Disposable<Pinner, EntityState>(
33-
this, state,
34-
(disposing, _this, _state) => _this.roots.Remove(_state));
35-
}
35+
public IDisposable RegisterRoot(EntityState state) =>
36+
roots.Add(state) ? new DisposableRemover { Pinner = this, State = state } : null;
3637

3738
public void ClearRoots()
3839
{

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,7 @@ public bool TryGetValue(TypeInfo typeInfo, out FullTextIndexInfo fullTextIndexIn
5454
public void Add(TypeInfo typeInfo, FullTextIndexInfo fullTextIndexInfo)
5555
{
5656
EnsureNotLocked();
57-
if (!container.Contains(fullTextIndexInfo))
58-
container.Add(fullTextIndexInfo);
57+
container.Add(fullTextIndexInfo);
5958
indexMap.Add(typeInfo, fullTextIndexInfo);
6059
}
6160

‎Orm/Xtensive.Orm/Orm/OperationLog.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public KeyMapping Replay(Session session)
6161
KeyMapping keyMapping;
6262

6363
using (session.Activate()) {
64-
using (isSystemOperationLog ? session.OpenSystemLogicOnlyRegion() : null)
64+
using (isSystemOperationLog ? (IDisposable) session.OpenSystemLogicOnlyRegion() : null)
6565
using (var tx = session.OpenTransaction(TransactionOpenMode.New)) {
6666

6767
foreach (var operation in operations)

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

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,24 @@ namespace Xtensive.Orm.Operations
1616
/// </summary>
1717
public sealed class OperationRegistry
1818
{
19+
public readonly struct SystemOperationRegistrationScope : IDisposable
20+
{
21+
private readonly OperationRegistry registry;
22+
private readonly bool prevIsSystemOperationRegistrationEnabled;
23+
24+
public SystemOperationRegistrationScope(OperationRegistry registry, bool enable)
25+
{
26+
this.registry = registry;
27+
prevIsSystemOperationRegistrationEnabled = registry.IsSystemOperationRegistrationEnabled;
28+
registry.IsSystemOperationRegistrationEnabled = enable;
29+
}
30+
31+
public void Dispose() => registry.IsSystemOperationRegistrationEnabled = prevIsSystemOperationRegistrationEnabled;
32+
}
33+
1934
private readonly ICompletableScope blockingScope;
2035
private bool isOperationRegistrationEnabled = true;
2136
private bool isUndoOperationRegistrationEnabled = true;
22-
private bool isSystemOperationRegistrationEnabled = true;
2337
private Collections.Deque<ICompletableScope> scopes = new Collections.Deque<ICompletableScope>();
2438

2539
/// <summary>
@@ -41,10 +55,7 @@ public bool IsRegistrationEnabled {
4155
/// <summary>
4256
/// Gets a value indicating whether system operation registration is enabled.
4357
/// </summary>
44-
public bool IsSystemOperationRegistrationEnabled {
45-
get { return isSystemOperationRegistrationEnabled; }
46-
internal set { isSystemOperationRegistrationEnabled = value; }
47-
}
58+
public bool IsSystemOperationRegistrationEnabled { get; internal set; } = true;
4859

4960
/// <summary>
5061
/// Gets a value indicating whether this instance can register operation
@@ -219,29 +230,13 @@ public IDisposable DisableUndoOperationRegistration()
219230
/// Temporarily disables system operation logging.
220231
/// </summary>
221232
/// <returns>An <see cref="IDisposable"/> object enabling the logging back on its disposal.</returns>
222-
public IDisposable DisableSystemOperationRegistration()
223-
{
224-
if (!isSystemOperationRegistrationEnabled)
225-
return null;
226-
var result = new Disposable<OperationRegistry, bool>(this, isSystemOperationRegistrationEnabled,
227-
(disposing, _this, previousState) => _this.isSystemOperationRegistrationEnabled = previousState);
228-
isSystemOperationRegistrationEnabled = false;
229-
return result;
230-
}
233+
public SystemOperationRegistrationScope DisableSystemOperationRegistration() => new(this, false);
231234

232235
/// <summary>
233236
/// Temporarily enables system operation logging.
234237
/// </summary>
235238
/// <returns>An <see cref="IDisposable"/> object disabling the logging back on its disposal.</returns>
236-
public IDisposable EnableSystemOperationRegistration()
237-
{
238-
if (isSystemOperationRegistrationEnabled)
239-
return null;
240-
var result = new Disposable<OperationRegistry, bool>(this, isSystemOperationRegistrationEnabled,
241-
(disposing, _this, previousState) => _this.isSystemOperationRegistrationEnabled = previousState);
242-
isSystemOperationRegistrationEnabled = true;
243-
return result;
244-
}
239+
public SystemOperationRegistrationScope EnableSystemOperationRegistration() => new(this, true);
245240

246241
/// <summary>
247242
/// Registers the operation.

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,7 @@ public sealed class DirectSessionAccessor : SessionBound,
2828
/// disposal will restore previous state of
2929
/// <see cref="Session.IsSystemLogicOnly"/> property.
3030
/// </returns>
31-
public IDisposable OpenSystemLogicOnlyRegion()
32-
{
33-
return Session.OpenSystemLogicOnlyRegion();
34-
}
31+
public Session.SystemLogicOnlyRegionScope OpenSystemLogicOnlyRegion() => Session.OpenSystemLogicOnlyRegion();
3532

3633
/// <summary>
3734
/// Changes the value of <see cref="Session.Handler"/>.

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ namespace Xtensive.Orm
1818
{
1919
public partial class Session
2020
{
21+
private static readonly IDisposable EmptyDisposable = new Disposable(b => { return; });
22+
2123
private bool disableAutoSaveChanges;
2224
private KeyRemapper remapper;
2325
private bool persistingIsFailed;
@@ -242,9 +244,9 @@ public IDisposable DisableSaveChanges(IEntity target)
242244
ArgumentValidator.EnsureArgumentNotNull(target, "target");
243245
var targetEntity = (Entity) target;
244246
targetEntity.EnsureNotRemoved();
245-
if (!Configuration.Supports(SessionOptions.AutoSaveChanges))
246-
return new Disposable(b => {return;}); // No need to pin in this case
247-
return pinner.RegisterRoot(targetEntity.State);
247+
return Configuration.Supports(SessionOptions.AutoSaveChanges)
248+
? pinner.RegisterRoot(targetEntity.State)
249+
: EmptyDisposable; // No need to pin in this case
248250
}
249251

250252
/// <summary>
@@ -257,8 +259,9 @@ public IDisposable DisableSaveChanges(IEntity target)
257259
/// </summary>
258260
public IDisposable DisableSaveChanges()
259261
{
260-
if (!Configuration.Supports(SessionOptions.AutoSaveChanges))
261-
return new Disposable(b => { return; }); // No need to pin in this case
262+
if (!Configuration.Supports(SessionOptions.AutoSaveChanges)) {
263+
return EmptyDisposable; // No need to pin in this case
264+
}
262265
if (disableAutoSaveChanges)
263266
return null;
264267

0 commit comments

Comments
 (0)