Skip to content

Commit

Permalink
Switch to strongly typed keys in diagnostics subsystem (#77055)
Browse files Browse the repository at this point in the history
  • Loading branch information
@CyrusNajmabadi
CyrusNajmabadi authored Feb 5, 2025
2 parents dead7da + 58092a9 commit 45350db
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 47 deletions.
32 changes: 7 additions & 25 deletions ...r/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.InMemoryStorage.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

using System.Collections.Concurrent;
using System.Collections.Immutable;
using Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Diagnostics.EngineV2
Expand All @@ -13,30 +14,25 @@ internal partial class DiagnosticIncrementalAnalyzer
private static class InMemoryStorage
{
// the reason using nested map rather than having tuple as key is so that I dont have a gigantic map
private static readonly ConcurrentDictionary<DiagnosticAnalyzer, ConcurrentDictionary<(object key, string stateKey), CacheEntry>> s_map =
private static readonly ConcurrentDictionary<DiagnosticAnalyzer, ConcurrentDictionary<(ProjectOrDocumentId key, string stateKey), CacheEntry>> s_map =
new(concurrencyLevel: 2, capacity: 10);

public static bool TryGetValue(DiagnosticAnalyzer analyzer, (object key, string stateKey) key, out CacheEntry entry)
public static bool TryGetValue(DiagnosticAnalyzer analyzer, (ProjectOrDocumentId key, string stateKey) key, out CacheEntry entry)
{
AssertKey(key);

entry = default;
return s_map.TryGetValue(analyzer, out var analyzerMap) &&
analyzerMap.TryGetValue(key, out entry);
}

public static void Cache(DiagnosticAnalyzer analyzer, (object key, string stateKey) key, CacheEntry entry)
public static void Cache(DiagnosticAnalyzer analyzer, (ProjectOrDocumentId key, string stateKey) key, CacheEntry entry)
{
AssertKey(key);

// add new cache entry
var analyzerMap = s_map.GetOrAdd(analyzer, _ => new ConcurrentDictionary<(object key, string stateKey), CacheEntry>(concurrencyLevel: 2, capacity: 10));
var analyzerMap = s_map.GetOrAdd(analyzer, _ => new ConcurrentDictionary<(ProjectOrDocumentId key, string stateKey), CacheEntry>(concurrencyLevel: 2, capacity: 10));
analyzerMap[key] = entry;
}

public static void Remove(DiagnosticAnalyzer analyzer, (object key, string stateKey) key)
public static void Remove(DiagnosticAnalyzer analyzer, (ProjectOrDocumentId key, string stateKey) key)
{
AssertKey(key);
// remove the entry
if (!s_map.TryGetValue(analyzer, out var analyzerMap))
{
Expand All @@ -56,23 +52,9 @@ public static void DropCache(DiagnosticAnalyzer analyzer)
// drop any cache related to given analyzer
s_map.TryRemove(analyzer, out _);
}

// make sure key is either documentId or projectId
private static void AssertKey((object key, string stateKey) key)
=> Contract.ThrowIfFalse(key.key is DocumentId or ProjectId);
}

// in memory cache entry
private readonly struct CacheEntry
{
public readonly VersionStamp Version;
public readonly ImmutableArray<DiagnosticData> Diagnostics;

public CacheEntry(VersionStamp version, ImmutableArray<DiagnosticData> diagnostics)
{
Version = version;
Diagnostics = diagnostics;
}
}
private readonly record struct CacheEntry(VersionStamp Version, ImmutableArray<DiagnosticData> Diagnostics);
}
}
34 changes: 15 additions & 19 deletions ...rver/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.ProjectState.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Shared.Extensions;
Expand Down Expand Up @@ -217,12 +218,12 @@ public async ValueTask SaveToInMemoryStorageAsync(Project project, DiagnosticAna
continue;
}

