Skip to content

Commit 43dcc90

Browse files
bitfasterbitfaster
authored
Mitigate LFU struct tearing using SeqLock (#621)
* seqlock * fix comment ---------
1 parent ab5878b commit 43dcc90

File tree

6 files changed

+258
-69
lines changed

6 files changed

+258
-69
lines changed

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using System.Collections.Generic;
33
using System.Diagnostics;
44
using System.Reflection;
5+
using System.Threading;
56
using System.Threading.Tasks;
67
using BitFaster.Caching.Buffers;
78
using BitFaster.Caching.Lfu;
@@ -316,6 +317,50 @@ public async Task WhenConcurrentUpdateAndRemoveKvp()
316317
await removal;
317318
}
318319

320+
// This test will run forever if there is a live lock.
321+
// Since the cache bookkeeping has some overhead, it is harder to provoke
322+
// spinning inside the reader thread compared to LruItemSoakTests.DetectTornStruct.
323+
[Theory]
324+
[Repeat(10)]
325+
public async Task WhenValueIsBigStructNoLiveLock(int _)
326+
{
327+
using var source = new CancellationTokenSource();
328+
var started = new TaskCompletionSource<bool>();
329+
var cache = new ConcurrentLfu<int, Guid>(1, 20, new BackgroundThreadScheduler(), EqualityComparer<int>.Default);
330+
331+
var setTask = Task.Run(() => Setter(cache, source.Token, started));
332+
await started.Task;
333+
Checker(cache, source);
334+
335+
await setTask;
336+
}
337+
338+
private void Setter(ICache<int, Guid> cache, CancellationToken cancelToken, TaskCompletionSource<bool> started)
339+
{
340+
started.SetResult(true);
341+
342+
while (true)
343+
{
344+
cache.AddOrUpdate(1, Guid.NewGuid());
345+
cache.AddOrUpdate(1, Guid.NewGuid());
346+
347+
if (cancelToken.IsCancellationRequested)
348+
{
349+
return;
350+
}
351+
}
352+
}
353+
354+
private void Checker(ICache<int, Guid> cache,CancellationTokenSource source)
355+
{
356+
for (int count = 0; count < 100_000; ++count)
357+
{
358+
cache.TryGet(1, out _);
359+
}
360+
361+
source.Cancel();
362+
}
363+
319364
private ConcurrentLfu<int, string> CreateWithBackgroundScheduler()
320365
{
321366
var scheduler = new BackgroundThreadScheduler();

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -600,6 +600,19 @@ public void WhenItemIsUpdatedItIsUpdated()
600600
value.Should().Be(2);
601601
}
602602

603+
[Fact]
604+
public void WhenKeyExistsAddOrUpdateGuidUpdatesExistingItem()
605+
{
606+
var lfu2 = new ConcurrentLfu<int, Guid>(1, 40, new BackgroundThreadScheduler(), EqualityComparer<int>.Default);
607+
608+
var b = new byte[8];
609+
lfu2.AddOrUpdate(1, new Guid(1, 0, 0, b));
610+
lfu2.AddOrUpdate(1, new Guid(2, 0, 0, b));
611+
612+
lfu2.TryGet(1, out var value).Should().BeTrue();
613+
value.Should().Be(new Guid(2, 0, 0, b));
614+
}
615+
603616
[Fact]
604617
public void WhenItemDoesNotExistUpdatedAddsItem()
605618
{

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

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
using System;
2+
using System.Threading;
3+
using System.Threading.Tasks;
4+
using BitFaster.Caching.Lfu;
5+
using Xunit;
6+
using static BitFaster.Caching.UnitTests.Lru.LruItemSoakTests;
7+
8+
namespace BitFaster.Caching.UnitTests.Lfu
9+
{
10+
[Collection("Soak")]
11+
public class LfuNodeSoakTest
12+
{
13+
private const int soakIterations = 3;
14+
private readonly LfuNode<int, MassiveStruct> item = new(1, MassiveStruct.A);
15+
16+
// Adapted from
17+
// https://stackoverflow.com/questions/23262513/reproduce-torn-reads-of-decimal-in-c-sharp
18+
[Theory]
19+
[Repeat(soakIterations)]
20+
public async Task DetectTornStruct(int _)
21+
{
22+
using var source = new CancellationTokenSource();
23+
var started = new TaskCompletionSource<bool>();
24+
25+
var setTask = Task.Run(() => Setter(source.Token, started));
26+
await started.Task;
27+
Checker(source);
28+
29+
await setTask;
30+
}
31+
32+
private void Setter(CancellationToken cancelToken, TaskCompletionSource<bool> started)
33+
{
34+
started.SetResult(true);
35+
36+
while (true)
37+
{
38+
item.SeqLockWrite(MassiveStruct.A);
39+
item.SeqLockWrite(MassiveStruct.B);
40+
41+
if (cancelToken.IsCancellationRequested)
42+
{
43+
return;
44+
}
45+
}
46+
}
47+
48+
private void Checker(CancellationTokenSource source)
49+
{
50+
// On my machine, without SeqLock, this consistently fails below 100 iterations
51+
// on debug build, and below 1000 on release build
52+
for (int count = 0; count < 10_000; ++count)
53+
{
54+
var t = item.SeqLockRead();
55+
56+
if (t != MassiveStruct.A && t != MassiveStruct.B)
57+
{
58+
throw new Exception($"Value is torn after {count} iterations");
59+
}
60+
}
61+
62+
source.Cancel();
63+
}
64+
}
65+
}

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

Lines changed: 64 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -15,77 +15,77 @@ public NodeMemoryLayoutDumps(ITestOutputHelper testOutputHelper)
1515
}
1616

