Skip to content

Commit 04fac44

Browse files
author
Oren (electricessence)
committed
Implemented Open.Disposable.ObjectPools for ReadWriteHelper
1 parent cb07c25 commit 04fac44

2 files changed

Lines changed: 37 additions & 51 deletions

File tree

Open.Threading.ReadWrite.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ Part of the "Open" set of libraries.</Description>
3232
<ItemGroup>
3333
<PackageReference Include="Open.Diagnostics" Version="1.3.0" />
3434
<PackageReference Include="Open.Disposable" Version="1.1.0" />
35+
<PackageReference Include="Open.Disposable.ObjectPools" Version="2.1.0" />
3536
</ItemGroup>
3637

3738
</Project>

ReadWriteHelper.cs

Lines changed: 36 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,8 @@ protected override void OnDispose(bool calledExplicitly)
8585
}
8686

8787

88-
readonly ConcurrentQueue<object> ContextPool = new ConcurrentQueue<object>();
89-
readonly ConcurrentQueue<ReaderWriterLockTracker> LockPool = new ConcurrentQueue<ReaderWriterLockTracker>();
88+
readonly OptimisticArrayObjectPool<object> ContextPool;
89+
readonly ConcurrentQueueObjectPool<ReaderWriterLockTracker> LockPool;
9090

9191
readonly ConcurrentDictionary<TKey, ReaderWriterLockTracker> Locks
9292
= new ConcurrentDictionary<TKey, ReaderWriterLockTracker>();
@@ -96,7 +96,6 @@ readonly ReaderWriterLockSlim CleanupManager
9696

9797
public ReadWriteHelper() : this(false)
9898
{
99-
10099
}
101100

102101
public ReadWriteHelper(bool supportRecursion)
@@ -106,6 +105,15 @@ public ReadWriteHelper(bool supportRecursion)
106105
RecursionPolicy = supportRecursion
107106
? LockRecursionPolicy.SupportsRecursion
108107
: LockRecursionPolicy.NoRecursion;
108+
109+
ContextPool = OptimisticArrayObjectPool.Create<object>();
110+
LockPool = ConcurrentQueueObjectPool.CreateAutoDisposal(() =>
111+
{
112+
var created = new ReaderWriterLockTracker(RecursionPolicy);
113+
if (Debugger.IsAttached)
114+
created.BeforeDispose += new EventHandler(Debug_TrackerDisposedWhileInUse);
115+
return created;
116+
}, 1000);
109117
}
110118

111119
public LockRecursionPolicy RecursionPolicy
@@ -140,16 +148,7 @@ private ReaderWriterLockTracker GetLock(TKey key, LockType type, object context,
140148
ReaderWriterLockTracker created = null;
141149
do
142150
{
143-
result = Locks.GetOrAdd(key, k =>
144-
{
145-
if (!LockPool.TryDequeue(out created))
146-
{
147-
created = new ReaderWriterLockTracker(RecursionPolicy);
148-
if (Debugger.IsAttached)
149-
created.BeforeDispose += new EventHandler(Debug_TrackerDisposedWhileInUse);
150-
}
151-
return created;
152-
});
151+
result = Locks.GetOrAdd(key, k => created = LockPool.Take());
153152
}
154153
// Safeguard against rare case of when a disposed tracker is retained via an exception (possibly?). :(
155154
while (!IsDisposed && result.IsDisposed);
@@ -161,7 +160,7 @@ private ReaderWriterLockTracker GetLock(TKey key, LockType type, object context,
161160
if (IsDisposed)
162161
created.Dispose();
163162
else
164-
LockPool.Enqueue(created);
163+
LockPool.Give(created);
165164
}
166165

167166
// This should never get out of sync, but just in case...
@@ -227,8 +226,8 @@ void Debug_TrackerDisposedWhileInUse(object sender, EventArgs e)
227226
var tracker = (ReaderWriterLockTracker)sender;
228227
if (Locks.Select(kvp => kvp.Value).Contains(tracker))
229228
Debug.Fail("Attempting to dispose a tracker that is in use.");
230-
if (LockPool.Contains(tracker))
231-
Debug.Fail("Attempting to dispose a tracker that is still availalbe in the pool.");
229+
//if (LockPool.Contains(tracker))
230+
// Debug.Fail("Attempting to dispose a tracker that is still availalbe in the pool.");
232231
}
233232

