Skip to content

Commit 2f120e0

Browse files
bitfasterbitfaster
authored
Align ICache.TryRemove overloads with ConcurrentDictionary (#394)
* overloads * ICache * defaults * atomic tests * atomic tests * test not removed * lfu+classic * cleanup logic * 6only * kvp tests * api docs * comment * verify value * test naming ---------
1 parent a112f23 commit 2f120e0

File tree

12 files changed

+467
-62
lines changed

12 files changed

+467
-62
lines changed

‎BitFaster.Caching.UnitTests/Atomic/AtomicFactoryCacheTests.cs‎

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,18 @@ namespace BitFaster.Caching.UnitTests.Atomic
1313
public class AtomicFactoryCacheTests
1414
{
1515
private const int capacity = 6;
16-
private readonly AtomicFactoryCache<int, int> cache = new(new ConcurrentLru<int, AtomicFactory<int, int>>(capacity));
16+
private readonly ConcurrentLru<int, AtomicFactory<int, int>> innerCache;
17+
private readonly AtomicFactoryCache<int, int> cache;
1718

1819
private List<ItemRemovedEventArgs<int, int>> removedItems = new();
1920
private List<ItemUpdatedEventArgs<int, int>> updatedItems = new();
2021

22+
public AtomicFactoryCacheTests()
23+
{
24+
innerCache = new ConcurrentLru<int, AtomicFactory<int, int>>(capacity);
25+
cache = new(innerCache);
26+
}
27+
2128
[Fact]
2229
public void WhenInnerCacheIsNullCtorThrows()
2330
{
@@ -70,10 +77,52 @@ public void WhenRemovedEventHandlerIsRegisteredItIsFired()
7077
this.cache.TryRemove(1);
7178

7279
this.removedItems.First().Key.Should().Be(1);
80+
}
81+
82+
// backcompat: remove conditional compile
83+
#if NETCOREAPP3_0_OR_GREATER
84+
[Fact]
85+
public void WhenRemovedValueIsReturned()
86+
{
87+
this.cache.AddOrUpdate(1, 1);
88+
this.cache.TryRemove(1, out var value);
89+
90+
value.Should().Be(1);
91+
}
92+
93+
[Fact]
94+
public void WhenNotRemovedValueIsDefault()
95+
{
96+
this.cache.AddOrUpdate(1, 1);
97+
this.cache.TryRemove(2, out var value);
98+
99+
value.Should().Be(0);
100+
}
101+
102+
[Fact]
103+
public void WhenRemoveKeyValueAndValueDoesntMatchDontRemove()
104+
{
105+
this.cache.AddOrUpdate(1, 1);
106+
this.cache.TryRemove(new KeyValuePair<int, int>(1, 2)).Should().BeFalse();
107+
}
108+
109+
[Fact]
110+
public void WhenRemoveKeyValueAndValueDoesMatchThenRemove()
111+
{
112+
this.cache.AddOrUpdate(1, 1);
113+
this.cache.TryRemove(new KeyValuePair<int, int>(1, 1)).Should().BeTrue();
114+
}
115+
116+
[Fact]
117+
public void WhenRemoveKeyValueAndValueIsNotCreatedDoesNotRemove()
118+
{
119+
// seed the inner cache with an not yet created value
120+
this.innerCache.AddOrUpdate(1, new AtomicFactory<int, int>());
121+
122+
// try to remove with the default value (0)
123+
this.cache.TryRemove(new KeyValuePair<int, int>(1, 0)).Should().BeFalse();
73124
}
74125

75-
// backcompat: remove conditional compile
76-
#if NETCOREAPP3_0_OR_GREATER
77126
[Fact]
78127
public void WhenUpdatedEventHandlerIsRegisteredItIsFired()
79128
{

‎BitFaster.Caching.UnitTests/Atomic/AtomicFactoryTests.cs‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,58 @@ public void WhenValueCreatedArgGetValueReturnsOriginalValue()
6363
a.GetValue(1, (k, a) => k + a, 9).Should().Be(8);
6464
}
6565

66+
[Fact]
67+
public void WhenValueNotCreatedHashCodeIsZero()
68+
{
69+
new AtomicFactory<int, int>()
70+
.GetHashCode()
71+
.Should().Be(0);
72+
}
73+
74+
[Fact]
75+
public void WhenValueCreatedHashCodeIsValueHashCode()
76+
{
77+
new AtomicFactory<int, int>(1)
78+
.GetHashCode()
79+
.Should().Be(1);
80+
}
81+
82+
[Fact]
83+
public void WhenValueNotCreatedEqualsFalse()
84+
{
85+
var a = new AtomicFactory<int, int>();
86+
var b = new AtomicFactory<int, int>();
87+
88+
a.Equals(b).Should().BeFalse();
89+
}
90+
91+
[Fact]
92+
public void WhenOtherValueNotCreatedEqualsFalse()
93+
{
94+
var a = new AtomicFactory<int, int>(1);
95+
var b = new AtomicFactory<int, int>();
96+
97+
a.Equals(b).Should().BeFalse();
98+
}
99+
100+
[Fact]
101+
public void WhenArgNullEqualsFalse()
102+
{
103+
new AtomicFactory<int, int>(1)
104+
.Equals(null)
105+
.Should().BeFalse();
106+
}
107+
108+
[Fact]
109+
public void WhenArgObjectValuesAreSameEqualsTrue()
110+
{
111+
object other = new AtomicFactory<int, int>(1);
112+
113+
new AtomicFactory<int, int>(1)
114+
.Equals(other)
115+
.Should().BeTrue();
116+
}
117+
66118
[Fact]
67119
public async Task WhenCallersRunConcurrentlyResultIsFromWinner()
68120
{

‎BitFaster.Caching.UnitTests/CacheTests.cs‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11

22
using System;
3+
using System.Collections.Generic;
34
using System.Threading.Tasks;
45
using FluentAssertions;
56
using Moq;
@@ -25,6 +26,28 @@ public void WhenCacheInterfaceDefaultGetOrAddFallback()
2526
1,
2627
(k, a) => k + a,
2728
2).Should().Be(3);
29+
}
30+
31+
[Fact]
32+
public void WhenCacheInterfaceDefaultTryRemoveKeyThrows()
33+
{
34+
var cache = new Mock<ICache<int, int>>();
35+
cache.CallBase = true;
36+
37+
Action tryRemove = () => { cache.Object.TryRemove(1, out var value); };
38+
39+
tryRemove.Should().Throw<NotSupportedException>();
40+
}
41+
42+
[Fact]
43+
public void WhenCacheInterfaceDefaultTryRemoveKeyValueThrows()
44+
{
45+
var cache = new Mock<ICache<int, int>>();
46+
cache.CallBase = true;
47+
48+
Action tryRemove = () => { cache.Object.TryRemove(new KeyValuePair<int, int>(1, 1)); };
49+
50+
tryRemove.Should().Throw<NotSupportedException>();
2851
}
2952

3053
[Fact]

‎BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -582,12 +582,39 @@ public void WhenItemDoesNotExistUpdatedAddsItem()
582582
}
583583

