Skip to content

Commit

Permalink
Switch to strongly typed keys in diagnostics subsystem
Browse files Browse the repository at this point in the history
  • Loading branch information
@CyrusNajmabadi
CyrusNajmabadi committed Feb 5, 2025
1 parent 5314c32 commit 950d607
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 35 deletions.
23 changes: 7 additions & 16 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,10 +14,10 @@ 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);

Expand All @@ -25,16 +26,16 @@ public static bool TryGetValue(DiagnosticAnalyzer analyzer, (object key, string
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
Expand Down Expand Up @@ -63,16 +64,6 @@ private static void AssertKey((object key, string stateKey) key)
}

// 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 @@ -226,12 +227,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 @@ -286,11 +287,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 @@ -300,7 +298,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 @@ -310,7 +308,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 @@ -320,7 +318,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 @@ -335,7 +333,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 @@ -346,10 +344,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 @@ -366,12 +362,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
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 950d607

Please sign in to comment.