Skip to content

Commit 84f25b8

Browse files
michaelstaibmichaelstaib
committedJul 21, 2021
Fixed cache locking issue. (#3975)
·
16.0.0-p.111.3.2
1 parent 18aae91 commit 84f25b8

File tree

2 files changed

+240
-53
lines changed

2 files changed

+240
-53
lines changed
 

‎src/HotChocolate/Utilities/src/Utilities/Cache.cs

+126-51
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,41 @@
1-
using System.Collections.Concurrent;
2-
using System;
3-
using System.Collections.Generic;
1+
using System;
2+
using System.Collections.Concurrent;
43
using System.Diagnostics.CodeAnalysis;
4+
using System.Runtime.CompilerServices;
55

66
namespace HotChocolate.Utilities
77
{
8-
public class Cache<TValue>
8+
public sealed class Cache<TValue>
99
{
1010
private const int _minimumSize = 10;
1111
private readonly object _sync = new();
12-
private readonly LinkedList<string> _ranking = new();
13-
private readonly ConcurrentDictionary<string, CacheEntry> _cache = new();
14-
private LinkedListNode<string>? _first;
12+
private readonly ConcurrentDictionary<string, Entry> _map = new(StringComparer.Ordinal);
13+
private readonly int _size;
14+
private readonly int _order;
15+
private int _usage;
16+
private Entry? _head;
1517

1618
public Cache(int size)
1719
{
18-
Size = size < _minimumSize ? _minimumSize : size;
20+
_size = size < _minimumSize ? _minimumSize : size;
21+
_order = Convert.ToInt32(size * 0.7);
1922
}
2023

21-
public int Size { get; }
24+
/// <summary>
25+
/// Gets the maximum allowed item count that can be stored in this cache.
26+
/// </summary>
27+
public int Size => _size;
2228

23-
public int Usage => _cache.Count;
29+
/// <summary>
30+
/// Gets the current item count that is currently stored in this cache.
31+
/// </summary>
32+
public int Usage => _usage;
2433

2534
public bool TryGet(string key, [MaybeNull] out TValue value)
2635
{
27-
if (_cache.TryGetValue(key, out CacheEntry? entry))
36+
if (_map.TryGetValue(key, out Entry? entry))
2837
{
29-
TouchEntry(entry.Rank);
38+
TouchEntryUnsafe(entry);
3039
value = entry.Value;
3140
return true;
3241
}
@@ -47,14 +56,17 @@ public TValue GetOrCreate(string key, Func<TValue> create)
4756
throw new ArgumentNullException(nameof(create));
4857
}
4958

50-
if (_cache.TryGetValue(key, out CacheEntry? entry))
59+
var read = true;
60+
61+
Entry entry = _map.GetOrAdd(key, k =>
5162
{
52-
TouchEntry(entry.Rank);
53-
}
54-
else
63+
read = false;
64+
return AddNewEntry(k, create());
65+
});
66+
67+
if (read)
5568
{
56-
entry = new CacheEntry(key, create());
57-
AddNewEntry(entry);
69+
TouchEntryUnsafe(entry);
5870
}
5971

6072
return entry.Value;
@@ -64,67 +76,130 @@ public void Clear()
6476
{
6577
lock (_sync)
6678
{
67-
_cache.Clear();
68-
_ranking.Clear();
69-
_first = null;
79+
_map.Clear();
80+
_head = null;
81+
_usage = 0;
7082
}
7183
}
7284

73-
private void TouchEntry(LinkedListNode<string> rank)
85+
internal string[] GetKeys()
7486
{
75-
if (_first != rank)
87+
lock (_sync)
7688
{
77-
lock (_sync)
89+
if (_head is null)
7890
{
79-
if (_ranking.First != rank)
80-
{
81-
_ranking.Remove(rank);
82-
_ranking.AddFirst(rank);
83-
_first = rank;
84-
}
91+
return Array.Empty<string>();
8592
}
93+
94+
var index = 0;
95+
var keys = new string[_usage];
96+
Entry current = _head!;
97+
98+
do
99+
{
100+
keys[index++] = current!.Key;
101+
current = current.Next!;
102+
103+
} while (current != _head);
104+
105+
return keys;
86106
}
87107
}
88108

89-
private void AddNewEntry(CacheEntry entry)
109+
private Entry AddNewEntry(string key, TValue value)
90110
{
91-
if (_cache.TryAdd(entry.Key, entry))
111+
lock (_sync)
92112
{
93-
lock (_sync)
94-
{
95-
ClearSpaceForNewEntry();
96-
_ranking.AddFirst(entry.Rank);
97-
_first = entry.Rank;
98-
}
113+
var entry = new Entry { Key = key, Value = value };
114+
AppendEntryUnsafe(entry);
115+
ClearSpaceForNewEntryUnsafe();
116+
return entry;
117+
}
118+
}
119+
120+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
121+
private void ClearSpaceForNewEntryUnsafe()
122+
{
123+
while (_head is not null && _usage > _size)
124+
{
125+
Entry last = _head.Previous!;
126+
RemoveEntryUnsafe(last);
127+
_map.TryRemove(last.Key, out _);
99128
}
100129
}
101130

102-
private void ClearSpaceForNewEntry()
131+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
132+
private void TouchEntryUnsafe(Entry touched)
103133
{
104-
if (_cache.Count > Size)
134+
if (_order > _usage || _head == touched)
135+
{
136+
return;
137+
}
138+
139+
lock (_sync)
105140
{
106-
LinkedListNode<string>? rank = _ranking.Last;
107-
if (rank is { } && _cache.TryRemove(rank.Value, out CacheEntry? entry))
141+
if (RemoveEntryUnsafe(touched))
108142
{
109-
_ranking.Remove(rank);
143+
AppendEntryUnsafe(touched);
110144
}
111145
}
112146
}
113147

114-
private class CacheEntry
148+
private void AppendEntryUnsafe(Entry newEntry)
115149
{
116-
public CacheEntry(string key, TValue value)
150+
if (_head is not null)
151+
{
152+
newEntry.Next = _head;
153+
newEntry.Previous = _head.Previous;
154+
_head.Previous!.Next = newEntry;
155+
_head.Previous = newEntry;
156+
_head = newEntry;
157+
}
158+
else
117159
{
118-
Key = key ?? throw new ArgumentNullException(nameof(key));
119-
Rank = new LinkedListNode<string>(key);
120-
Value = value;
160+
newEntry.Next = newEntry;
161+
newEntry.Previous = newEntry;
162+
_head = newEntry;
121163
}
122164

123-
public string Key { get; }
165+
_usage++;
166+
}
124167

125-
public LinkedListNode<string> Rank { get; }
168+
private bool RemoveEntryUnsafe(Entry entry)
169+
{
170+
if (entry.Next == null)
171+
{
172+
return false;
173+
}
126174

127-
public TValue Value { get; }
175+
if (entry.Next == entry)
176+
{
177+
_head = null;
178+
}
179+
else
180+
{
181+
entry.Next!.Previous = entry.Previous;
182+
entry.Previous!.Next = entry.Next;
183+
184+
if (_head == entry)
185+
{
186+
_head = entry.Next;
187+
}
188+
189+
entry.Next = null;
190+
entry.Previous = null;
191+
}
192+
193+
_usage--;
194+
return true;
195+
}
196+
197+
private class Entry
198+
{
199+
public string Key = default!;
200+
public TValue Value = default!;
201+
public Entry? Next;
202+
public Entry? Previous;
128203
}
129204
}
130205
}