AddToInMemoryStorage(serializerVersion, project, document, document.Id, SyntaxStateName, result.GetDocumentDiagnostics(document.Id, AnalysisKind.Syntax));
AddToInMemoryStorage(serializerVersion, project, document, document.Id, SemanticStateName, result.GetDocumentDiagnostics(document.Id, AnalysisKind.Semantic));
AddToInMemoryStorage(serializerVersion, project, document, document.Id, NonLocalStateName, result.GetDocumentDiagnostics(document.Id, AnalysisKind.NonLocal));
AddToInMemoryStorage(serializerVersion, new(document.Id), SyntaxStateName, result.GetDocumentDiagnostics(document.Id, AnalysisKind.Syntax));
AddToInMemoryStorage(serializerVersion, new(document.Id), SemanticStateName, result.GetDocumentDiagnostics(document.Id, AnalysisKind.Semantic));
AddToInMemoryStorage(serializerVersion, new(document.Id), NonLocalStateName, result.GetDocumentDiagnostics(document.Id, AnalysisKind.NonLocal));
}

AddToInMemoryStorage(serializerVersion, project, document: null, result.ProjectId, NonLocalStateName, result.GetOtherDiagnostics());
AddToInMemoryStorage(serializerVersion, new(result.ProjectId), NonLocalStateName, result.GetOtherDiagnostics());
}

private async Task<DiagnosticAnalysisResult> LoadInitialAnalysisDataAsync(Project project, CancellationToken cancellationToken)
Expand Down Expand Up @@ -277,11 +278,8 @@ private async Task<DiagnosticAnalysisResult> LoadInitialProjectAnalysisDataAsync
}

private void AddToInMemoryStorage(
VersionStamp serializerVersion, Project project, TextDocument? document, object key, string stateKey, ImmutableArray<DiagnosticData> diagnostics)
VersionStamp serializerVersion, ProjectOrDocumentId key, string stateKey, ImmutableArray<DiagnosticData> diagnostics)
{
Contract.ThrowIfFalse(document == null || document.Project == project);

// if serialization fail, hold it in the memory
InMemoryStorage.Cache(_owner.Analyzer, (key, stateKey), new CacheEntry(serializerVersion, diagnostics));
}