1717
//Type layout for 'AccessOrderNode`2'
18-
//Size: 48 bytes.Paddings: 2 bytes(%4 of empty space)
19-
//|====================================================|
20-
//| Object Header(8 bytes) |
21-
//|----------------------------------------------------|
22-
//| Method Table Ptr(8 bytes) |
23-
//|====================================================|
24-
//| 0-7: LfuNodeList`2 list(8 bytes) |
25-
//|----------------------------------------------------|
26-
//| 8-15: LfuNode`2 next(8 bytes) |
27-
//|----------------------------------------------------|
28-
//| 16-23: LfuNode`2 prev(8 bytes) |
29-
//|----------------------------------------------------|
30-
//| 24-31: Object Key(8 bytes) |
31-
//|----------------------------------------------------|
32-
//| 32-39: Object<Value> k__BackingField(8 bytes) |
33-
//|----------------------------------------------------|
34-
//| 40-43: Position<Position> k__BackingField(4 bytes) |
35-
//| |===============================| |
36-
//| | 0-3: Int32 value__(4 bytes) | |
37-
//| |===============================| |
38-
//|----------------------------------------------------|
39-
//| 44: Boolean wasRemoved(1 byte) |
40-
//|----------------------------------------------------|
41-
//| 45: Boolean wasDeleted(1 byte) |
42-
//|----------------------------------------------------|
43-
//| 46-47: padding(2 bytes) |
44-
//|====================================================|
18+
//Size: 48 bytes. Paddings: 0 bytes (%0 of empty space)
19+
//|=====================================================|
20+
//| Object Header (8 bytes) |
21+
//|-----------------------------------------------------|
22+
//| Method Table Ptr (8 bytes) |
23+
//|=====================================================|
24+
//| 0-7: Object data (8 bytes) |
25+
//|-----------------------------------------------------|
26+
//| 8-15: LfuNodeList`2 list (8 bytes) |
27+
//|-----------------------------------------------------|
28+
//| 16-23: LfuNode`2 next (8 bytes) |
29+
//|-----------------------------------------------------|
30+
//| 24-31: LfuNode`2 prev (8 bytes) |
31+
//|-----------------------------------------------------|
32+
//| 32-39: Object Key (8 bytes) |
33+
//|-----------------------------------------------------|
34+
//| 40-43: Int32 sequence (4 bytes) |
35+
//|-----------------------------------------------------|
36+
//| 44-45: Position <Position>k__BackingField (2 bytes) |
37+
//| |================================| |
38+
//| | 0-1: Int16 value__ (2 bytes) | |
39+
//| |================================| |
40+
//|-----------------------------------------------------|
41+
//| 46: Boolean wasRemoved (1 byte) |
42+
//|-----------------------------------------------------|
43+
//| 47: Boolean wasDeleted (1 byte) |
44+
//|=====================================================|
4545
[Fact]
4646
public void DumpAccessOrderNode()
4747
{
4848
var layout = TypeLayout.GetLayout<AccessOrderNode<object, object>>(includePaddings: true);
4949
testOutputHelper.WriteLine(layout.ToString());
5050
}
5151