584584
[Fact]
585-
public void WhenItemIsRemovedItIsRemoved()
585+
public void WhenKeyExistsTryRemoveRemovesItem()
586586
{
587587
cache.GetOrAdd(1, k => k);
588588

589589
cache.TryRemove(1).Should().BeTrue();
590-
cache.TryGet(1, out var value).Should().BeFalse();
590+
cache.TryGet(1, out _).Should().BeFalse();
591+
}
592+
593+
[Fact]
594+
public void WhenKeyExistsTryRemoveReturnsValue()
595+
{
596+
cache.GetOrAdd(1, valueFactory.Create);
597+
598+
cache.TryRemove(1, out var value).Should().BeTrue();
599+
value.Should().Be(1);
600+
}
601+
602+
[Fact]
603+
public void WhenItemExistsTryRemoveRemovesItem()
604+
{
605+
cache.GetOrAdd(1, k => k);
606+
607+
cache.TryRemove(new KeyValuePair<int, int>(1, 1)).Should().BeTrue();
608+
cache.TryGet(1, out _).Should().BeFalse();
609+
}
610+
611+
[Fact]
612+
public void WhenItemDoesntMatchTryRemoveDoesNotRemove()
613+
{
614+
cache.GetOrAdd(1, k => k);
615+
616+
cache.TryRemove(new KeyValuePair<int, int>(1, 2)).Should().BeFalse();
617+
cache.TryGet(1, out var value).Should().BeTrue();
591618
}
592619

593620
[Fact]

‎BitFaster.Caching.UnitTests/Lru/ClassicLruTests.cs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,33 @@ public void WhenKeyExistsTryRemoveRemovesItemAndReturnsTrue()
362362
lru.TryGet(1, out var value).Should().BeFalse();
363363
}
364364

365+
[Fact]
366+
public void WhenKeyExistsTryRemoveReturnsValue()
367+
{
368+
lru.GetOrAdd(1, valueFactory.Create);
369+
370+
lru.TryRemove(1, out var value).Should().BeTrue();
371+
value.Should().Be("1");
372+
}
373+
374+
[Fact]
375+
public void WhenItemExistsTryRemovesItemAndReturnsTrue()
376+
{
377+
lru.GetOrAdd(1, valueFactory.Create);
378+
379+
lru.TryRemove(new KeyValuePair<int, string>(1, "1")).Should().BeTrue();
380+
lru.TryGet(1, out var value).Should().BeFalse();
381+
}
382+
383+
[Fact]
384+
public void WhenTryRemoveKvpDoesntMatchItemNotRemovedAndReturnsFalse()
385+
{
386+
lru.GetOrAdd(1, valueFactory.Create);
387+
388+
lru.TryRemove(new KeyValuePair<int, string>(1, "2")).Should().BeFalse();
389+
lru.TryGet(1, out var value).Should().BeTrue();
390+
}
391+
365392
[Fact]
366393
public void WhenItemIsRemovedItIsDisposed()
367394
{

‎BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -634,6 +634,15 @@ public void WhenKeyExistsTryRemoveRemovesItemAndReturnsTrue()
634634

635635
lru.TryRemove(1).Should().BeTrue();
636636
lru.TryGet(1, out var value).Should().BeFalse();
637+
}
638+
639+
[Fact]
640+
public void WhenKeyExistsTryRemoveReturnsValue()
641+
{
642+
lru.GetOrAdd(1, valueFactory.Create);
643+
644+
lru.TryRemove(1, out var value).Should().BeTrue();
645+
value.Should().Be("1");
637646
}
638647