‎src/HotChocolate/Utilities/test/Utilities.Tests/CacheTests.cs

+114-2
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@ public void Fill_Cache_Up()
99
{
1010
// arrange
1111
var cache = new Cache<string>(10);
12+
1213
for (var i = 0; i < 9; i++)
1314
{
14-
cache.GetOrCreate(i.ToString(), () => i.ToString());
15+
var item = i.ToString();
16+
cache.GetOrCreate(item, () => item);
1517
}
1618

1719
// assert
@@ -21,16 +23,31 @@ public void Fill_Cache_Up()
2123
Assert.Equal("10", value);
2224
Assert.Equal(10, cache.Size);
2325
Assert.Equal(10, cache.Usage);
26+
27+
Assert.Collection(
28+
cache.GetKeys(),
29+
key => Assert.Equal("10", key),
30+
key => Assert.Equal("8", key),
31+
key => Assert.Equal("7", key),
32+
key => Assert.Equal("6", key),
33+
key => Assert.Equal("5", key),
34+
key => Assert.Equal("4", key),
35+
key => Assert.Equal("3", key),
36+
key => Assert.Equal("2", key),
37+
key => Assert.Equal("1", key),
38+
key => Assert.Equal("0", key));
2439
}
2540

2641
[Fact]
2742
public void Add_More_Items_To_The_Cache_Than_We_Have_Space()
2843
{
2944
// arrange
3045
var cache = new Cache<string>(10);
46+
3147
for (var i = 0; i < 10; i++)
3248
{
33-
cache.GetOrCreate(i.ToString(), () => i.ToString());
49+
var item = i.ToString();
50+
cache.GetOrCreate(item, () => item);
3451
}
3552

3653
// assert
@@ -40,6 +57,101 @@ public void Add_More_Items_To_The_Cache_Than_We_Have_Space()
4057
Assert.Equal("10", value);
4158
Assert.Equal(10, cache.Size);
4259
Assert.Equal(10, cache.Usage);
60+
61+
Assert.Collection(
62+
cache.GetKeys(),
63+
key => Assert.Equal("10", key),
64+
key => Assert.Equal("9", key),
65+
key => Assert.Equal("8", key),
66+
key => Assert.Equal("7", key),
67+
key => Assert.Equal("6", key),
68+
key => Assert.Equal("5", key),
69+
key => Assert.Equal("4", key),
70+
key => Assert.Equal("3", key),
71+
key => Assert.Equal("2", key),
72+
key => Assert.Equal("1", key));
73+
}
74+
75+
[Fact]
76+
public void Avoid_Item_Reorder_If_Cache_Is_Not_Full()
77+
{
78+
// arrange
79+
var cache = new Cache<string>(10);
80+
cache.GetOrCreate("a", () => "a");
81+
cache.GetOrCreate("b", () => "b");
82+
cache.GetOrCreate("c", () => "c");
83+
cache.GetOrCreate("d", () => "d");
84+
cache.GetOrCreate("e", () => "e");
85+
cache.GetOrCreate("f", () => "f");
86+
87+
// act
88+
Assert.True(cache.TryGet("c", out _));
89+
90+
// assert
91+
Assert.Collection(
92+
cache.GetKeys(),
93+
key => Assert.Equal("f", key),
94+
key => Assert.Equal("e", key),
95+
key => Assert.Equal("d", key),
96+
key => Assert.Equal("c", key),
97+
key => Assert.Equal("b", key),
98+
key => Assert.Equal("a", key));
99+
}
100+
101+
[Fact]
102+
public void Reorder_Items_When_Cache_Is_Filling_Up_With_TryGet()
103+
{
104+
// arrange
105+
var cache = new Cache<string>(10);
106+
cache.GetOrCreate("a", () => "a");
107+
cache.GetOrCreate("b", () => "b");
108+
cache.GetOrCreate("c", () => "c");
109+
cache.GetOrCreate("d", () => "d");
110+
cache.GetOrCreate("e", () => "e");
111+
cache.GetOrCreate("f", () => "f");
112+
cache.GetOrCreate("g", () => "g");
113+
114+
// act
115+
Assert.True(cache.TryGet("c", out _));
116+
117+
// assert
118+
Assert.Collection(
119+
cache.GetKeys(),
120+
key => Assert.Equal("c", key),
121+
key => Assert.Equal("g", key),
122+
key => Assert.Equal("f", key),
123+
key => Assert.Equal("e", key),
124+
key => Assert.Equal("d", key),
125+
key => Assert.Equal("b", key),
126+
key => Assert.Equal("a", key));
127+
}
128+
129+
[Fact]
130+
public void Reorder_Items_When_Cache_Is_Filling_Up_With_GetOrCreate()
131+
{
132+
// arrange
133+
var cache = new Cache<string>(10);
134+
cache.GetOrCreate("a", () => "a");
135+
cache.GetOrCreate("b", () => "b");
136+
cache.GetOrCreate("c", () => "c");
137+
cache.GetOrCreate("d", () => "d");
138+
cache.GetOrCreate("e", () => "e");
139+
cache.GetOrCreate("f", () => "f");
140+
cache.GetOrCreate("g", () => "g");
141+
142+
// act
143+
cache.GetOrCreate("c", () => "c");
144+
145+
// assert
146+
Assert.Collection(
147+
cache.GetKeys(),
148+
key => Assert.Equal("c", key),
149+
key => Assert.Equal("g", key),
150+
key => Assert.Equal("f", key),
151+
key => Assert.Equal("e", key),
152+
key => Assert.Equal("d", key),
153+
key => Assert.Equal("b", key),
154+
key => Assert.Equal("a", key));
43155
}
44156
}
45157
}

0 commit comments

Comments
 (0)
Please sign in to comment.