Skip to content

Commit ad878cc

Browse files
Merge pull request #72521 from CyrusNajmabadi/solutionCache
2 parents 629715c + 2c3db16 commit ad878cc

File tree

5 files changed

+315
-8
lines changed

5 files changed

+315
-8
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using Microsoft.CodeAnalysis.Remote;
6+
using Xunit;
7+
8+
namespace Roslyn.VisualStudio.Next.UnitTests.Remote;
9+
10+
public class RemoteSolutionCacheTests
11+
{
12+
[Fact]
13+
public void TestAddAndFind1()
14+
{
15+
var cache = new RemoteSolutionCache<int, string>(maxCapacity: 4);
16+
cache.Add(1, "1");
17+
Assert.Equal("1", cache.Find(1));
18+
Assert.Null(cache.Find(2));
19+
}
20+
21+
[Fact]
22+
public void TestAddAndFindOverwrite1()
23+
{
24+
var cache = new RemoteSolutionCache<int, string>(maxCapacity: 4);
25+
cache.Add(1, "1");
26+
cache.Add(1, "2");
27+
Assert.Equal("2", cache.Find(1));
28+
Assert.Null(cache.Find(2));
29+
}
30+
31+
[Fact]
32+
public void TestAddAndFindOverwrite2()
33+
{
34+
var cache = new RemoteSolutionCache<int, string>(maxCapacity: 4);
35+
cache.Add(1, "1");
36+
cache.Add(2, "2");
37+
cache.Add(1, "3");
38+
Assert.Equal("3", cache.Find(1));
39+
Assert.Equal("2", cache.Find(2));
40+
Assert.Null(cache.Find(3));
41+
}
42+
43+
[Fact]
44+
public void TestAddAndFindFour()
45+
{
46+
var cache = new RemoteSolutionCache<int, string>(maxCapacity: 4);
47+
cache.Add(1, "1");
48+
cache.Add(2, "2");
49+
cache.Add(3, "3");
50+
cache.Add(4, "4");
51+
Assert.Equal("1", cache.Find(1));
52+
Assert.Equal("2", cache.Find(2));
53+
Assert.Equal("3", cache.Find(3));
54+
Assert.Equal("4", cache.Find(4));
55+
Assert.Null(cache.Find(5));
56+
}
57+
58+
[Fact]
59+
public void TestAddAndFindFive_A()
60+
{
61+
var cache = new RemoteSolutionCache<int, string>(maxCapacity: 4);
62+
cache.Add(1, "1");
63+
cache.Add(2, "2");
64+
cache.Add(3, "3");
65+
cache.Add(4, "4");
66+
cache.Add(5, "5");
67+
Assert.Null(cache.Find(1));
68+
Assert.Equal("2", cache.Find(2));
69+
Assert.Equal("3", cache.Find(3));
70+
Assert.Equal("4", cache.Find(4));
71+
Assert.Equal("5", cache.Find(5));
72+
Assert.Null(cache.Find(6));
73+
}
74+
75+
[Fact]
76+
public void TestAddAndFindFive_B()
77+
{
78+
var cache = new RemoteSolutionCache<int, string>(maxCapacity: 4);
79+
cache.Add(1, "1");
80+
cache.Add(2, "2");
81+
cache.Add(3, "3");
82+
cache.Add(4, "4");
83+
cache.Add(1, "1"); // re-add. should ensure that this doesn't fall out of the cache.
84+
cache.Add(5, "5");
85+
Assert.Equal("1", cache.Find(1));
86+
Assert.Null(cache.Find(2));
87+
Assert.Equal("3", cache.Find(3));
88+
Assert.Equal("4", cache.Find(4));
89+
Assert.Equal("5", cache.Find(5));
90+
Assert.Null(cache.Find(6));
91+
}
92+
93+
[Fact]
94+
public void TestLargeHistory_A()
95+
{
96+
var cache = new RemoteSolutionCache<int, string>(maxCapacity: 4, totalHistory: 16);
97+
98+
for (var i = 0; i < 20; i++)
99+
cache.Add(i, $"{i}");
100+
101+
for (var i = 0; i < 20; i++)
102+
{
103+
if (i < 16)
104+
{
105+
Assert.Null(cache.Find(i));
106+
}
107+
else
108+
{
109+
Assert.Equal($"{i}", cache.Find(i));
110+
}
111+
}
112+
}
113+
114+
[Fact]
115+
public void TestLargeHistory_B()
116+
{
117+
var cache = new RemoteSolutionCache<int, string>(maxCapacity: 4, totalHistory: 16);
118+
119+
for (var i = 0; i < 20; i++)
120+
cache.Add(i, $"{i}");
121+
122+
for (var i = 20 - 1; i >= 0; i--)
123+
cache.Add(i, $"{i}");
124+
125+
for (var i = 0; i < 20; i++)
126+
{
127+
if (i >= 4)
128+
{
129+
Assert.Null(cache.Find(i));
130+
}
131+
else
132+
{
133+
Assert.Equal($"{i}", cache.Find(i));
134+
}
135+
}
136+
}
137+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System.Collections.Generic;
6+
using Microsoft.CodeAnalysis.Internal.Log;
7+
using Roslyn.Utilities;
8+
9+
namespace Microsoft.CodeAnalysis.Remote;
10+
11+
/// <summary>
12+
/// LRU cache of checksum+solution pairs. Used to keep track of the last few solutions the remote server knows about,
13+
/// helping to avoid unnecessary syncs/recreations of those solutions while many requests are coming into the server.
14+
/// Not threadsafe. Only use while under a lock.
15+
/// </summary>
16+
internal sealed class RemoteSolutionCache<TChecksum, TSolution>
17+
where TChecksum : struct
18+
where TSolution : class
19+
{
20+
/// <summary>
21+
/// The max number of solution instances we'll hold onto at a time.
22+
/// </summary>
23+
private readonly int _maxCapacity;
24+
25+
/// <summary>
26+
/// The total history kept. Used to record telemetry about how useful it would be to increase the max capacity.
27+
/// </summary>
28+
private readonly int _totalHistory;
29+
30+
/// <summary>
31+
/// Keep track of what index we found find a checksum at in the history. This will help us tell both if the cache
32+
/// is too large, or if it's too small.
33+
/// </summary>
34+
private readonly HistogramLogAggregator<int>.HistogramCounter _cacheHitIndexHistogram;
35+
36+
/// <summary>
37+
/// The number of times we successfully found a solution. When this happens we'll increment <see
38+
/// cref="_cacheHitIndexHistogram"/>.
39+
/// </summary>
40+
private int _cacheHits;
41+
42+
/// <summary>
43+
/// The number of times we failed to find a solution, but could have found it if we cached more items (up to
44+
/// TotalHistory). When this happens we'll increment <see cref="_cacheHitIndexHistogram"/>.
45+
/// </summary>
46+
private int _cacheMissesInHistory;
47+
48+
/// <summary>
49+
/// The number of times we failed to find a solution, and would not have found it even if we didn't cache more items.
50+
/// </summary>
51+
private int _cacheMissesNotInHistory;
52+
53+
/// <summary>
54+
/// The list of checksum+solution pairs. Note: only the first <see cref="_maxCapacity"/> items will actually point
55+
/// at a non-null solution. The ones after that will point at <see langword="null"/>. We store both so that we can
56+
/// collect useful telemetry on how much benefit we would get by having a larger history.
57+
/// </summary>
58+
private readonly LinkedList<CacheNode> _cacheNodes = new();
59+
60+
public RemoteSolutionCache(int maxCapacity = 4, int totalHistory = 16)
61+
{
62+
_maxCapacity = maxCapacity;
63+
_totalHistory = totalHistory;
64+
_cacheHitIndexHistogram = new(bucketSize: 1, maxBucketValue: int.MaxValue, bucketCount: _totalHistory + 1);
65+
}
66+
67+
private void FindAndMoveNodeToFront(TChecksum checksum)
68+
{
69+
var index = 0;
70+
for (var current = _cacheNodes.First; current != null; current = current.Next, index++)
71+
{
72+
if (current.Value.Checksum.Equals(checksum))
73+
{
74+
// Found the item.
75+
if (index > 0)
76+
{
77+
// If it's not already at the front, move it there.
78+
_cacheNodes.Remove(current);
79+
_cacheNodes.AddFirst(current);
80+
}
81+
82+
return;
83+
}
84+
}
85+
86+
// Didn't find the item at all. Just add to the front.
87+
_cacheNodes.AddFirst(new CacheNode(checksum));
88+
}
89+
90+
public void Add(TChecksum checksum, TSolution solution)
91+
{
92+
Contract.ThrowIfTrue(_cacheNodes.Count > _totalHistory);
93+
94+
FindAndMoveNodeToFront(checksum);
95+
96+
Contract.ThrowIfTrue(_cacheNodes.Count > _totalHistory + 1);
97+
Contract.ThrowIfNull(_cacheNodes.First);
98+
Contract.ThrowIfFalse(_cacheNodes.First.Value.Checksum.Equals(checksum));
99+
100+
// Ensure we're holding onto the solution.
101+
_cacheNodes.First.Value.Solution = solution;
102+
103+
// Now, if our history is too long, remove the last item.
104+
if (_cacheNodes.Count == _totalHistory + 1)
105+
_cacheNodes.RemoveLast();
106+
107+
// Finally, ensure that only the first `MaxCapacity` are pointing at solutions and the rest are not.
108+
var index = 0;
109+
for (var current = _cacheNodes.First; current != null; current = current.Next, index++)
110+
{
111+
if (index >= _maxCapacity)
112+
{
113+
current.Value.Solution = null;
114+
break;
115+
}
116+
}
117+
}
118+
119+
public TSolution? Find(TChecksum checksum)
120+
{
121+
// Note: we intentionally do not move an item when we find it. That's because our caller will always call 'Add'
122+
// with the found solution afterwards. This will ensure that that solution makes it to the front of the line.
123+
var index = 0;
124+
for (var current = _cacheNodes.First; current != null; current = current.Next, index++)
125+
{
126+
if (current.Value.Checksum.Equals(checksum))
127+
{
128+
// Found it!
129+
if (current.Value.Solution is null)
130+
{
131+
// Track that we would have been able to return this if our history was longer.
132+
_cacheMissesInHistory++;
133+
}
134+
else
135+
{
136+
// Success!
137+
_cacheHits++;
138+
}
139+
140+
_cacheHitIndexHistogram.IncreaseCount(index);
141+
return current.Value.Solution;
142+
}
143+
}
144+
145+
// Couldn't find it at all, even in the history.
146+
_cacheMissesNotInHistory++;
147+
return null;
148+
}
149+
150+
public void ReportTelemetry()
151+
{
152+
Logger.Log(FunctionId.RemoteWorkspace_SolutionCachingStatistics, KeyValueLogMessage.Create(m =>
153+
{
154+
m.Add(nameof(_cacheHits), _cacheHits);
155+
m.Add(nameof(_cacheMissesInHistory), _cacheMissesInHistory);
156+
m.Add(nameof(_cacheMissesNotInHistory), _cacheMissesNotInHistory);
157+
_cacheHitIndexHistogram.WriteTelemetryPropertiesTo(m, prefix: nameof(_cacheHitIndexHistogram));
158+
}));
159+
}
160+
161+
private sealed class CacheNode(TChecksum checksum)
162+
{
163+
public readonly TChecksum Checksum = checksum;
164+
public TSolution? Solution;
165+
}
166+
}

src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public AssetProvider CreateAssetProvider(Checksum solutionChecksum, SolutionAsse
4848
/// <summary>
4949
/// Syncs over the solution corresponding to <paramref name="solutionChecksum"/> and sets it as the current
5050
/// solution for <see langword="this"/> workspace. This will also end up updating <see
51-
/// cref="_lastRequestedAnyBranchSolution"/> and <see cref="_lastRequestedPrimaryBranchSolution"/>, allowing
51+
/// cref="_lastRequestedAnyBranchSolutions"/> and <see cref="_lastRequestedPrimaryBranchSolution"/>, allowing
5252
/// them to be pre-populated for feature requests that come in soon after this call completes.
5353
/// </summary>
5454
public async Task UpdatePrimaryBranchSolutionAsync(
@@ -164,7 +164,7 @@ await RunWithSolutionAsync(
164164
if (updatePrimaryBranch)
165165
_lastRequestedPrimaryBranchSolution = (solutionChecksum, solution);
166166
else
167-
_lastRequestedAnyBranchSolution = (solutionChecksum, solution);
167+
_lastRequestedAnyBranchSolutions.Add(solutionChecksum, solution);
168168
}
169169

170170
// Now, pass it to the callback to do the work. Any other callers into us will be able to benefit from

src/Workspaces/Remote/ServiceHub/Host/RemoteWorkspace_SolutionCaching.cs

+8-6
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@
44

55
using System;
66
using System.Collections.Generic;
7+
using Microsoft.CodeAnalysis.Internal.Log;
78
using Roslyn.Utilities;
89

910
namespace Microsoft.CodeAnalysis.Remote
1011
{
12+
1113
internal sealed partial class RemoteWorkspace
1214
{
1315
/// <summary>
@@ -18,10 +20,10 @@ internal sealed partial class RemoteWorkspace
1820
private (Checksum checksum, Solution solution) _lastRequestedPrimaryBranchSolution;
1921

2022
/// <summary>
21-
/// The last solution requested by a service. Cached as it's very common to have a flurry of requests for the
22-
/// same checksum that don't run concurrently. Only read/write while holding <see cref="_gate"/>.
23+
/// Cache of last N solutions requested by a service. Cached as it's very common to have a flurry of requests
24+
/// for the same few checksum that don't run concurrently. Only read/write while holding <see cref="_gate"/>.
2325
/// </summary>
24-
private (Checksum checksum, Solution solution) _lastRequestedAnyBranchSolution;
26+
private readonly RemoteSolutionCache<Checksum, Solution> _lastRequestedAnyBranchSolutions = new();
2527

2628
/// <summary>
2729
/// Mapping from solution-checksum to the solution computed for it. This is used so that we can hold a solution
@@ -80,9 +82,9 @@ InFlightSolution GetOrCreateSolutionAndAddInFlightCount_NoLock()
8082

8183
// See if we're being asked for a checksum we already have cached a solution for. Safe to read directly
8284
// as we're holding _gate.
83-
var cachedSolution =
84-
_lastRequestedPrimaryBranchSolution.checksum == solutionChecksum ? _lastRequestedPrimaryBranchSolution.solution :
85-
_lastRequestedAnyBranchSolution.checksum == solutionChecksum ? _lastRequestedAnyBranchSolution.solution : null;
85+
var cachedSolution = _lastRequestedPrimaryBranchSolution.checksum == solutionChecksum
86+
? _lastRequestedPrimaryBranchSolution.solution
87+
: _lastRequestedAnyBranchSolutions.Find(solutionChecksum);
8688

8789
// We're the first call that is asking about this checksum. Kick off async computation to compute it
8890
// (or use an existing cached value we already have). Start with an in-flight-count of 1 to represent

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs

+2
Original file line numberDiff line numberDiff line change
@@ -619,4 +619,6 @@ internal enum FunctionId
619619
SuggestedAction_Preview_Summary = 745,
620620

621621
LSP_DocumentIdCacheMiss = 746,
622+
623+
RemoteWorkspace_SolutionCachingStatistics = 750,
622624
}

0 commit comments

Comments
 (0)