639648
[Fact]
@@ -1196,6 +1205,26 @@ await Threaded.Run(4, () => {
11961205
}
11971206
}
11981207

1208+
[Fact]
1209+
public async Task WhenSoakConcurrentGetAndRemoveKvpCacheEndsInConsistentState()
1210+
{
1211+
for (int i = 0; i < 10; i++)
1212+
{
1213+
await Threaded.Run(4, () => {
1214+
for (int i = 0; i < 100000; i++)
1215+
{
1216+
lru.TryRemove(new KeyValuePair<int, string>(i + 1, (i + 1).ToString()));
1217+
lru.GetOrAdd(i + 1, i => i.ToString());
1218+
}
1219+
});
1220+
1221+
this.testOutputHelper.WriteLine($"{lru.HotCount} {lru.WarmCount} {lru.ColdCount}");
1222+
this.testOutputHelper.WriteLine(string.Join(" ", lru.Keys));
1223+
1224+
RunIntegrityCheck();
1225+
}
1226+
}
1227+
11991228
[Fact]
12001229
public async Task WhenSoakConcurrentGetAndUpdateCacheEndsInConsistentState()
12011230
{

‎BitFaster.Caching/Atomic/AtomicFactory.cs‎

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.Collections.Generic;
23
using System.Diagnostics;
34
using System.Threading;
45

@@ -11,7 +12,7 @@ namespace BitFaster.Caching.Atomic
1112
/// <typeparam name="K">The type of the key.</typeparam>
1213
/// <typeparam name="V">The type of the value.</typeparam>
1314
[DebuggerDisplay("IsValueCreated={IsValueCreated}, Value={ValueIfCreated}")]
14-
public sealed class AtomicFactory<K, V>
15+
public sealed class AtomicFactory<K, V> : IEquatable<AtomicFactory<K, V>>
1516
{
1617
private Initializer initializer;
1718

@@ -102,8 +103,36 @@ private V CreateValue<TFactory>(K key, TFactory valueFactory) where TFactory : s
102103
}
103104

104105
return value;
105-
}
106-
106+
}
107+
108+
///<inheritdoc/>
109+
public override bool Equals(object obj)
110+
{
111+
return Equals(obj as AtomicFactory<K, V>);
112+
}
113+
114+
///<inheritdoc/>
115+
public bool Equals(AtomicFactory<K, V> other)
116+
{
117+
if (other is null || !IsValueCreated || !other.IsValueCreated)
118+
{
119+
return false;
120+
}
121+
122+
return EqualityComparer<V>.Default.Equals(ValueIfCreated, other.ValueIfCreated);
123+
}
124+
125+
///<inheritdoc/>
126+
public override int GetHashCode()
127+
{
128+
if (!IsValueCreated)
129+
{
130+
return 0;
131+
}
132+
133+
return ValueIfCreated.GetHashCode();
134+
}
135+
107136
private class Initializer
108137
{
109138
private readonly object syncLock = new();
@@ -129,6 +158,6 @@ public V CreateValue<TFactory>(K key, TFactory valueFactory) where TFactory : st
129158
return value;
130159
}
131160
}
132-
}
161+
}
133162
}
134163
}

‎BitFaster.Caching/Atomic/AtomicFactoryCache.cs‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
using System.Collections;
33
using System.Collections.Generic;
44
using System.Diagnostics;
5-
5+
using System.Linq.Expressions;
6+
67
namespace BitFaster.Caching.Atomic
78
{
89
/// <summary>
@@ -100,9 +101,38 @@ public bool TryGet(K key, out V value)
100101
return true;
101102
}
102103

104+
value = default;
105+
return false;
106+
}
107+
108+
// backcompat: remove conditional compile
109+
#if NETCOREAPP3_0_OR_GREATER
110+
///<inheritdoc/>
111+
///<remarks>
112+
///If the value factory is still executing, returns false.
113+
///</remarks>
114+
public bool TryRemove(KeyValuePair<K, V> item)
115+
{
116+
var kvp = new KeyValuePair<K, AtomicFactory<K, V>>(item.Key, new AtomicFactory<K, V>(item.Value));
117+
return cache.TryRemove(kvp);
118+
}
119+
120+
///<inheritdoc/>
121+
/// <remarks>
122+
/// If the value factory is still executing, the default value will be returned.
123+
/// </remarks>
124+
public bool TryRemove(K key, out V value)
125+
{
126+
if (cache.TryRemove(key, out var atomic))
127+
{
128+
value = atomic.ValueIfCreated;
129+
return true;
130+
}
131+
103132
value = default;
104133
return false;
105134
}
135+
#endif
106136

107137
///<inheritdoc/>
108138
public bool TryRemove(K key)

0 commit comments

Comments
 (0)