234233
private bool AcquireLock(ReaderWriterLockSlim target, LockType type, int? millisecondsTimeout = null, bool throwsOnTimeout = false)
@@ -290,8 +289,7 @@ private bool Execute(TKey key, LockType type, Action<ReaderWriterLockSlim> closu
290289
ReaderWriterLockSlimExensions.ValidateMillisecondsTimeout(millisecondsTimeout);
291290
Contract.EndContractBlock();
292291

293-
if (!ContextPool.TryDequeue(out object context) || context == null)
294-
context = new Object();
292+
var context = ContextPool.Take();
295293

296294
ReaderWriterLockTracker rwlock = GetLock(key, type, context, millisecondsTimeout, throwsOnTimeout);
297295
if (rwlock == null)
@@ -307,7 +305,7 @@ private bool Execute(TKey key, LockType type, Action<ReaderWriterLockSlim> closu
307305
{
308306
ReleaseLock(rwlock.Lock, type);
309307
rwlock.Clear(context);
310-
ContextPool.Enqueue(context);
308+
ContextPool.Give(context);
311309
}
312310
catch (Exception ex)
313311
{
@@ -680,10 +678,11 @@ private void CleanupInternal()
680678
{
681679
try
682680
{
683-
LockPool.Enqueue(tempLock);
681+
LockPool.Give(tempLock);
684682
}
685683
catch (ObjectDisposedException)
686684
{
685+
// Rare case where lock pool get's disposed inbetween iterations.
687686
}
688687
}
689688
}
@@ -759,11 +758,10 @@ protected override void OnCleanup()
759758
var count = Locks.Count;
760759
var maxCount = Math.Min(count, 100);
761760

762-
TrimPool(ContextPool, maxCount * 2);
763-
TrimPool(LockPool, maxCount, true);//ConcurrentQueueTrimAndDispose(LockPool, maxCount);
761+
LockPool.TrimTo(maxCount);
764762

765-
if (Debugger.IsAttached && LockPool.Any(l => l.IsDisposed))
766-
Debug.Fail("LockPool is retaining a disposed tracker.");
763+
//if (Debugger.IsAttached && LockPool.Any(l => l.IsDisposed))
764+
// Debug.Fail("LockPool is retaining a disposed tracker.");
767765

768766
if (count == 0)
769767
ClearCleanup();
@@ -785,40 +783,27 @@ protected override void OnDispose(bool calledExplicitly)
785783
{
786784
base.OnDispose(calledExplicitly);
787785

788-
bool lockHeld = CleanupManager.Write(() =>
786+
if (calledExplicitly)
789787
{
790788

791-
CleanupInternal();
792-
Locks.Clear(); // Some locks may be removed, but releasing will still occur.
793-
TrimPool(LockPool, 0, true, calledExplicitly);
794-
795-
}, calledExplicitly ? 1000 : 0); // We don't want to block for any reason for too long.
796-
// Dispose shouldn't be called without this being able to be cleaned.
797-
798-
CleanupManager.Dispose(); // No more locks can be added after this...
789+
bool lockHeld = CleanupManager.Write(() =>
790+
{
799791

800-
if (!lockHeld)
801-
{
802-
// Just to be sure...
803-
Debug.WriteLineIf(calledExplicitly, "ReadWriteHelper unable to synchronize during dispose.");
804-
CleanupInternal(); // Migrates to LockPool for cleanup or disposal...
805-
}
792+
CleanupInternal();
793+
Locks.Clear(); // Some locks may be removed, but releasing will still occur.
794+
LockPool.Dispose();
806795

807-
Locks.Clear();
796+
}, 1000); // We don't want to block for any reason for too long.
797+
// Dispose shouldn't be called without this being able to be cleaned.
808798

809-
TrimPool(ContextPool, 0, false, calledExplicitly);
810-
if (!lockHeld)
811-
TrimPool(LockPool, 0, true, calledExplicitly);
799+
CleanupManager.Dispose(); // No more locks can be added after this...
812800

813-
if (calledExplicitly && Debugger.IsAttached)
814-
{
815-
if (Locks.Any())
816-
Debug.Fail("A lock was added after dispose.");
817-
if (LockPool.Any())
818-
Debug.Fail("Remaining trackers in lock pool.");
819-
if (ContextPool.Any())
820-
Debug.Fail("Remaining objects in context pool.");
801+
if (!lockHeld) CleanupInternal(); // Migrates to LockPool for cleanup or disposal...
802+
Locks.Clear();
803+
ContextPool.Dispose();
804+
if (!lockHeld) LockPool.Dispose();
821805
}
806+
822807
}
823808
#endregion
824809

0 commit comments

Comments
 (0)