Skip to content

Commit d5cae59

Browse files
alex-jitbitalex-jitbit
committed
Performance! No double lookups in GetOrAdd, faster Enumerator etc.
Also more unit tests
1 parent a4b8631 commit d5cae59

File tree

3 files changed

+348
-44
lines changed

3 files changed

+348
-44
lines changed

‎FastCache/FastCache.cs

Lines changed: 68 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public void EvictExpired()
6464

6565
foreach (var p in _dict)
6666
{
67-
if (currTime > p.Value.TickCountWhenToKill) //instead of calling "p.Value.IsExpired" we're essentially doing the same thing manually
67+
if (p.Value.IsExpired(currTime)) //call IsExpired with "currTime" to avoid calling Environment.TickCount64 multiple times
6868
_dict.TryRemove(p);
6969
}
7070
}
@@ -181,14 +181,23 @@ public bool TryAdd(TKey key, TValue value, TimeSpan ttl)
181181
/// <param name="ttl">TTL of the item</param>
182182
public TValue GetOrAdd(TKey key, Func<TKey, TValue> valueFactory, TimeSpan ttl)
183183
{
184-
if (TryGet(key, out var value))
185-
return value;
186-
187-
return _dict.GetOrAdd(
184+
bool wasAdded = false; //flag to indicate "add vs get". TODO: wrap in ref type some day to avoid captures/closures
185+
var ttlValue = _dict.GetOrAdd(
188186
key,
189-
(k, arg) => new TtlValue(arg.valueFactory(k), arg.ttl),
190-
(ttl, valueFactory)
191-
).Value;
187+
(k) =>
188+
{
189+
wasAdded = true;
190+
return new TtlValue(valueFactory(k), ttl);
191+
});
192+
193+
//if the item is expired, update value and TTL
194+
//since TtlValue is a reference type we can update its properties in-place, instead of removing and re-adding to the dictionary (extra lookups)
195+
if (!wasAdded) //performance hack: skip expiration check if a brand item was just added
196+
{
197+
ttlValue.ModifyIfExpired(() => valueFactory(key), ttl);
198+
}
199+
200+
return ttlValue.Value;
192201
}
193202

194203
/// <summary>
@@ -200,14 +209,23 @@ public TValue GetOrAdd(TKey key, Func<TKey, TValue> valueFactory, TimeSpan ttl)
200209
/// <param name="factoryArgument">Argument value to pass into valueFactory</param>
201210
public TValue GetOrAdd<TArg>(TKey key, Func<TKey, TArg, TValue> valueFactory, TimeSpan ttl, TArg factoryArgument)
202211
{
203-
if (TryGet(key, out var value))
204-
return value;
205-
206-
return _dict.GetOrAdd(
212+
bool wasAdded = false; //flag to indicate "add vs get"
213+
var ttlValue = _dict.GetOrAdd(
207214
key,
208-
(k, arg) => new TtlValue(arg.valueFactory(k, arg.factoryArgument), arg.ttl),
209-
(ttl, valueFactory, factoryArgument)
210-
).Value;
215+
(k) =>
216+
{
217+
wasAdded = true;
218+
return new TtlValue(valueFactory(k, factoryArgument), ttl);
219+
});
220+
221+
//if the item is expired, update value and TTL
222+
//since TtlValue is a reference type we can update its properties in-place, instead of removing and re-adding to the dictionary (extra lookups)
223+
if (!wasAdded) //performance hack: skip expiration check if a brand item was just added
224+
{
225+
ttlValue.ModifyIfExpired(() => valueFactory(key, factoryArgument), ttl);
226+
}
227+
228+
return ttlValue.Value;
211229
}
212230

213231
/// <summary>
@@ -218,10 +236,22 @@ public TValue GetOrAdd<TArg>(TKey key, Func<TKey, TArg, TValue> valueFactory, Ti
218236
/// <param name="ttl">TTL of the item</param>
219237
public TValue GetOrAdd(TKey key, TValue value, TimeSpan ttl)
220238
{
221-
if (TryGet(key, out var existingValue))
222-
return existingValue;
239+
bool wasAdded = false; //flag to indicate "add vs get"
240+
var ttlValue = _dict.GetOrAdd(key,
241+
(k) =>
242+
{
243+
wasAdded = true;
244+
return new TtlValue(value, ttl);
245+
});
246+
247+
//if the item is expired, update value and TTL
248+
//since TtlValue is a reference type we can update its properties in-place, instead of removing and re-adding to the dictionary (extra lookups)
249+
if (!wasAdded) //performance hack: skip expiration check if a brand item was just added
250+
{
251+
ttlValue.ModifyIfExpired(() => value, ttl);
252+
}
223253

224-
return _dict.GetOrAdd(key, new TtlValue(value, ttl)).Value;
254+
return ttlValue.Value;
225255
}
226256

227257
/// <summary>
@@ -245,11 +275,13 @@ public bool TryRemove(TKey key, out TValue value)
245275
return res;
246276
}
247277