Expand All @@ -291,7 +289,7 @@ private async ValueTask<bool> TryGetDiagnosticsFromInMemoryStorageAsync(VersionS
var project = document.Project;
var documentId = document.Id;

var diagnostics = await GetDiagnosticsFromInMemoryStorageAsync(serializerVersion, project, document, documentId, SyntaxStateName, cancellationToken).ConfigureAwait(false);
var diagnostics = await GetDiagnosticsFromInMemoryStorageAsync(serializerVersion, new(documentId), SyntaxStateName, cancellationToken).ConfigureAwait(false);
if (!diagnostics.IsDefault)
{
builder.AddSyntaxLocals(documentId, diagnostics);
Expand All @@ -301,7 +299,7 @@ private async ValueTask<bool> TryGetDiagnosticsFromInMemoryStorageAsync(VersionS
success = false;
}

diagnostics = await GetDiagnosticsFromInMemoryStorageAsync(serializerVersion, project, document, documentId, SemanticStateName, cancellationToken).ConfigureAwait(false);
diagnostics = await GetDiagnosticsFromInMemoryStorageAsync(serializerVersion, new(documentId), SemanticStateName, cancellationToken).ConfigureAwait(false);
if (!diagnostics.IsDefault)
{
builder.AddSemanticLocals(documentId, diagnostics);
Expand All @@ -311,7 +309,7 @@ private async ValueTask<bool> TryGetDiagnosticsFromInMemoryStorageAsync(VersionS
success = false;
}

diagnostics = await GetDiagnosticsFromInMemoryStorageAsync(serializerVersion, project, document, documentId, NonLocalStateName, cancellationToken).ConfigureAwait(false);
diagnostics = await GetDiagnosticsFromInMemoryStorageAsync(serializerVersion, new(documentId), NonLocalStateName, cancellationToken).ConfigureAwait(false);
if (!diagnostics.IsDefault)
{
builder.AddNonLocals(documentId, diagnostics);
Expand All @@ -326,7 +324,7 @@ private async ValueTask<bool> TryGetDiagnosticsFromInMemoryStorageAsync(VersionS

private async ValueTask<bool> TryGetProjectDiagnosticsFromInMemoryStorageAsync(VersionStamp serializerVersion, Project project, Builder builder, CancellationToken cancellationToken)
{
var diagnostics = await GetDiagnosticsFromInMemoryStorageAsync(serializerVersion, project, document: null, project.Id, NonLocalStateName, cancellationToken).ConfigureAwait(false);
var diagnostics = await GetDiagnosticsFromInMemoryStorageAsync(serializerVersion, new(project.Id), NonLocalStateName, cancellationToken).ConfigureAwait(false);
if (!diagnostics.IsDefault)
{
builder.AddOthers(diagnostics);
Expand All @@ -337,10 +335,8 @@ private async ValueTask<bool> TryGetProjectDiagnosticsFromInMemoryStorageAsync(V
}

private ValueTask<ImmutableArray<DiagnosticData>> GetDiagnosticsFromInMemoryStorageAsync(
VersionStamp serializerVersion, Project project, TextDocument? document, object key, string stateKey, CancellationToken _)
VersionStamp serializerVersion, ProjectOrDocumentId key, string stateKey, CancellationToken _)
{
Contract.ThrowIfFalse(document == null || document.Project == project);

return InMemoryStorage.TryGetValue(_owner.Analyzer, (key, stateKey), out var entry) && serializerVersion == entry.Version
? new(entry.Diagnostics)
: default;
Expand All @@ -357,12 +353,12 @@ private void RemoveInMemoryCache(DiagnosticAnalysisResult lastResult)

private void RemoveInMemoryCacheEntries(DocumentId id)
{
RemoveInMemoryCacheEntry(id, SyntaxStateName);
RemoveInMemoryCacheEntry(id, SemanticStateName);
RemoveInMemoryCacheEntry(id, NonLocalStateName);
RemoveInMemoryCacheEntry(new(id), SyntaxStateName);
RemoveInMemoryCacheEntry(new(id), SemanticStateName);
RemoveInMemoryCacheEntry(new(id), NonLocalStateName);
}

private void RemoveInMemoryCacheEntry(object key, string stateKey)
private void RemoveInMemoryCacheEntry(ProjectOrDocumentId key, string stateKey)
{
// remove in memory cache if entry exist
InMemoryStorage.Remove(_owner.Analyzer, (key, stateKey));
Expand Down
4 changes: 1 addition & 3 deletions src/LanguageServer/Protocol/Handler/Diagnostics/ProjectOrDocumentId.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,16 @@ namespace Microsoft.CodeAnalysis.LanguageServer.Handler.Diagnostics;
/// Wrapper around project and document ids for convenience in caching diagnostic results and
/// use in the <see cref="IDiagnosticSource"/>
/// </summary>
internal readonly struct ProjectOrDocumentId
internal readonly record struct ProjectOrDocumentId
{
/// <summary>
/// Non-null if this represents a documentId. Used for equality comparisons.
/// </summary>
[SuppressMessage("CodeQuality", "IDE0052:Remove unread private members", Justification = "Used for equality comparison.")]
private readonly DocumentId? _documentId;

/// <summary>
/// Non-null if this represents a projectId. Used for equality comparisons.
/// </summary>
[SuppressMessage("CodeQuality", "IDE0052:Remove unread private members", Justification = "Used for equality comparison.")]
private readonly ProjectId? _projectId;

public ProjectOrDocumentId(ProjectId projectId)
Expand Down
1 change: 1 addition & 0 deletions src/Workspaces/Remote/ServiceHub/Services/DiagnosticAnalyzer/DiagnosticComputer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,7 @@ private async Task<CompilationWithAnalyzersCacheEntry> CreateCompilationWithAnal
{
hostAnalyzerBuilder.AddRange(analyzers);
}

analyzerMapBuilder.AppendAnalyzerMap(analyzers);
}

Expand Down

0 comments on commit 45350db

Please sign in to comment.