52-
//Type layout for 'TimeOrderNode`2'
53-
//Size: 72 bytes.Paddings: 2 bytes(%2 of empty space)
54-
//|====================================================|
55-
//| Object Header(8 bytes) |
56-
//|----------------------------------------------------|
57-
//| Method Table Ptr(8 bytes) |
58-
//|====================================================|
59-
//| 0-7: LfuNodeList`2 list(8 bytes) |
60-
//|----------------------------------------------------|
61-
//| 8-15: LfuNode`2 next(8 bytes) |
62-
//|----------------------------------------------------|
63-
//| 16-23: LfuNode`2 prev(8 bytes) |
64-
//|----------------------------------------------------|
65-
//| 24-31: Object Key(8 bytes) |
66-
//|----------------------------------------------------|
67-
//| 32-39: Object<Value> k__BackingField(8 bytes) |
68-
//|----------------------------------------------------|
69-
//| 40-43: Position<Position> k__BackingField(4 bytes) |
70-
//| |===============================| |
71-
//| | 0-3: Int32 value__(4 bytes) | |
72-
//| |===============================| |
73-
//|----------------------------------------------------|
74-
//| 44: Boolean wasRemoved(1 byte) |
75-
//|----------------------------------------------------|
76-
//| 45: Boolean wasDeleted(1 byte) |
77-
//|----------------------------------------------------|
78-
//| 46-47: padding(2 bytes) |
79-
//|----------------------------------------------------|
80-
//| 48-55: TimeOrderNode`2 prevTime(8 bytes) |
81-
//|----------------------------------------------------|
82-
//| 56-63: TimeOrderNode`2 nextTime(8 bytes) |
83-
//|----------------------------------------------------|
84-
//| 64-71: Duration timeToExpire(8 bytes) |
85-
//| |===========================| |
86-
//| | 0-7: Int64 raw(8 bytes) | |
87-
//| |===========================| |
88-
//|====================================================|
52+
// Type layout for 'TimeOrderNode`2'
53+
//Size: 72 bytes. Paddings: 0 bytes (%0 of empty space)
54+
//|=====================================================|
55+
//| Object Header (8 bytes) |
56+
//|-----------------------------------------------------|
57+
//| Method Table Ptr (8 bytes) |
58+
//|=====================================================|
59+
//| 0-7: Object data (8 bytes) |
60+
//|-----------------------------------------------------|
61+
//| 8-15: LfuNodeList`2 list (8 bytes) |
62+
//|-----------------------------------------------------|
63+
//| 16-23: LfuNode`2 next (8 bytes) |
64+
//|-----------------------------------------------------|
65+
//| 24-31: LfuNode`2 prev (8 bytes) |
66+
//|-----------------------------------------------------|
67+
//| 32-39: Object Key (8 bytes) |
68+
//|-----------------------------------------------------|
69+
//| 40-43: Int32 sequence (4 bytes) |
70+
//|-----------------------------------------------------|
71+
//| 44-45: Position <Position>k__BackingField (2 bytes) |
72+
//| |================================| |
73+
//| | 0-1: Int16 value__ (2 bytes) | |
74+
//| |================================| |
75+
//|-----------------------------------------------------|
76+
//| 46: Boolean wasRemoved (1 byte) |
77+
//|-----------------------------------------------------|
78+
//| 47: Boolean wasDeleted (1 byte) |
79+
//|-----------------------------------------------------|
80+
//| 48-55: TimeOrderNode`2 prevTime (8 bytes) |
81+
//|-----------------------------------------------------|
82+
//| 56-63: TimeOrderNode`2 nextTime (8 bytes) |
83+
//|-----------------------------------------------------|
84+
//| 64-71: Duration timeToExpire (8 bytes) |
85+
//| |============================| |
86+
//| | 0-7: Int64 raw (8 bytes) | |
87+
//| |============================| |
88+
//|=====================================================|
8989
[Fact]
9090
public void DumpTimeOrderNode()
9191
{

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -345,8 +345,6 @@ private void Setter(ICache<int, Guid> cache, CancellationToken cancelToken, Task
345345

346346
private void Checker(ICache<int, Guid> cache,CancellationTokenSource source)
347347
{
348-
// On my machine, without SeqLock, this consistently fails below 100 iterations
349-
// on debug build, and below 1000 on release build
350348
for (int count = 0; count < 100_000; ++count)
351349
{
352350
cache.TryGet(1, out _);

0 commit comments

Comments
 (0)