278+
/// <inheritdoc/>
248279
public IEnumerator<KeyValuePair<TKey, TValue>> GetEnumerator()
249280
{
281+
var currTime = Environment.TickCount64; //save to a var to prevent multiple calls to Environment.TickCount64
250282
foreach (var kvp in _dict)
251283
{
252-
if (!kvp.Value.IsExpired())
284+
if (!kvp.Value.IsExpired(currTime))
253285
yield return new KeyValuePair<TKey, TValue>(kvp.Key, kvp.Value.Value);
254286
}
255287
}
@@ -261,18 +293,31 @@ IEnumerator IEnumerable.GetEnumerator()
261293

262294
private class TtlValue
263295
{
264-
public readonly TValue Value;
265-
public readonly long TickCountWhenToKill;
296+
public TValue Value { get; private set; }
297+
private long TickCountWhenToKill;
266298

267299
public TtlValue(TValue value, TimeSpan ttl)
268300
{
269301
Value = value;
270302
TickCountWhenToKill = Environment.TickCount64 + (long)ttl.TotalMilliseconds;
271303
}
272304

273-
public bool IsExpired()
305+
public bool IsExpired() => IsExpired(Environment.TickCount64);
306+
307+
//use an overload instead of optional param to avoid extra IF's
308+
public bool IsExpired(long currTime) => currTime > TickCountWhenToKill;
309+
310+
/// <summary>
311+
/// Updates the value and TTL only if the item is expired
312+
/// </summary>
313+
public void ModifyIfExpired(Func<TValue> newValueFactory, TimeSpan newTtl)
274314
{
275-
return Environment.TickCount64 > TickCountWhenToKill;
315+
var ticks = Environment.TickCount64; //save to a var to prevent multiple calls to Environment.TickCount64
316+
if (IsExpired(ticks)) //if expired - update the value and TTL
317+
{
318+
TickCountWhenToKill = ticks + (long)newTtl.TotalMilliseconds; //update the expiration time first for better concurrency
319+
Value = newValueFactory();
320+
}
276321
}
277322
}
278323

‎UnitTests/UnitTests.cs

Lines changed: 51 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ public class UnitTests
99
[TestMethod]
1010
public async Task TestGetSetCleanup()
1111
{
12-
var _cache = new FastCache<int, int>(cleanupJobInterval: 200);
12+
using var _cache = new FastCache<int, int>(cleanupJobInterval: 200); //add "using" to stop cleanup timer, to prevent cleanup job from clashing with other tests
1313
_cache.AddOrUpdate(42, 42, TimeSpan.FromMilliseconds(100));
1414
Assert.IsTrue(_cache.TryGet(42, out int v));
1515
Assert.IsTrue(v == 42);
@@ -22,24 +22,24 @@ public async Task TestGetSetCleanup()
2222
public async Task TestEviction()
2323
{
2424
var list = new List<FastCache<int, int>>();
25-
for (int i = 0; i < 20; i++)
26-
{
27-
var cache = new FastCache<int, int>(cleanupJobInterval: 200);
28-
cache.AddOrUpdate(42, 42, TimeSpan.FromMilliseconds(100));
29-
list.Add(cache);
25+
for (int i = 0; i < 20; i++)
26+
{
27+
var cache = new FastCache<int, int>(cleanupJobInterval: 200);
28+
cache.AddOrUpdate(42, 42, TimeSpan.FromMilliseconds(100));
29+
list.Add(cache);
3030
}
3131
await Task.Delay(300);
3232

33-
for (int i = 0; i < 20; i++)
34-
{
35-
Assert.IsTrue(list[i].Count == 0); //cleanup job has run?
36-
}
37-
38-
//cleanup
39-
for (int i = 0; i < 20; i++)
40-
{
41-
list[i].Dispose();
42-
}
33+
for (int i = 0; i < 20; i++)
34+
{
35+
Assert.IsTrue(list[i].Count == 0); //cleanup job has run?
36+
}
37+
38+
//cleanup
39+
for (int i = 0; i < 20; i++)
40+
{
41+
list[i].Dispose();
42+
}
4343
}
4444

4545
[TestMethod]
@@ -80,9 +80,9 @@ public void TestTryRemove()
8080
cache.AddOrUpdate("42", 42, TimeSpan.FromMilliseconds(100));
8181
var res = cache.TryRemove("42", out int value);
8282
Assert.IsTrue(res && value == 42);
83-
Assert.IsFalse(cache.TryGet("42", out _));
84-
85-
//now try remove non-existing item
83+
Assert.IsFalse(cache.TryGet("42", out _));
84+
85+
//now try remove non-existing item
8686
res = cache.TryRemove("blabblah", out value);
8787
Assert.IsFalse(res);
8888
Assert.IsTrue(value == 0);
@@ -117,10 +117,33 @@ public async Task TestGetOrAdd()
117117
{
118118
var cache = new FastCache<string, int>();
119119
cache.GetOrAdd("key", k => 1024, TimeSpan.FromMilliseconds(100));
120-
Assert.IsTrue(cache.TryGet("key", out int res) && res == 1024);
120+
Assert.AreEqual(cache.GetOrAdd("key", k => 1025, TimeSpan.FromMilliseconds(100)), 1024); //old value
121+
Assert.IsTrue(cache.TryGet("key", out int res) && res == 1024); //another way to retrieve
121122
await Task.Delay(110);
122123

123-
Assert.IsFalse(cache.TryGet("key", out _));
124+
Assert.IsFalse(cache.TryGet("key", out _)); //expired
125+
126+
//now try non-factory overloads
127+
Assert.IsTrue(cache.GetOrAdd("key123", 123321, TimeSpan.FromMilliseconds(100)) == 123321);
128+
Assert.IsTrue(cache.GetOrAdd("key123", -1, TimeSpan.FromMilliseconds(100)) == 123321); //still old value
129+
await Task.Delay(110);
130+
Assert.IsTrue(cache.GetOrAdd("key123", -1, TimeSpan.FromMilliseconds(100)) == -1); //new value
131+
}
132+
133+
134+
[TestMethod]
135+
public async Task TestGetOrAddExpiration()
136+
{
137+
var cache = new FastCache<string, int>();
138+
cache.GetOrAdd("key", k => 1024, TimeSpan.FromMilliseconds(100));
139+
140+
Assert.AreEqual(cache.GetOrAdd("key", k => 1025, TimeSpan.FromMilliseconds(100)), 1024); //old value
141+
Assert.IsTrue(cache.TryGet("key", out int res) && res == 1024); //another way to retrieve
142+
143+
await Task.Delay(110); //let the item expire
144+
145+
Assert.AreEqual(cache.GetOrAdd("key", k => 1025, TimeSpan.FromMilliseconds(100)), 1025); //new value
146+
Assert.IsTrue(cache.TryGet("key", out res) && res == 1025); //another way to retrieve
124147
}
125148

126149
[TestMethod]
@@ -133,6 +156,12 @@ public async Task TestGetOrAddWithArg()
133156
//eviction
134157
await Task.Delay(110);
135158
Assert.IsFalse(cache.TryGet("key", out _));
159+
160+
//now try without "TryGet"
161+
Assert.IsTrue(cache.GetOrAdd("key2", (k, arg) => 21 + arg.Length, TimeSpan.FromMilliseconds(100), "123") == 24);
162+
Assert.IsTrue(cache.GetOrAdd("key2", (k, arg) => 2211 + arg.Length, TimeSpan.FromMilliseconds(100), "123") == 24);
163+
await Task.Delay(110);
164+
Assert.IsTrue(cache.GetOrAdd("key2", (k, arg) => 2211 + arg.Length, TimeSpan.FromMilliseconds(100), "123") == 2214);
136165
}
137166

138167
[TestMethod]
@@ -164,6 +193,7 @@ await TestHelper.RunConcurrently(20, () => {
164193
Assert.IsTrue(i == 1, i.ToString());
165194
}
166195

196+
//this text can occasionally fail becasue factory is not guaranteed to be called only once. only panic if it fails ALL THE TIME
167197
[TestMethod]
168198
public async Task TestGetOrAddAtomicNess()
169199
{

0 commit comments

Comments
 (0)